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

(AS4 + #6398): Usage reporting: don't throw errors if willResolveField is called "late" #7747

Merged
merged 5 commits into from
Oct 4, 2023

Conversation

trevor-scheer
Copy link
Member

Porting #6398 from AS3. This fix was unintentionally left out of AS4.
Fixes #7746

As mentioned in #7746 (comment), this issue only surfaces due to a bug in graphql-js@<=16.6.0. Upgrading your graphql-js version will also prevent this issue from occurring.

@netlify
Copy link

netlify bot commented Oct 4, 2023

Deploy Preview for apollo-server-docs ready!

Name Link
🔨 Latest commit 5750d33
🔍 Latest deploy log https://app.netlify.com/sites/apollo-server-docs/deploys/651da2fed6ce620008b2edfd
😎 Deploy Preview https://deploy-preview-7747--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 configuration.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 4, 2023

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 5750d33:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

Copy link
Member

@glasser glasser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also an allusion to the minimum version number in the migration docs page — perhaps we should expand it to something like "AS4 technically requires graphql 16.6 but we strongly encourage the use of at least 16.7 to avoid a crashing bug"?

.changeset/late-coats-fry.md Outdated Show resolved Hide resolved
@trevor-scheer trevor-scheer requested a review from a team as a code owner October 4, 2023 17:35
@@ -379,7 +379,7 @@ If you're using Node.js 12, upgrade your runtime before upgrading to Apollo Serv

### `graphql`

Apollo Server has a peer dependency on [`graphql`](https://www.npmjs.com/package/graphql) (the core JS GraphQL implementation). Apollo Server 4 supports `graphql` v16.6.0 and later. (Apollo Server 3 supports `graphql` v15.3.0 through v16.)
Apollo Server has a peer dependency on [`graphql`](https://www.npmjs.com/package/graphql) (the core JS GraphQL implementation). Apollo Server 4 supports `graphql` v16.6.0 and later, but we *strongly* recommend using at least v16.7.0 due to a bug which can crash your server (Apollo Server 3 supports `graphql` v15.3.0 through v16.).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Apollo Server has a peer dependency on [`graphql`](https://www.npmjs.com/package/graphql) (the core JS GraphQL implementation). Apollo Server 4 supports `graphql` v16.6.0 and later, but we *strongly* recommend using at least v16.7.0 due to a bug which can crash your server (Apollo Server 3 supports `graphql` v15.3.0 through v16.).
Apollo Server has a peer dependency on [`graphql`](https://www.npmjs.com/package/graphql) (the core JS GraphQL implementation). Apollo Server 4 supports `graphql` v16.6.0 and later, but we *strongly* recommend using at least v16.7.0 due to [a bug which can crash your server](https://github.com/graphql/graphql-js/issues/3528). (Apollo Server 3 supports `graphql` v15.3.0 through v16.)

@trevor-scheer trevor-scheer merged commit ddce036 into main Oct 4, 2023
20 checks passed
@trevor-scheer trevor-scheer deleted the trevor/port-6398 branch October 4, 2023 17:47
@github-actions github-actions bot mentioned this pull request Oct 4, 2023
trevor-scheer pushed a commit that referenced this pull request Oct 4, 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-integration-testsuite@4.9.4

### Patch Changes

- Updated dependencies
\[[`ddce036e1`](ddce036)]:
    -   @apollo/server@4.9.4

## @apollo/server@4.9.4

### Patch Changes

- [#7747](#7747)
[`ddce036e1`](ddce036)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - The minimum
version of `graphql` officially supported by Apollo Server 4 as a peer
dependency, v16.6.0, contains a [serious bug that can crash your Node
server](graphql/graphql-js#3528). This bug is
fixed in the immediate next version, `graphql@16.7.0`, and we **strongly
encourage you to upgrade your installation of `graphql` to at least
v16.7.0** to avoid this bug. (For backwards compatibility reasons, we
cannot change Apollo Server 4's minimum peer dependency, but will change
it when we release Apollo Server 5.)

Apollo Server 4 contained a particular line of code that makes
triggering this crashing bug much more likely. This line was already
removed in Apollo Server v3.8.2 (see #6398) but the fix was accidentally
not included in Apollo Server 4. We are now including this change in
Apollo Server 4, which will **reduce** the likelihood of hitting this
crashing bug for users of `graphql` v16.6.0. That said, taking this
`@apollo/server` upgrade **does not prevent** this bug from being
triggered in other ways, and the real fix to this crashing bug is to
upgrade `graphql`.

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 Nov 4, 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.

AS4 contains code that makes triggering a crashing bug in graphql@16.6 (fixed in 16.7) more likely
2 participants