Repository navigation
feat: support GraphQL subscriptions - #2763
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds GraphQL WebSocket subscription interception using ChangesGraphQL subscription interception
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MSW
participant SubscriptionHandler
participant OriginalServer
Client->>MSW: Open GraphQL WebSocket subscription
MSW->>SubscriptionHandler: Match and resolve operation
SubscriptionHandler->>Client: Publish subscription payload
SubscriptionHandler->>OriginalServer: Forward through passthrough
OriginalServer->>MSW: Return next, error, or complete
MSW->>Client: Forward or transform server event
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/graphql/graphql-subscription.ts`:
- Around line 321-369: Update finalize and `#endSubscription` to handle
MaybePromise cleanup results: await each cleanup sequentially in LIFO order and
catch/report any rejection so failures do not become unhandled. Propagate the
necessary async behavior through `#endAllSubscriptions` and callers while
preserving immediate cleanup for missing subscriptions and the existing
at-most-once semantics.
- Around line 512-517: Update the client close handler around
`#endAllSubscriptions` to remove transportConnection from every participating
transport’s `#connections` registry, not just the module-level connections map.
Preserve subscription cleanup and the existing connections.delete(client.id)
behavior while ensuring no transport retains the closed connection.
- Around line 555-558: Update the connection_init handling in the message switch
to preserve the incoming message payload when acknowledging it locally, then
update the passthrough logic to send that stored payload upstream instead of
constructing a blank init frame. Ensure connectionParams and other
initialization context are replayed unchanged.
- Around line 584-606: Update the subscription document resolution in the
message handling flow to select the operation named by
message.payload.operationName when the query contains multiple operations,
rather than always using the first parsed operation. Resolve the requested
operation before the OperationTypeNode.SUBSCRIPTION check and retain the
existing invalid-document and unsupported-operation handling.
- Around line 952-961: Update the passthrough termination handling in
GraphQLPassthroughSubscription creation so it does not close the shared
connection.server. Preserve connection-level ownership of the upstream session
and have onTerminate end only this subscription by completing the matching
subscriptionId through the transport.
In `@src/graphql/graphql.ts`:
- Around line 86-96: Update the subscription handling around the operation
resolver callback so it no longer casts the reduced object to Parameters<typeof
resolver>[0]. Make the resolver contract explicitly support the
subscription-specific reduced info shape, or construct and pass all fields
required by the GraphQLResponseResolver contract, including request and
finalize; preserve the existing operationName, query, and variables values.
- Around line 84-98: Configure the subscription sibling created by
subscriptionFactory with the once option when the operation is registered with
once enabled. Ensure graphql.link(url).operation(resolver, { once: true })
consumes the subscription handler after its first match, rather than relying on
the HTTP handler’s options.
In `@test/support/graphqlServer.ts`:
- Around line 14-30: Use the existing pathname value in the graphql server
setup: pass pathname to createYoga, register the HTTP route with pathname
instead of the hardcoded /graphql path, and configure createWebSocketMiddleware
with that same value so all endpoints match http.url().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e7272eea-0d58-45c4-87ec-320e5a4a99a2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
package.jsonsrc/core/experimental/define-network.tssrc/core/experimental/frames/websocket-frame.tssrc/core/experimental/handlers-controller.test.tssrc/core/experimental/handlers-controller.tssrc/core/experimental/setup-api.tssrc/core/handlers/RequestHandler.tssrc/core/handlers/WebSocketHandler.tssrc/core/utils/internal/attachSiblingHandlers.tssrc/core/ws.tssrc/graphql/graphql-handler.tssrc/graphql/graphql-subscription.test.tssrc/graphql/graphql-subscription.tssrc/graphql/graphql.tssrc/graphql/index.tstest/browser/graphql-api/subscription.mocks.tstest/browser/graphql-api/subscription.test.tstest/node/graphql-api/events.test.tstest/node/graphql-api/graphql-subscription.test.tstest/node/msw-api/finalize.test.tstest/support/graphqlClient.tstest/support/graphqlServer.tstest/typings/graphql.test-d.ts
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/graphql/parse-graphql-request.ts (1)
59-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unnecessary
asyncfromparseQuery.The
parseQueryfunction does not perform any asynchronous operations (bothparseandparseDocumentNodeare synchronous). Consider removing theasynckeyword here and the correspondingawaitin theparseGraphQLRequestcaller (line 210) to slightly reduce execution overhead.♻️ Proposed refactor
-async function parseQuery( +function parseQuery( query: string, operationName?: string | null, -): Promise<ParsedGraphQLQuery | Error> { +): ParsedGraphQLQuery | Error { try { const ast = parse(query) return parseDocumentNode(ast, operationName)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/graphql/parse-graphql-request.ts` around lines 59 - 69, Remove the unnecessary async behavior from parseQuery and update its parseGraphQLRequest caller to stop awaiting the synchronous result. Preserve the existing Promise<ParsedGraphQLQuery | Error> contract and error handling while eliminating the redundant await at the call site.test/node/msw-api/finalize.test.ts (1)
365-408: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrevent global event listener leakage on test failure.
If an expectation or test logic throws between
process.onandprocess.off, theunhandledRejectionlistener will remain attached, potentially interfering with subsequent tests. Wrap the test body in atry...finallyblock to guarantee cleanup.🛡️ Proposed refactor
const unhandledRejectionListener = vi.fn() process.on('unhandledRejection', unhandledRejectionListener) - const secondCleanup = vi.fn() - - const api = graphql.link('http://localhost:4000/graphql') - server.use( - api.subscription('OnCommentAdded', ({ subscription, finalize }) => { - finalize(secondCleanup) - finalize(async () => { - throw new Error('Cleanup error') - }) - queueMicrotask(() => { - subscription.complete() - }) - }), - ) - - await using client = createClient({ - url: 'ws://localhost:4000/graphql', - lazy: false, - }) - const subscription = client.iterate({ - query: gql` - subscription OnCommentAdded { - commentAdded { - text - } - } - `, - }) - await subscription.next() - - await expect - .poll(() => vi.mocked(console.error).mock.calls.flat().join('\n')) - .toMatch(/Failed to execute the cleanup for a GraphQL subscription/) - - // A failing cleanup must not prevent the remaining cleanups from running. - expect(secondCleanup).toHaveBeenCalledOnce() - - process.off('unhandledRejection', unhandledRejectionListener) - expect(unhandledRejectionListener).not.toHaveBeenCalled() + try { + const secondCleanup = vi.fn() + + const api = graphql.link('http://localhost:4000/graphql') + server.use( + api.subscription('OnCommentAdded', ({ subscription, finalize }) => { + finalize(secondCleanup) + finalize(async () => { + throw new Error('Cleanup error') + }) + queueMicrotask(() => { + subscription.complete() + }) + }), + ) + + await using client = createClient({ + url: 'ws://localhost:4000/graphql', + lazy: false, + }) + const subscription = client.iterate({ + query: gql` + subscription OnCommentAdded { + commentAdded { + text + } + } + `, + }) + await subscription.next() + + await expect + .poll(() => vi.mocked(console.error).mock.calls.flat().join('\n')) + .toMatch(/Failed to execute the cleanup for a GraphQL subscription/) + + // A failing cleanup must not prevent the remaining cleanups from running. + expect(secondCleanup).toHaveBeenCalledOnce() + expect(unhandledRejectionListener).not.toHaveBeenCalled() + } finally { + process.off('unhandledRejection', unhandledRejectionListener) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/node/msw-api/finalize.test.ts` around lines 365 - 408, Wrap the test body after registering unhandledRejectionListener in a try...finally block, and move process.off('unhandledRejection', unhandledRejectionListener) into the finally clause. Keep the existing assertions and cleanup behavior unchanged while guaranteeing listener removal if any expectation or test operation throws.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/experimental/define-network.ts`:
- Around line 244-248: Update the disposal return logic in the network disable
flow to use Promise.all when both handlersDisposal and sourcesDisposal are
promises, ensuring both rejections are observed while preserving handler
disposal before source disposal. Keep the synchronous sourcesDisposal path
unchanged and retain the existing MaybePromise return contract.
In `@src/graphql/parse-graphql-request.ts`:
- Around line 47-57: Remove the first-operation fallback from the operation
lookup in parseGraphQLRequest, preserving the explicitly requested operationName
while returning an undefined operationType when no matching definition exists.
Update src/graphql/parse-graphql-request.test.ts lines 131-136 to assert this
unknown-name behavior instead of expecting the first operation.
---
Nitpick comments:
In `@src/graphql/parse-graphql-request.ts`:
- Around line 59-69: Remove the unnecessary async behavior from parseQuery and
update its parseGraphQLRequest caller to stop awaiting the synchronous result.
Preserve the existing Promise<ParsedGraphQLQuery | Error> contract and error
handling while eliminating the redundant await at the call site.
In `@test/node/msw-api/finalize.test.ts`:
- Around line 365-408: Wrap the test body after registering
unhandledRejectionListener in a try...finally block, and move
process.off('unhandledRejection', unhandledRejectionListener) into the finally
clause. Keep the existing assertions and cleanup behavior unchanged while
guaranteeing listener removal if any expectation or test operation throws.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fbe77c98-02fd-423f-b3e4-1be97fd91db3
📒 Files selected for processing (14)
src/core/experimental/define-network.tssrc/core/experimental/handlers-controller.tssrc/core/handlers/Handler.tssrc/core/handlers/RequestHandler.tssrc/core/handlers/WebSocketHandler.tssrc/graphql/graphql-subscription.tssrc/graphql/graphql.tssrc/graphql/index.tssrc/graphql/parse-graphql-request.test.tssrc/graphql/parse-graphql-request.tstest/node/graphql-api/graphql-subscription.test.tstest/node/graphql-api/multi-operation-document.test.tstest/node/msw-api/finalize.test.tstest/support/graphqlServer.ts
💤 Files with no reviewable changes (1)
- src/graphql/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/support/graphqlServer.ts
- src/graphql/graphql.ts
- test/node/graphql-api/graphql-subscription.test.ts
- src/graphql/graphql-subscription.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/experimental/define-network.test.ts`:
- Around line 130-149: Wrap the network setup, enable/disable assertions, and
delayed rejection check in the test’s try block, and move
process.off('unhandledRejection', unhandledRejectionListener) into a finally
block. Ensure listener cleanup runs whether network.disable() resolves
unexpectedly, rejects as expected, or any preceding assertion fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a63f65f4-f82b-4451-93c5-8364f361091b
📒 Files selected for processing (5)
src/core/experimental/define-network.test.tssrc/core/experimental/define-network.tssrc/graphql/parse-graphql-request.test.tssrc/graphql/parse-graphql-request.tstest/node/graphql-api/multi-operation-document.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/core/experimental/define-network.ts
- src/graphql/parse-graphql-request.ts
##### [v3.0.2](https://github.com/mswjs/msw/releases/tag/v3.0.2) #### v3.0.2 (2026-10-03) ##### Bug Fixes - **vite:** support `virtual:msw` imports in vitest ([#2802](mswjs/msw#2802)) ([`e73c381`](mswjs/msw@e73c381)) [@kettanaito](https://github.com/kettanaito) ##### [v3.0.1](https://github.com/mswjs/msw/releases/tag/v3.0.1) #### v3.0.1 (2026-09-30) ##### Bug Fixes - treat `CONNECT` requests as the transport mechanism ([#2796](mswjs/msw#2796)) ([`7cad86a`](mswjs/msw@7cad86a)) [@kettanaito](https://github.com/kettanaito) ##### [v3.0.0](https://github.com/mswjs/msw/releases/tag/v3.0.0) #### v3.0.0 (2026-09-28) - v3.0.0 ([#2692](mswjs/msw#2692)) ([`2bb4342`](mswjs/msw@2bb4342)) [@kettanaito](https://github.com/kettanaito) [@Andarist](https://github.com/Andarist) #####⚠️ BREAKING CHANGES - **MSW is now ESM-only** ([#2765](mswjs/msw#2765)). - Deprecate support for Node.js v18 and v20 ([#2729](mswjs/msw#2729)). The minimal supported Node.js version is v22. - `msw/native` is removed in favor of [`@msw/react-native`](https://github.com/mswjs/react-native). - Require TypeScript 5.9 as the minimal version. - Deprecate support for TypeScript 5.1 and 5.2 ([#2730](mswjs/msw#2730)) - `worker.stop()` returns a Promise ([#2724](mswjs/msw#2724)). Awaiting it is relevant only for in-flight critical scenarios. - `graphql` is now exported as `msw/graphql` ([#2691](mswjs/msw#2691)). GraphQL is now handled as a proper optional peer dependency. - Removes the `handleRequest()` utility. Please use the `defineNetwork()` API instead. - `GraphQLLinkHandlers` type has been renamed to `GraphQLLink`. - The `file://` requests now throw in Node.js as they normally do (not supported by `fetch`). - The `Content-Length` request header is now removed by `bypass()`. - HTTP-to-WebSocket upgrade requests performed via `fetch()` now throw in Node.js as they normally do (not supported by `fetch`). - Cookies are no longer managed in Node.js for better compatibilty. Instead, whatever is the default behavior of your environment that's the cookie behavior you get. This has no effect on the browser. - WebSocket `connection` life cycle event has been renamed to `websocket:connection` ([#2783](mswjs/msw#2783)). - `onUnhandledRequest` option has been renamed to `onUnhandledFrame`. - The library no longer patches `setTimeout` to circumvent fake timers in test runners. You should advance the fake timers normally for delayed mocked responses to resolve. - The library no longer returns a mocked cookie string on `request.headers.get('cookie')` in the handlers. It returns `null` in all environments. Please use the `cookies` resolver argument to have cookie-based logic. ##### Features - Supports Node.js v24 and v26. - Supports TypeScript v6.0 and v7.0. - You can import individual utilities, like `delay` or `passthrough`, from the new `msw/utils` entrypoint for smaller footprint on your bundle. - Support GraphQL v17.0. - Support intercepting page navigations and form submissions ([#2721](mswjs/msw#2721)). - Support GraphQL subscriptions ([#285](mswjs/msw#285), [#2763](mswjs/msw#2763)). - Migrate from `path-to-regexp` to `@msw/url` ([#2678](mswjs/msw#2678)). - Remove the following dependencies: - `graphql` - `path-to-regexp` - `picocolors` - `statuses` - `strict-event-emitter` - Adds a Vite plugin for MSW ([#2781](mswjs/msw#2781)). - Adds a `websocket:error` life cycle event to listen to WebSocket connection errors ([#2784](mswjs/msw#2784)). - Support modifying the intercepted request headers in passthrough scenarios ([#2786](mswjs/msw#2786)). - Support a new WebSocket Extension API that allows for better custom experiences ([#2791](mswjs/msw#2791)). ##### Bug fixes - Fix an issue where preventing a page closure via the `beforeunload` listener would unregister the MSW worker ([#1105](mswjs/msw#1105)). - Fix an issue that caused intermittent test failures in Vitest Browser Mode and Firefox if using `maxWorkers: 1` ([#2726](mswjs/msw#2726)). - Fix an issue where in-flight requests were incorrectly passthrough when performed before calling `worker.stop()` ([#2724](mswjs/msw#2724), [#2597](mswjs/msw#2597)). - Fix an issue where persisting cookies in `localStorage` would cause a maximum quota exceeded errors. - Fix various issues related to the network interception in Node.js.
##### [v3.0.3](https://github.com/mswjs/msw/releases/tag/v3.0.3) #### v3.0.3 (2026-10-10) ##### Bug Fixes - **HandlersController:** dispose of reset handlers ([#2810](mswjs/msw#2810)) ([`3818a81`](mswjs/msw@3818a81)) [@kettanaito](https://github.com/kettanaito) - expose network context on `enable` in network sources ([#2809](mswjs/msw#2809)) ([`d1fe8e6`](mswjs/msw@d1fe8e6)) [@kettanaito](https://github.com/kettanaito) ##### [v3.0.2](https://github.com/mswjs/msw/releases/tag/v3.0.2) #### v3.0.2 (2026-10-03) ##### Bug Fixes - **vite:** support `virtual:msw` imports in vitest ([#2802](mswjs/msw#2802)) ([`e73c381`](mswjs/msw@e73c381)) [@kettanaito](https://github.com/kettanaito) ##### [v3.0.1](https://github.com/mswjs/msw/releases/tag/v3.0.1) #### v3.0.1 (2026-09-30) ##### Bug Fixes - treat `CONNECT` requests as the transport mechanism ([#2796](mswjs/msw#2796)) ([`7cad86a`](mswjs/msw@7cad86a)) [@kettanaito](https://github.com/kettanaito) ##### [v3.0.0](https://github.com/mswjs/msw/releases/tag/v3.0.0) #### v3.0.0 (2026-09-28) - v3.0.0 ([#2692](mswjs/msw#2692)) ([`2bb4342`](mswjs/msw@2bb4342)) [@kettanaito](https://github.com/kettanaito) [@Andarist](https://github.com/Andarist) #####⚠️ BREAKING CHANGES - **MSW is now ESM-only** ([#2765](mswjs/msw#2765)). - Deprecate support for Node.js v18 and v20 ([#2729](mswjs/msw#2729)). The minimal supported Node.js version is v22. - `msw/native` is removed in favor of [`@msw/react-native`](https://github.com/mswjs/react-native). - Require TypeScript 5.9 as the minimal version. - Deprecate support for TypeScript 5.1 and 5.2 ([#2730](mswjs/msw#2730)) - `worker.stop()` returns a Promise ([#2724](mswjs/msw#2724)). Awaiting it is relevant only for in-flight critical scenarios. - `graphql` is now exported as `msw/graphql` ([#2691](mswjs/msw#2691)). GraphQL is now handled as a proper optional peer dependency. - Removes the `handleRequest()` utility. Please use the `defineNetwork()` API instead. - `GraphQLLinkHandlers` type has been renamed to `GraphQLLink`. - The `file://` requests now throw in Node.js as they normally do (not supported by `fetch`). - The `Content-Length` request header is now removed by `bypass()`. - HTTP-to-WebSocket upgrade requests performed via `fetch()` now throw in Node.js as they normally do (not supported by `fetch`). - Cookies are no longer managed in Node.js for better compatibility. Instead, whatever is the default behavior of your environment that's the cookie behavior you get. This has no effect on the browser. - WebSocket `connection` life cycle event has been renamed to `websocket:connection` ([#2783](mswjs/msw#2783)). - `onUnhandledRequest` option has been renamed to `onUnhandledFrame`. - The library no longer patches `setTimeout` to circumvent fake timers in test runners. You should advance the fake timers normally for delayed mocked responses to resolve. - The library no longer returns a mocked cookie string on `request.headers.get('cookie')` in the handlers. It returns `null` in all environments. Please use the `cookies` resolver argument to have cookie-based logic. ##### Features - Supports Node.js v24 and v26. - Supports TypeScript v6.0 and v7.0. - You can import individual utilities, like `delay` or `passthrough`, from the new `msw/utils` entrypoint for smaller footprint on your bundle. - Support GraphQL v17.0. - Support intercepting page navigations and form submissions ([#2721](mswjs/msw#2721)). - Support GraphQL subscriptions ([#285](mswjs/msw#285), [#2763](mswjs/msw#2763)). - Migrate from `path-to-regexp` to `@msw/url` ([#2678](mswjs/msw#2678)). - Remove the following dependencies: - `graphql` - `path-to-regexp` - `picocolors` - `statuses` - `strict-event-emitter` - Adds a Vite plugin for MSW ([#2781](mswjs/msw#2781)). - Adds a `websocket:error` life cycle event to listen to WebSocket connection errors ([#2784](mswjs/msw#2784)). - Support modifying the intercepted request headers in passthrough scenarios ([#2786](mswjs/msw#2786)). - Support a new WebSocket Extension API that allows for better custom experiences ([#2791](mswjs/msw#2791)). ##### Bug fixes - Fix an issue where preventing a page closure via the `beforeunload` listener would unregister the MSW worker ([#1105](mswjs/msw#1105)). - Fix an issue that caused intermittent test failures in Vitest Browser Mode and Firefox if using `maxWorkers: 1` ([#2726](mswjs/msw#2726)). - Fix an issue where in-flight requests were incorrectly passthrough when performed before calling `worker.stop()` ([#2724](mswjs/msw#2724), [#2597](mswjs/msw#2597)). - Fix an issue where persisting cookies in `localStorage` would cause a maximum quota exceeded errors. - Fix various issues related to the network interception in Node.js.
API
Changes
graphql.subscription().Handlerfor common methods likereset().dispose()common handler method to react to whennetwork.disable()is called.Roadmap
graphql.subscription()API.HandlersController.graphql.operation()also captures subscriptions.finalize()fires when the subscription is complete.connectionevents fire for a single subscription. Must have to do with the transport layering.Variablestype argument presence ongraphql.subscription().