Question: Buffer and Shared. How to detect Poll::Ready(Err(_))?

[yokomizor]

Hi there,

I am trying to combine Buffer and Shared as described in the Shared service doc:

tower/tower/src/make/make_service/shared.rs

Lines 59 to 65 in 7f004da

	/// let svc = MyService;
	///
	/// // Make it `Clone` by putting a channel in front
	/// let buffered = Buffer::new(svc, 1024);
	///
	/// // Convert it into a `MakeService`
	/// let make = Shared::new(buffered);

In this example, if svc.poll_ready returns Poll::Ready(Err(_)), then buffered.poll_ready will propagate it and stop calling the inner service, since it is no good anymore.

make however, will keep returning Poll::Ready(Ok(())) and cloning the bad buffered service.

tower/tower/src/make/make_service/shared.rs

Lines 91 to 97 in 7f004da

	fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
	Poll::Ready(Ok(()))
	}
	fn call(&mut self, _target: T) -> Self::Future {
	SharedFuture::new(futures_util::future::ready(Ok(self.service.clone())))
	}

I could not find a way to make the shared service detect this scenario, so I ended up implementing my own shared service with some basic recover logic.

In this scenario, I understand that both svc and buffered should be dropped. The layer above them should be notified somehow so it could try to recover or just propagate the error all the way to the top.

Did I get it right? If yes, then I would say Shared and Buffered don't play well together, and I would also suggest changing the example in the docs.

cheer!

[yokomizor]

I have just realized that Reconnect exists. I am considering a setup like Shared<Buffer<Reconnect<MakeService, Target>, Request>>.

It would not cover the case Err(_) = MakeService::poll_ready(...) though.

p.s. I am just playing around with tower, not trying to achieve anything specific, so I am not sure if this would be a problem in the real world.

[davidpdrsn]

Hm it feels like a bug to me that Shared doesn’t forward poll_ready to the inner service 🤔

although it would require adding a S: Service bound to fix, which I guess is a breaking change 😞

[yokomizor]

also type Error = S::Service::Error...