Proper keep-alive #11
Merged
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
yoshuawuyts
reviewed
Nov 13, 2019
| req = match timeout(timeout_duration, decode(stream)).await { | ||
| Ok(Ok(Some(r))) => r, | ||
| Ok(Ok(None)) | Err(TimeoutError { .. }) => break, /* EOF or timeout */ | ||
| Ok(Err(e)) => return Err(e), |
There was a problem hiding this comment.
Heh, how do we handle the top-level Err case? Are we maybe double wrapping the Result accidentally?
There was a problem hiding this comment.
The outer error is the timeout even though in our case it's not actually an error. The inner error is in fact an actual error in reading the underlying TCP stream. This PR doesn't change how that error gets handled. It currently is propagated to the user so they can handle such errors.
|
@yoshuawuyts can we merge this? wanted to hack some |
|
@dignifiedquire @rylev feel free to review PRs and merge! -- I think you both get the spirit of this library, so please don't let me be a limiting factor to review and move forward! |
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.
The previous implementation was wrong due to me not understanding how things actually work 馃榾. This now keeps a connection alive until either waiting for a new HTTP request times out or the connection is closed.