Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make top-level errors consistent and observable #6514

Merged
merged 1 commit into from
Jun 3, 2022

Conversation

glasser
Copy link
Member

@glasser glasser commented Jun 2, 2022

Previously, errors that weren't specially cased as part of the "request
pipeline" were not observable by plugins. (In some but not all cases,
they were visible to formatError hooks.) Some "HTTP-level" errors were
handled by returning text/plain responses, whereas many other errors
were handled by returning GraphQL-style JSON responses (using
the formatError hook).

This change makes things more consistent by returning all errors as JSON
responses (using the formatError hook). In addition, for better
observability, several new top-level plugin APIs are introduced that are
called when various kinds of errors occur. Specifically:

  • New plugin hook startupDidFail. This is called once if startup fails
    (eg, if a serverWillStart plugin throws or if gateway fails to load
    the schema). (If this hook throws, its error is logged.) Future calls to
    executeHTTPGraphQLRequest (if the server was started in the
    background) now return a formatted JSON error instead of throwing.

  • New plugin hook contextCreationDidFail. (If this hook throws, its
    error is logged.) As before, these errors are sent as formatted JSON
    errors.

  • New plugin hook invalidRequestWasReceived. This is called for all
    the errors that occur before being able to construct a GraphQLRequest
    from an HTTPGraphQLRequest; this includes things like the top-level
    fields being of the wrong type, missing body, CSRF failure, etc).
    These errors are now sent as formatted JSON instead of as text/plain.

  • New plugin hook unexpectedErrorProcessingRequest. This is called if
    processGraphQLRequest throws (typically because a plugin hook
    threw). We are making the assumption that this means there is a
    programming error, so we are making the behavior change that the
    thrown error will be logged and "Internal server error" will be sent
    in the HTTP response.

  • Fix grammar of "GET supports only query operation" and don't return it
    if the operation is unknown.

Fixes #6140. Part of #6053.

Previously, errors that weren't specially cased as part of the "request
pipeline" were not observable by plugins. (In some but not all cases,
they were visible to formatError hooks.) Some "HTTP-level" errors were
handled by returning text/plain responses, whereas many other errors
were handled by returning GraphQL-style JSON responses (using
the formatError hook).

This change makes things more consistent by returning all errors as JSON
responses (using the formatError hook). In addition, for better
observability, several new top-level plugin APIs are introduced that are
called when various kinds of errors occur. Specifically:

- New plugin hook `startupDidFail`. This is called once if startup fails
  (eg, if a serverWillStart plugin throws or if gateway fails to load
  the schema). (If this hook throws, its error is logged.) Future calls to
  executeHTTPGraphQLRequest (if the server was started in the
  background) now return a formatted JSON error instead of throwing.

- New plugin hook `contextCreationDidFail`. (If this hook throws, its
  error is logged.) As before, these errors are sent as formatted JSON
  errors.

- New plugin hook `invalidRequestWasReceived`. This is called for all
  the errors that occur before being able to construct a GraphQLRequest
  from an HTTPGraphQLRequest; this includes things like the top-level
  fields being of the wrong type, missing body, CSRF failure, etc).
  These errors are now sent as formatted JSON instead of as text/plain.

- New plugin hook `unexpectedErrorProcessingRequest`. This is called if
  `processGraphQLRequest` throws (typically because a plugin hook
  threw). We are making the assumption that this means there is a
  programming error, so we are making the behavior change that the
  thrown error will be logged and "Internal server error" will be sent
  in the HTTP response.

- Fix grammar of "GET supports only query operation" and don't return it
  if the operation is unknown.

Fixes #6140. Part of #6053.
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 2, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1310c1b:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

@glasser
Copy link
Member Author

glasser commented Jun 3, 2022

@trevor-scheer and I paired on this. I'm going to merge now though it would be nice if @trevor-scheer gave it another post-merge skim.

@glasser glasser merged commit 1aeb215 into version-4 Jun 3, 2022
@glasser glasser deleted the glasser/error-handling-top-level branch June 3, 2022 00:13
@trevor-scheer
Copy link
Member

Approved in post, proposed followups in #6518

trevor-scheer added a commit that referenced this pull request Jun 3, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants