Errors thrown by serverWillStop handlers aren't rethrown by stop() #5649
Labels
size/small
Estimated to take LESS THAN A DAY
Comments
glasser
added a commit
that referenced
this issue
Aug 25, 2021
If a `serverWillStop` callback invoked by `server.stop()` throws (or `gateway.stop()`), then `server.stop()` now correctly throws that error instead of returning. This was a regression introduced in Apollo Server v2.22.0. (Between v2.22 and this release, a second subsequent or concurrent call to `server.stop()` would correctly throw the error, but not the original call.) Fixes #5649.
glasser
added a commit
that referenced
this issue
Aug 25, 2021
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
added a commit
that referenced
this issue
Aug 25, 2021
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
added a commit
that referenced
this issue
Aug 25, 2021
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.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
If serverWillStop throws, the error gets saved into the internal state under
stopErrorand future (or concurrent) calls tostopwill throw it, but thenstop()just returns instead of re-throwing stopError. Pretty sure this is basically just a typo.The text was updated successfully, but these errors were encountered: