apollo-server-core: properly re-throw serverWillStop exceptions #5653
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
glasser
force-pushed
the
glasser/stop-throw
branch
from
24c31d9
to
4316e65
Compare
If a `serverWillStop` callback (or `gateway.stop()`) invoked by `server.stop()` throws, the behavior of `server.stop()` is now consistent: the original call to `server.stop()` throws the error, and any concurrent and subsequent calls to `server.stop()` throw the same error. Prior to Apollo Server v2.22.0, the original call threw the error and the behavior of concurrent and subsequent calls was undefined (in practice, it would call shutdown handlers a second time). Apollo Server v2.22.0 intended to put these semantics into place where all three kinds of calls would throw, but due to bugs, the original call would return without error and concurrent calls would hang. (Subsequent calls would correctly throw the error.) In addition, errors thrown by the `drainServer` hook introduced in Apollo Server v3.2.0 are now handled in the same way. The bugs are: - The error just wasn't being rethrown after being saved - `barrier.resolve()` wasn't called so concurrent calls would hang - `drainServer` was not in the same `try` block as `serverWillStop` - There weren't tests Fixes #5649.
glasser
force-pushed
the
glasser/stop-throw
branch
from
4316e65
to
7d1ea20
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
If a
serverWillStopcallback (orgateway.stop()) invoked byserver.stop()throws, the behavior ofserver.stop()is nowconsistent: the original call to
server.stop()throws the error, andany concurrent and subsequent calls to
server.stop()throw the sameerror.
Prior to Apollo Server v2.22.0, the original call threw the error and
the behavior of concurrent and subsequent calls was undefined (in
practice, it would call shutdown handlers a second time).
Apollo Server v2.22.0 intended to put these semantics into place where
all three kinds of calls would throw, but due to bugs, the original call
would return without error and concurrent calls would hang. (Subsequent
calls would correctly throw the error.)
In addition, errors thrown by the
drainServerhook introduced inApollo Server v3.2.0 are now handled in the same way.
The bugs are:
barrier.resolve()wasn't called so concurrent calls would hangdrainServerwas not in the sametryblock asserverWillStopFixes #5649.