refactor(s2n-quic): Add Send + Sync to StartError

[jmayclin]

Resolved issues:

#1709

Description of changes:

This PR adds Start and Sync to StartError, which also requires adding it to the error types that the providers use.

refactor(s2n-quic): Add Send + Sync to StartError

This commit adds Send and Sync bounds to the StartError type. This makes usage of the error across threads and tasks more ergonomic.

BREAKING CHANGE: Adding additional bounds to a public trait is a breaking change. Customers using externally implemented providers will have to add Send and Sync implementations to their error types.

Call-outs:

I think this is a breaking change :(

We are adding additional requirements for a public trait that customers may have implemented. If a customer has written their own EndpointLimits provider that uses an Error type that doesn't implement Send + Sync, they will receive a compile time error when they try and and upgrade their version of quic.

Testing:

All CI passes. I really wish something would break in our CI, because I'm pretty sure that this is a breaking change.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[camshaft]

To make the change less breaking, I wonder if we can get away with just adding Send and not Sync? But you're correct: it was an oversight not to include these bounds when the traits were made public. In practice most errors are Send so I don't anticipate it breaking much, if any.

[jmayclin]

Ya, I think we could get away with only adding Send.

However, do you think people in the future might also ask for Sync to be added? In which case would the single breaking change be better than two smaller breaking changes?

Or is the concern that Sync is more rarely implemented, so adding that to customer implemented error types is going to be more pain than adding Send?

[camshaft]

However, do you think people in the future might also ask for Sync to be added?

It's totally possible 😄.

In which case would the single breaking change be better than two smaller breaking changes?

Yeah probably better in one pass...

My thinking was maybe we don't need it and would cover 99% of the use cases. But I guess while we're here, it's probably best to just do it.

[cavemanloverboy]

Not sure if you've cut this release already, but honestly if you're introducing a breaking change I would just make it Box<dyn std::error::Error> which is sort of the Rust standard.

https://docs.rs/anyhow/latest/anyhow/struct.Error.html