reconnect: Fix maker error to return in future #386
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hm, this is a significant behavior change, I'll need to think through its repercussions. This also seems related to #244. |
|
Yeah, 100% related, funny how I came to the same conclusion again naturally #244 (comment) @seanmonstar so I guess if we think about it this way, a service should be thrown away if poll_ready returns an error, in this case its not returning an error thus our inner service is valid. Therefore, it makes sense to return a make_service error on call since we got that error from call and not from poll_ready. |
|
@seanmonstar friendly ping |
seanmonstar
approved these changes
Apr 14, 2020
|
I am going to reopen this since we moved things around. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #383
These changes reconnect to return the make service failure in the future instead of in poll_ready. This change allows us to reuse the reconnect service on failed reconnects. This pushes the error down to the caller of
callsince technically themake_serviceis ready. This makes it so you can keep calling call until the service reconnects.This service is a bit odd, because it doesn't really fit in with the other types of services. This is because its an adapter to go from
MakeServicetoService. Thus, thepoll_readycontract here doesn't mean the same as a regularServicebecause it has to bundle connecting.This question brings up a big issue with how buffer is implemented, because if
poll_readyreturns the error then we must throw away the service, and buffer will just keep returning the failed connection error, even if we can successfully reestablish.