fix(transport): return Poll::ready until error is consumed #536
Conversation
When a lazy connection fails to connect it first returns Poll::ready from the reconnect service, yet the subsequent call returns Poll::pending making tower_balance loop forever. Instead, on error we return Ready until the error is consumed in the call method.
…helge-hoff/tonic into tl-helge-hoff-helge-reconnect-forever-loop
| if let Some(error) = self.error.take() { | ||
| return ResponseFuture::error(error); | ||
| tracing::error!("error: {:?}", error); |
There was a problem hiding this comment.
Why log messages at error when we're returning the error? As this is a library, it seems preferable to keep these at something like debug so that the application has more control over whether these are handled/logged, etc.
There was a problem hiding this comment.
Also, if I understand correctly, this would log the errors twice -- once above in poll_ready and then again at call-time.
There was a problem hiding this comment.
Yeah, no one has complained about it yet, but I think thats not a bad idea. Let me do a pass of those in a follow up PR.
There was a problem hiding this comment.
no one has complained about it yet
If I'm correct, this logging was introduced by this PR and not there before...
There was a problem hiding this comment.
We had two other places where we logged at error!, so I moved them all to debug!, thats what I meant.
Fixup of #495