Router::layer should allow services whose Error: Into<Infallible>

[lilyball]

Bug Report

Version

axum v0.4.8
axum v0.5.0

Crates

axum

Description

I'm writing a generic tower::Service that needs to wrap two other services and thus combine their errors. I would like to represent this using a strongly-typed error enum, to be able to distinguish between errors raised by either service. The problem is if both services have Error = Infallible then my service will still have a strongly-typed uninhabited error. It can't ever produce an error but it won't be compatible with axum::Router::layer().

My idea here is to implement From<MyError<Infallible, Infallible>> for Infallible such that my error can be trivially converted to Infallible if it's uninhabited. This still won't make it directly compatible with Router::layer() though, but if Router::layer() required Error: Into<Infallible> rather than Error = Infallible it would all work.

More generally, any time that implements Into<Infallible> is presumably uninhabited, and axum really just cares that the error isn't possible rather than needing it to be the specific type Infallible. And so this should be reflected in the type system.

[lilyball]

A potential worry here is if a layer supports multiple error types and is relying on the use-case to constrain it to a specific type, using Into<Infallible> will presumably cause an ambiguous type error. But such a layer would need to include this error type in its own type parameters and so the workaround is relatively simple.

Compare that with what we have now, where I have to wrap my layer in tower::ServiceBuilder::new().map_err(Into::into).layer(my_layer) just to map the error.

[davidpdrsn]

Sounds fine to me!

[davidpdrsn]

Turns out #924 was a breaking change so we have to yank+revert. So keeping this issue open for now. This has unfortunately has to wait until axum 0.6 which we don't have a timeline for at the moment.

[davidpdrsn]

#948 is open now which includes this, however as mentioned its a breaking change so we cannot merge it yet.

However with the PR open I think I'll just close this for now as its accepted and implemented.