util: rework Either

[davidpdrsn]

In practice I've found Either be hard to use since it changes the error type to BoxError. That means if you combine two infallible services you get a service that, to the type system, is fallible. That doesn't work well with axum's error handling model which requires all services to be infallible and thus always return a response. So you end up having to add boilerplate just to please the type system.

Additionally, the fact that Either implements Future also means we cannot fully remove the dependency on pin-project since pin-project-lite doesn't support tuple enum variants, only named fields.

This PR reworks Either to address these:

• It now requires the two services to have the same error type so no type information is lost. I did consider doing something like where B::Error: From<A::Error> but I hope this simpler model will lead to better compiler errors.
• Changes the response future to be a struct with a private enum using pin-project-lite
• Removes the Future impl so we can remove the dependency on pin-project

Goes without saying that this is a breaking change so we have to wait until tower 0.5 to ship this.

cc @jplatte

Fixes #594
Fixes #550

[jplatte]

Have you considered implementing Layer and Service for futures_util::future::Either instead and using that type in the relevant places? That approach seemed a little bit cleaner to me when going over the possible solutions last time.

[jplatte]

Additionally, IIRC there was some talk about changing A and B to Left and Right, which should maybe be part of the same PR?

[davidpdrsn]

Have you considered implementing Layer and Service for futures_util::future::Either instead and using that type in the relevant places? That approach seemed a little bit cleaner to me when going over the possible solutions last time.

That would mean tower gets a public dependency on futures_util which I don't think we have today 🤔

Additionally, IIRC there was some talk about changing A and B to Left and Right, which should maybe be part of the same PR?

Yeah that seems fine to me. I'll do that.

[hawkw]

Have you considered implementing Layer and Service for futures_util::future::Either instead and using that type in the relevant places? That approach seemed a little bit cleaner to me when going over the possible solutions last time.

That would mean tower gets a public dependency on futures_util which I don't think we have today

FWIW, tower does depend on futures_util, but it's not exposed in any public APIs as far as I know.

[davidpdrsn]

Actually implementing Service and Layer directly for futures_util::future::Either would require putting those impls in tower-service and tower-layer and probably get in the way of making them 1.0, at least until futures_util itself is 1.0. Dunno what the timeline is on that but I doubt it'll happen any time soon.

[hawkw]

Actually implementing Service and Layer directly for futures_util::future::Either would require putting those impls in tower-service and tower-layer and probably get in the way of making them 1.0, at least until futures_util itself is 1.0. Dunno what the timeline is on that but I doubt it'll happen any time soon.

Oh, you're right, I didn't think through the orphan rules here. I don't think we'll be able to do that, then.

We could use futures_util::future::Either instead of the custom response future type, but I don't think it's worth making it public API for that purpose. The implementation is trivial enough that we don't need to use it from futures_util, I think the only motivation would be compatibility, and compatibility would require putting it in tower-service, which I don't think we should do.

[hawkw]

seems good to me (in 0.5, of course)

[olix0r]

I'm generally +1 on this. we should do 0.5 sooner than later...

The CI failure seems legit, though.