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

Don't override custom code extensions from scalar parseValue errors #7183

Merged
merged 3 commits into from Nov 23, 2022

Conversation

glasser
Copy link
Member

@glasser glasser commented Nov 22, 2022

Adding BAD_USER_INPUT is a nice default (and overrides the inappropriate default of INTERNAL_SERVER_ERROR) but if somebody has set a code already, we shouldn't override.

(Note that there's a separate issue where graphql-js throws out extensions from the thrown error itself and only pays attention to extensions on the error's originalError; we're trying to fix that in graphql/graphql-js#3785 but this is orthogonal.)

Fixes #7178.

Adding `BAD_USER_INPUT` is a nice default (and overrides the
inappropriate default of `INTERNAL_SERVER_ERROR`) but if somebody has
set a `code` already, we shouldn't override.

(Note that there's a separate issue where graphql-js throws out
extensions from the thrown error itself and only pays attention to
extensions on the error's originalError; we're trying to fix that in
graphql/graphql-js#3785 but this is orthogonal.)

Fixes #7178.
@netlify
Copy link

netlify bot commented Nov 22, 2022

Deploy Preview for apollo-server-docs ready!

Name Link
🔨 Latest commit 527f787
🔍 Latest deploy log https://app.netlify.com/sites/apollo-server-docs/deploys/637d71399e51e3000ae86eff
😎 Deploy Preview https://deploy-preview-7183--apollo-server-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 22, 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 527f787:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

packages/server/src/requestPipeline.ts Outdated Show resolved Hide resolved
Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>
@glasser glasser enabled auto-merge (squash) November 22, 2022 22:40
@glasser glasser merged commit 46af825 into main Nov 23, 2022
@glasser glasser deleted the glasser/bad-user-input-only-as-override branch November 23, 2022 01:05
@github-actions github-actions bot mentioned this pull request Nov 23, 2022
glasser pushed a commit that referenced this pull request Nov 23, 2022
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @apollo/server-integration-testsuite@4.2.0

### Minor Changes

- [#7171](#7171)
[`37b3b7fb5`](37b3b7f)
Thanks [@glasser](https://github.com/glasser)! - If a POST body contains
a non-string `operationName` or a non-object `variables` or
`extensions`, fail with status code 400 instead of ignoring the field.

In addition to being a reasonable idea, this provides more compliance
with the "GraphQL over HTTP" spec.

This is a backwards incompatible change, but we are still early in the
Apollo Server 4 adoption cycle and this is in line with the change
already made in Apollo Server 4 to reject requests providing `variables`
or `extensions` as strings. If this causes major problems for users who
have already upgraded to Apollo Server 4 in production, we can consider
reverting or partially reverting this change.

### Patch Changes

- [#7170](#7170)
[`4ce738193`](4ce7381)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Update
@apollo/utils packages to v2 (dropping node 12 support)

- [#7179](#7179)
[`c8129c23f`](c8129c2)
Thanks [@renovate](https://github.com/apps/renovate)! - Fix a few tests
to support (but not require) TypeScript 4.9.

- [#7171](#7171)
[`37b3b7fb5`](37b3b7f)
Thanks [@glasser](https://github.com/glasser)! - The integration test
suite now incorporates the `graphql-http` package's audit suite for the
"GraphQL over HTTP" specification.

- [#7183](#7183)
[`46af8255c`](46af825)
Thanks [@glasser](https://github.com/glasser)! - Apollo Server tries to
detect if execution errors are variable coercion errors in order to give
them a `code` extension of `BAD_USER_INPUT` rather than
`INTERNAL_SERVER_ERROR`. Previously this would unconditionally set the
`code`; now, it only sets the `code` if no `code` is already set, so
that (for example) custom scalar `parseValue` methods can throw errors
with specific `code`s. (Note that a separate graphql-js bug can lead to
these extensions being lost; see
<graphql/graphql-js#3785> for details.)

- Updated dependencies
\[[`4ce738193`](4ce7381),
[`37b3b7fb5`](37b3b7f),
[`b1548c1d6`](b1548c1),
[`7ff96f533`](7ff96f5),
[`46af8255c`](46af825)]:
    -   @apollo/server@4.2.0

## @apollo/server@4.2.0

### Minor Changes

- [#7171](#7171)
[`37b3b7fb5`](37b3b7f)
Thanks [@glasser](https://github.com/glasser)! - If a POST body contains
a non-string `operationName` or a non-object `variables` or
`extensions`, fail with status code 400 instead of ignoring the field.

In addition to being a reasonable idea, this provides more compliance
with the "GraphQL over HTTP" spec.

This is a backwards incompatible change, but we are still early in the
Apollo Server 4 adoption cycle and this is in line with the change
already made in Apollo Server 4 to reject requests providing `variables`
or `extensions` as strings. If this causes major problems for users who
have already upgraded to Apollo Server 4 in production, we can consider
reverting or partially reverting this change.

- [#7184](#7184)
[`b1548c1d6`](b1548c1)
Thanks [@glasser](https://github.com/glasser)! - Don't automatically
install the usage reporting plugin in servers that appear to be hosting
a federated subgraph (based on the existence of a field `_Service.sdl:
String`). This is generally a misconfiguration. If an API key and graph
ref are provided to the subgraph, log a warning and do not enable the
usage reporting plugin. If the usage reporting plugin is explicitly
installed in a subgraph, log a warning but keep it enabled.

### Patch Changes

- [#7170](#7170)
[`4ce738193`](4ce7381)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Update
@apollo/utils packages to v2 (dropping node 12 support)

- [#7172](#7172)
[`7ff96f533`](7ff96f5)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! -
startStandaloneServer: Restore body-parser request limit to 50mb (as it
was in the `apollo-server` package in Apollo Server 3)

- [#7183](#7183)
[`46af8255c`](46af825)
Thanks [@glasser](https://github.com/glasser)! - Apollo Server tries to
detect if execution errors are variable coercion errors in order to give
them a `code` extension of `BAD_USER_INPUT` rather than
`INTERNAL_SERVER_ERROR`. Previously this would unconditionally set the
`code`; now, it only sets the `code` if no `code` is already set, so
that (for example) custom scalar `parseValue` methods can throw errors
with specific `code`s. (Note that a separate graphql-js bug can lead to
these extensions being lost; see
<graphql/graphql-js#3785> for details.)

- Updated dependencies
\[[`4ce738193`](4ce7381),
[`45856e1dd`](45856e1)]:
    -   @apollo/server-gateway-interface@1.0.6

## @apollo/server-gateway-interface@1.0.6

### Patch Changes

- [#7170](#7170)
[`4ce738193`](4ce7381)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Update
@apollo/utils packages to v2 (dropping node 12 support)

- [#7173](#7173)
[`45856e1dd`](45856e1)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Remove
unnecessary engines constraint on types-only package

## @apollo/server-plugin-response-cache@4.0.2

### Patch Changes

- [#7170](#7170)
[`4ce738193`](4ce7381)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Update
@apollo/utils packages to v2 (dropping node 12 support)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 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.

Error from GraphQLScalarType is not properly propagated to formatError hook
2 participants