Ensure startup errors are redacted even the first time #5064
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/redact-all-startup-errors
branch
from
081ff9e
to
cde6656
Compare
glasser
commented
Mar 26, 2021
| @@ -1270,7 +1274,10 @@ export class ApolloServerBase { | |||
| * `{req: express.Request, res: express.Response }` object) and to keep it | |||
| * updated as you upgrade Apollo Server. | |||
| */ | |||
| public async executeOperation(request: GraphQLRequest, integrationContextArgument?: Record<string, any>) { | |||
| public async executeOperation( | |||
There was a problem hiding this comment.
I've been keeping this particular file prettier-happy but failed to do so on a PR I merged today, sigh.
There was a problem hiding this comment.
eh I'll roll this part of the diff back. since I do want to cherry-pick this on top of the last release.
glasser
force-pushed
the
glasser/redact-all-startup-errors
branch
from
cde6656
to
648aa2a
Compare
This is a regression in #4981. If the server start process is begun implicitly by the execution of an operation (ensureStarted inside graphQLServerOptions) and startup throws, the log-and-redact logic wasn't being invoked. Note that this case doesn't usually happen in practice, because: - If you're using `apollo-server`, startup is begun in `listen()` before you can serve requests - If you're using a serverless framework integration, startup is begun in the constructor - If you're using a non-serverless framework integration, the function you call to connect it to your framework begins startup with `ensureStarting()` So mostly this just affects the case that you're running `executeOperation` without calling `start()` or `listen()`, or maybe you have your own custom framework integration that doesn't call `ensureStarting()`. But it's still worth missing. Add some tests of this behavior and fix some TypeScript issues in the test file.
glasser
force-pushed
the
glasser/redact-all-startup-errors
branch
from
648aa2a
to
285c5a5
Compare
glasser
added a commit
that referenced
this pull request
Mar 26, 2021
This is a regression in #4981. If the server start process is begun implicitly by the execution of an operation (ensureStarted inside graphQLServerOptions) and startup throws, the log-and-redact logic wasn't being invoked. Note that this case doesn't usually happen in practice, because: - If you're using `apollo-server`, startup is begun in `listen()` before you can serve requests - If you're using a serverless framework integration, startup is begun in the constructor - If you're using a non-serverless framework integration, the function you call to connect it to your framework begins startup with `ensureStarting()` So mostly this just affects the case that you're running `executeOperation` without calling `start()` or `listen()`, or maybe you have your own custom framework integration that doesn't call `ensureStarting()`. But it's still worth missing. Add some tests of this behavior and fix some TypeScript issues in the test file.
|
We're seeing a bunch of errors when trying to start the apollo-server-lambda in RedwoodJS. Pinning the |
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.
This is a regression in #4981. If the server start process is begun implicitly
by the execution of an operation (ensureStarted inside graphQLServerOptions)
rather than by an explicit start call (or the implicit start calls in
apollo-serveror the serverless integrations) and startup throws, thelog-and-redact logic wasn't being invoked.
Note that this case doesn't usually happen in practice, because:
apollo-server, startup is begun inlisten()before you canserve requests
constructor
to connect it to your framework begins startup with
ensureStarting()So mostly this just affects the case that you're running
executeOperationwithout calling
start()orlisten(), or maybe you have your own customframework integration that doesn't call
ensureStarting(). But it's still worthfixing.
Add some tests of this behavior and fix some TypeScript issues in the test file.