Fix state reverting #298
Merged
Fix state reverting #298
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
artemredkin
force-pushed
the
fix_state_reverting
branch
from
3d48ccc to
91c7263
Compare
Lukasa
reviewed
Aug 24, 2020
| if !head.isKeepAlive { | ||
| self.closing = true | ||
| } | ||
| switch self.state { |
There was a problem hiding this comment.
This may read more clearly as:
if case .endOrError = self.state {
return
}Rather than forcing the indentation of the rest of the body.
There was a problem hiding this comment.
ah! thank you, always forget about this pattern
There was a problem hiding this comment.
Seems like the only place you didn't apply this pattern was this block? 😉
| self.state = .bodySent | ||
| if let expectedBodyLength = self.expectedBodyLength, expectedBodyLength != self.actualBodyLength { | ||
| let error = HTTPClientError.bodyLengthMismatch | ||
| self.errorCaught(context: context, error: error) |
There was a problem hiding this comment.
We don't need errorCaught here: the future chain will call this code path anyway.
There was a problem hiding this comment.
fixed, thanks!
artemredkin
force-pushed
the
fix_state_reverting
branch
from
91c7263 to
21c826f
Compare
Motivation: Right now if task handler encounters an error, it changes state to `.endOrError`. We gate on that state to make sure that we do not process errors in the pipeline twice. Unfortunately, that state can be reset when we upload body or receive response parts. Modifications: Adds state validation before state is updated to a new value Adds a test Result: Fixes swift-server#297
artemredkin
force-pushed
the
fix_state_reverting
branch
from
21c826f to
c7b67e0
Compare
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.
Prevent TaskHandler state change after
.endOrErrorMotivation:
Right now if task handler encounters an error, it changes state to
.endOrError. We gate on that state to make sure that we do not process errors in the pipeline twice. Unfortunately, that state can be reset when we upload body or receive response parts.Modifications:
Adds state validation before state is updated to a new value
Adds a test
Result:
Fixes #297