Pure JS: Fixed two bugs with goaway handling #908
Merged
Conversation
This file contains hidden or 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
nicolasnoble
reviewed
Jun 7, 2019
| * function is in the call stack, any exception thrown by the application | ||
| * (or our tests) will bubble up and turn into promise rejection, which | ||
| * will result in an UnhandledPromiseRejectionWarning. Because that is | ||
| * a warning, the error will be effectively swallowed and execution will |
Contributor
There was a problem hiding this comment.
Note this isn't a warning any longer in node 12. But yes.
nicolasnoble
approved these changes
Jun 7, 2019
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
First, I think this fixes #734. When a Node http2 client session receives a GOAWAY from the server, it sets its closed flag and emits a
'goaway'event immediately, and then emits the'close'and'error'(if needed) events a tick or two later. We listen for the'close'event to know when a session should be considered unusable, and if the channel tries to start a stream in between the'goaway'event and the'close'event, it ends with an error. The fix is to simply stop using the subchannel as soon is the'goaway'event comes in.Second, RST_STREAM error codes were not getting handled correctly in the stream code, including artificial RST_STREAM errors created by the Node http2 code when a GOAWAY is received.