refactor: overhaul errors

[Fishrock123]

Overhauls errors to allow for 3 types of errors:

• Concrete errors (enum) from http-types itself
• A dynamic error type for response handlers (almost identical to how Error was previously)
• A combined type for handling request+response for a client

There is some exposition for this in Zulip: https://http-rs.zulipchat.com/#narrow/stream/261414-http-types/topic/New.20error.20types.20.26.20result.20apis/near/254273467

Basically this solves two problems:

• Errors from within http-types/tide/surf ware currently unable to be matched upon except by string search.
• Surf's and http-types's error return types not being StdError is a massive pain point.

The first problem is pretty simple to solve but was quite laborious. It meant changing all those bail!("some string error") and what not to be concrete enum variants to some degree of specificity. I have kept some errors grouped together where the string output could mean something to anyone debugging a problem but is unlikely to be something which someone would reasonably want to check during program runtime. This is to reduce the amount of labor doing this but also to cap the enum variants and prevent the number of variants from becoming unmanageable.

The second problem is tricky and weird. My solution is RequestError. This allows dynamic errors from surf middleware but also allows us to have concrete errors there. Some indirection is required for ResponseError due to StdError's conflict with anyhow (The problem is a conflict with From<T> for T, which I will never not be profusely annoyed at).

---

There are some potential issues here / open questions:

• Does this actually work well with Tide and Surf?
  • Basically, those libraries will also need their own concrete error types, and one of the varients would be HTTP(http_types::Error) or such, which... isn't really correct.
  • We could solve that by removing the central error enum here and only having the specific sub error types HeaderError, BodyError, etc. Would need to make a couple extra types.
  • We could also reinterpret the main http-types error enum and translate it directly to surf and tide ones.
  • I have a slightly worry about how this would work between tide and surf, i.e. Tide would have to turn a Surf concrete error into a dynamic error unless it shared the same error enum for all cases even from those libraries.
• Should Error::ArgTryIntoError(Box<dyn std::error::Error + Send + Sync + 'static>) exist? Would returning Result<Result<T>> in the necessary places be better? (@yoshuawuyts this is what I was asking on Zulip)
• Are the Send + Sync + 'static requirements correct or a problem?

---

Edit: the diff is obviously quite large, so here are the interesting parts:

• errors::error_kind.rs
• errors::mod.rs
• errors::request_error.rs
• errors::response_error.rs

[Fishrock123]

Hmph. The CI duly notes that this makes interop with hyper http more difficult (obviously).

[Fishrock123]

@yoshuawuyts Hey I would like to merge this tomorrow if there is no objection

[Fishrock123]

Made one additional change to this... all the enums are #[non_exhaustive] now so we can add more cases in minors where it fits well

[joshtriplett]

Posted a few bits of feedback, but generally this looks great. Thanks for the substantial amount of work here.

[Fishrock123]

@yoshuawuyts if you are going to write a bunch of typed headers could you please take a look at this first?

[yoshuawuyts]

@Fishrock123 thanks so much for the ping! - we were actually running into very similar issues around error handling while looking to integrate http-types into the Azure SDK (https://github.com/Azure/azure-sdk-for-rust/issues/848), and I'm fairly comfortable saying that this seems like the right direction to go.

I still need to look at this line-by-line, but want to at least respond that I agree that we should land this before any further work on typed headers.

[rbtcollins]

I haven't read the diff, but just a quick though since future API breakage seems to be a strong concern here. Consider making all the concrete enums nonexhaustive. That will permit expanding them in future without API breakage - everyone will need to explicitly handle unexpected errors and not assume the error space will expand. (See e.g. filesystem errors in the stdlib for the pain of not doing this in advance).

[Fishrock123]

I haven't read the diff, but just a quick though since future API breakage seems to be a strong concern here. Consider making all the concrete enums nonexhaustive. That will permit expanding them in future without API breakage - everyone will need to explicitly handle unexpected errors and not assume the error space will expand. (See e.g. filesystem errors in the stdlib for the pain of not doing this in advance).

Yes, it will be a large api change. I think that’s fine since no one really likes the current api anyway.

Also, yes I already did mark them all as #[non_exhaustive]: https://github.com/http-rs/http-types/pull/395/files#diff-87f966704ebddb378e447e13ddf9ee203cd9d31bb502338a5f3ed3478df41588R9

[OneOfOne]

I'm maintaining a hard fork of this repo that I plan to keep up to date, please resubmit this PR at https://github.com/OneOfOne/http-types-rs