tower: Consider generating error enums with `thiserror`
In https://github.com/tower-rs/tower/pull/654#discussion_r824935110, @olix0r brought up the idea of using thiserror for tower middleware error types, instead of BoxErrors everywhere:
I wonder if it's more or less onerous to take a
thiserrordependency and explicitly handle the error cases than requiring boxing. Note, for instance, thatanyhow::Errordoes not implInto<BoxError>; and we don't really have any requirements internally (i.e. on theServicetrait) thatT::Errorimplstd::error::Error.Originally posted by @olix0r in https://github.com/tower-rs/tower/pull/654#discussion_r824935110
This would mean switching back to an enum-based error world. Now that #[non_exhaustive] exists, though, this would pose less of a forwards-compatibility hazard than it did back in the day. Also, a nice benefit of thiserror for generating error enums is that it is not a public-API dependency; its derive macros would be used purely internally to tower. We could easily swap out thiserror later as long as we exposed the same error types with the same trait impls...
thiserror can also be used as a build-time tool, in that you commit the generated output of thiserror's macros through cargo-expand.
(yes, this is hacky, but it does mean you can drop thiserror if the hit to compile times is unacceptable)
- The proc macro hit seems unfortunate.
- Say more about switching back to enums. Do most just get an
Innervariant again? That sucked for trying to match on specific errors, and was even worse when layers switched orders.
I'm not totally sure what @olix0r had in mind here, FWIW, so he can probably better answer these questions.
- Say more about switching back to enums. Do most just get an
Innervariant again? That sucked for trying to match on specific errors, and was even worse when layers switched orders.
I think the intent would be that specific errors would still be extracted through downcasting or through source, which thiserror will generate. So, complex errors don't have to be matched. But, the generic error types are still an issue that I'm not sure if there's a solution for?
Could we just improve the downcast situation? I am also concerned about the deep matching that can make it really hard to get it right plus what happens when you swap layers around?
My instinct prefers enums more, but honestly it gets very messy and I think the Boxing and downcasting works just fine. Not sure how you would anyway ever compose reliable anyway because your matching logic would be messed up as soon as you reverse the order of some layers.
Maybe there is a better way then Boxing, but don't think thiserror is the way for a flexible system like tower. There might be something custom possible though that is still generic enough but has perhaps better semantics/ergonomic then boxing. But that's a whole different discussion... I think for now boxing works just fine with the downcasting... But that's just my POV. All I know is it needs a decision and if I learned anything from the internal rust language discussions that I lurk, it just needs to be a decision even if not perfect