Skip to content

feat: pass abortSignal to resolvers via GraphQLResolveInfo #4425

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

Merged
merged 3 commits into from
Jun 10, 2025

Conversation

glasser
Copy link
Contributor

@glasser glasser commented Jun 3, 2025

In #4261 (not yet released in v17) we made abortSignal available to
resolvers via a fifth argument to the field resolver. Among other
things, this means that any code that processes schemas to wrap
resolvers in other functions would have to be aware of this one new
feature and specially thread through the new behavior. It also changed
the TypeScript signature of GraphQLFieldResolver to require passing
the fifth argument (even if undefined).

But the field resolver interface already has a place for GraphQL-JS to
put a grab-bag of helpful named objects for use by resolvers:
GraphQLResolveInfo.

This PR (which is not backwards compatible with v17.0.0-alpha.8, but is
backwards-compatible with v16) moves the abortSignal into
GraphQLResolveInfo.

It also improves the test of this feature to actually make use of the
AbortSignal API (the previous test actually passes when this change
is made, without changing the test to find the AbortSignal in the new
location).

glasser added 2 commits June 3, 2025 15:11
The previous test didn't really make use of the fact that the
AbortSignal could be used in the resolver: the only use it made of it
was to call `signal.throwIfAborted` *after* the cancellable promise was
already cancelled. The "This operation was aborted" message that shows
up in the GraphQL response actually came from the cancellable promise,
not the throwIfAborted call. You can see that because if you just
replace `throwIfAborted` with throwing another error (as this commit
does), the test still passed.

Instead, actually make use of the AbortSignal API to observe the abort
explicitly.
In graphql#4261 (not yet released in v17) we made abortSignal available to
resolvers via a fifth argument to the field resolver. Among other
things, this means that any code that processes schemas to wrap
resolvers in other functions would have to be aware of this one new
feature and specially thread through the new behavior. It also changed
the TypeScript signature of GraphQLFieldResolver to *require* passing
the fifth argument (even if undefined).

But the field resolver interface already has a place for GraphQL-JS to
put a grab-bag of helpful named objects for use by resolvers:
`GraphQLResolveInfo`.

This PR (which is not backwards compatible with v17.0.0-alpha.8, but is
backwards-compatible with v16) moves the abortSignal into
`GraphQLResolveInfo`.
@glasser
Copy link
Contributor Author

glasser commented Jun 4, 2025

Self-centeredly, I'd love to see an alpha.9 get released after this is merged, as this is, I believe, the only change needed to make the current version of Apollo Server (which is tested against v16 and v17.0.0-alpha.2) also compile against the more recent v17.

glasser added a commit to apollographql/apollo-server that referenced this pull request Jun 4, 2025
AS4 is tested against v16 and v17.0.0-alpha.2.

They've made it to v17.0.0-alpha.8 by now. We don't build against that
alpha for reasons that are fixed by
graphql/graphql-js#4425.

This commit is based on installing a build of that PR (downloaded from
its GH action artifact `npmDist`, unzipped, and `npm pack`'d') and
trying to get tests to pass. These tests should pass with v16,
v17.0.0-alpha.2, and the #4425 custom build.

Incremental delivery tests do NOT pass with the custom build, because
the incremental delivery protocol has changed and we haven't updated
yet.

Smoke test also doesn't pass yet for at least the reason that we haven't
asked it to install the right prerelease.

Changes include:
- Both AS itself and integration tests depend on the particular wording
  of some error messages, which have changed. (Once we can drop v16
  support, AS proper won't need that hack any more, but we're not there
  yet.)
- For some reason we tested the exact size of the JSON serialization of
  a GraphQL-JS AST in documentStore.test.ts.
- One subscriptionCallback debug log line was occurred one tick later
  with the newer module. I added `await setImmediate` so that it now
  *consistently* shows up in that slightly later spot.
@yaacovCR yaacovCR merged commit fba352d into graphql:next Jun 10, 2025
16 checks passed
@yaacovCR yaacovCR added PR: feature 🚀 requires increase of "minor" version number PR: breaking change 💥 implementation requires increase of "major" version number and removed PR: feature 🚀 requires increase of "minor" version number labels Jun 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: breaking change 💥 implementation requires increase of "major" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants