fix: Introduce status code regression mitigation #7465
Merged
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
Apollo Server v4 introduced a regression with respect to invalid
`variables` and http status codes. AS4 incorrectly started responding
with a 200 status code, where AS3 would respond with a 400.
Providing the following config to your AS4 constructor options will
opt-in to the regression mitigation:
```
new ApolloServer({
// ...
status400WithErrorsAndNoData: true,
})
```
✅ Deploy Preview for apollo-server-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
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 4fa3401:
|
StephenBarlow
approved these changes
Mar 23, 2023
Co-authored-by: Stephen Barlow <stephen@apollographql.com>
Co-authored-by: Stephen Barlow <stephen@apollographql.com>
Co-authored-by: Stephen Barlow <stephen@apollographql.com>
glasser
reviewed
Mar 24, 2023
docs/source/migration.mdx
Outdated
|
|
||
| ### Appropriate 400 status codes | ||
|
|
||
| Apollo Server v4 will respond to an invalid `variables` object with a _200_ status code, where v3 would correctly respond with a 400 status code. This regression was introduced in [PR #6502]https://github.com/apollographql/apollo-server/pull/6502) and brought to our attention in [Issue #7462](https://github.com/apollographql/apollo-server/issues/7462). |
There was a problem hiding this comment.
maybe give examples of what invalid means (I think the big ones are "surprisingly null/surprisingly missing fields on input objects" and "scalar function whatever it's called throws"). Maybe in the changeset too?
Might be worth noting that v4 does 400 on, eg, variables key that doesn't map to an object, idk.
clenfest
approved these changes
Mar 24, 2023
glasser
approved these changes
Mar 24, 2023
Co-authored-by: David Glasser <glasser@apollographql.com>
Merged
trevor-scheer
pushed a commit
that referenced
this pull request
Apr 3, 2023
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@4.6.0 ### Minor Changes - [#7465](#7465) [`1e808146a`](1e80814) Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Introduce new opt-in configuration option to mitigate v4 status code regression Apollo Server v4 accidentally started responding to requests with an invalid `variables` object with a 200 status code, where v3 previously responded with a 400. In order to not break current behavior (potentially breaking users who have creatively worked around this issue) and offer a mitigation, we've added the following configuration option which we recommend for all users. ```ts new ApolloServer({ // ... status400ForVariableCoercionErrors: true, }); ``` Specifically, this regression affects cases where _input variable coercion_ fails. Variables of an incorrect type (i.e. `String` instead of `Int`) or unexpectedly `null` are examples that fail variable coercion. Additionally, missing or incorrect fields on input objects as well as custom scalars that throw during validation will also fail variable coercion. For more specifics on variable coercion, see the "Input Coercion" sections in the [GraphQL spec](https://spec.graphql.org/June2018/#sec-Scalars). This will become the default behavior in Apollo Server v5 and the configuration option will be ignored / no longer needed. ### Patch Changes - [#7454](#7454) [`f6e3ae021`](f6e3ae0) Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Start building packages with TS 5.x, which should have no effect for users - [#7433](#7433) [`e0db95b96`](e0db95b) Thanks [@KGAdamCook](https://github.com/KGAdamCook)! - Previously, when users provided their own `documentStore`, Apollo Server used a random prefix per schema in order to guarantee there was no shared state from one schema to the next. Now Apollo Server uses a hash of the schema, which enables the provided document store to be shared if you choose to do so. ## @apollo/server-integration-testsuite@4.6.0 ### Patch Changes - [#7454](#7454) [`f6e3ae021`](f6e3ae0) Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Start building packages with TS 5.x, which should have no effect for users - Updated dependencies \[[`1e808146a`](1e80814), [`f6e3ae021`](f6e3ae0), [`e0db95b96`](e0db95b)]: - @apollo/server@4.6.0 ## @apollo/server-plugin-response-cache@4.1.2 ### Patch Changes - [#7454](#7454) [`f6e3ae021`](f6e3ae0) Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Start building packages with TS 5.x, which should have no effect for users Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
|
@jessemyers this is released in v4.6.0 via the |
trevor-scheer
pushed a commit
that referenced
this pull request
Apr 17, 2023
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@4.6.0 ### Minor Changes - [#7465](#7465) [`1e808146a`](1e80814) Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Introduce new opt-in configuration option to mitigate v4 status code regression Apollo Server v4 accidentally started responding to requests with an invalid `variables` object with a 200 status code, where v3 previously responded with a 400. In order to not break current behavior (potentially breaking users who have creatively worked around this issue) and offer a mitigation, we've added the following configuration option which we recommend for all users. ```ts new ApolloServer({ // ... status400ForVariableCoercionErrors: true, }); ``` Specifically, this regression affects cases where _input variable coercion_ fails. Variables of an incorrect type (i.e. `String` instead of `Int`) or unexpectedly `null` are examples that fail variable coercion. Additionally, missing or incorrect fields on input objects as well as custom scalars that throw during validation will also fail variable coercion. For more specifics on variable coercion, see the "Input Coercion" sections in the [GraphQL spec](https://spec.graphql.org/June2018/#sec-Scalars). This will become the default behavior in Apollo Server v5 and the configuration option will be ignored / no longer needed. ### Patch Changes - [#7454](#7454) [`f6e3ae021`](f6e3ae0) Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Start building packages with TS 5.x, which should have no effect for users - [#7433](#7433) [`e0db95b96`](e0db95b) Thanks [@KGAdamCook](https://github.com/KGAdamCook)! - Previously, when users provided their own `documentStore`, Apollo Server used a random prefix per schema in order to guarantee there was no shared state from one schema to the next. Now Apollo Server uses a hash of the schema, which enables the provided document store to be shared if you choose to do so. ## @apollo/server-integration-testsuite@4.6.0 ### Patch Changes - [#7454](#7454) [`f6e3ae021`](f6e3ae0) Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Start building packages with TS 5.x, which should have no effect for users - Updated dependencies \[[`1e808146a`](1e80814), [`f6e3ae021`](f6e3ae0), [`e0db95b96`](e0db95b)]: - @apollo/server@4.6.0 ## @apollo/server-plugin-response-cache@4.1.2 ### Patch Changes - [#7454](#7454) [`f6e3ae021`](f6e3ae0) Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Start building packages with TS 5.x, which should have no effect for users Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.
Apollo Server v4 introduced a regression with respect to invalid
variablesand http status codes. AS4 incorrectly started responding with a 200 status code, where AS3 would respond with a 400 when the provided variables object failed variable coercion (duringgraphql-jsexecute).Providing the following config to your AS4 constructor options will opt-in to the regression mitigation:
Fixes #7462
Related discussion #7460
TODO: