Skip to content

Conversation

@Boog900
Copy link

@Boog900 Boog900 commented Sep 27, 2024

The trait bounds on AsService & IntoService both require that M: Service, however this defeats the purpose of the types as if we already require that M: Service then all the ServiceExt methods would already be reachable on M.

This PR changes the bounds to M: MakeService.

@Boog900
Copy link
Author

Boog900 commented Jun 9, 2025

Just to add a bit more detail on why this is needed, see this issue: #253 and this PR: #492, this code is supposed to make this possible:

use tower::{make::MakeService, ServiceExt};

async fn example<T: MakeService<(), ()>>(mut t: T) {
    let _ = t.as_service().ready().await;
}

But currently this will give a compile error as T does not impl Service.

Just looking at the Service impl on AsService will show the mistake:

impl<M, S, Target, Request> Service<Target> for AsService<'_, M, Request>
where
M: Service<Target, Response = S>,
S: Service<Request>,
{

M is required to impl Service, but it doesn't, that's why we need AsService as M doesn't impl Service it impls MakeService. We could add the Service bounds to T in my example but then that defeats the purpose of as_service/into_service.

If you instead pull in this patch you will see the example builds.

Copy link
Contributor

@ripytide ripytide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this makes sense. I tried coming up with what the trait bounds would look like if you make M: MakeService and ended up with exactly the same results.

@ripytide
Copy link
Contributor

ripytide commented Jul 5, 2025

Actually, after a bit more thought, while I still stand by that this PR fixes a accidental mistake in the blanket trait for implementing Service for all MakeServices. I don't actually see the point in the MakeService trait to begin with, in the docs it says its a trait alias, and some of the associated types/methods are named slightly differently, some are named the same as Service (like Future). Overall, I don't see it reducing the complexity of trait bounds for functions that take services returning other services and would probably choose to go without it in my own code.

And if you did want a trait alias making Service a super-trait of MakeService would make things a lot simpler.

Oh, I see MakeService is for where you don't want to explicitly implement Service to prevent it getting confused with regular Services. Hence the into_ and as_ methods that require explicit conversion. I'm still not sure it's worth the extra confusion of another trait though.

@Boog900
Copy link
Author

Boog900 commented Aug 9, 2025

@ripytide I guess this trait was more necessary when expressing trait bounds was more verbose. I can see the argument for removing this, with this PR fixing an issue though I am not sure this is the best place for that discussion, maybe a separate issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants