From 445017ebeeede622c41d730466dd1335521df89c Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 17 Jan 2025 00:14:05 -0700 Subject: [PATCH 01/27] Allow error link to handle protocol errors --- src/link/error/__tests__/index.ts | 121 ++++++++++++++++++++++++++++++ src/link/error/index.ts | 38 +++++++--- 2 files changed, 150 insertions(+), 9 deletions(-) diff --git a/src/link/error/__tests__/index.ts b/src/link/error/__tests__/index.ts index 0a3bf2bbfb8..38460e05a8e 100644 --- a/src/link/error/__tests__/index.ts +++ b/src/link/error/__tests__/index.ts @@ -6,6 +6,7 @@ import { ServerError, throwServerError } from "../../utils/throwServerError"; import { Observable } from "../../../utilities/observables/Observable"; import { onError, ErrorLink } from "../"; import { ObservableStream } from "../../../testing/internal"; +import { PROTOCOL_ERRORS_SYMBOL } from "../../../errors"; describe("error handling", () => { it("has an easy way to handle GraphQL errors", async () => { @@ -71,6 +72,66 @@ describe("error handling", () => { expect(called).toBe(true); }); + it("handles protocol errors", async () => { + expect.assertions(3); + const query = gql` + query Foo { + foo { + bar + } + } + `; + + const errorLink = onError(({ operation, protocolErrors }) => { + expect(operation.operationName).toBe("Foo"); + expect(protocolErrors).toEqual([ + { + message: "cannot read message from websocket", + extensions: [{ code: "WEBSOCKET_MESSAGE_ERROR" }], + }, + ]); + }); + + const mockLink = new ApolloLink((_operation) => { + return new Observable((observer) => { + observer.next({ + data: null, + extensions: { + [PROTOCOL_ERRORS_SYMBOL]: [ + { + message: "cannot read message from websocket", + extensions: [ + { + code: "WEBSOCKET_MESSAGE_ERROR", + }, + ], + }, + ], + }, + }); + }); + }); + + const link = errorLink.concat(mockLink); + const stream = new ObservableStream(execute(link, { query })); + + await expect(stream).toEmitValue({ + data: null, + extensions: { + [PROTOCOL_ERRORS_SYMBOL]: [ + { + message: "cannot read message from websocket", + extensions: [ + { + code: "WEBSOCKET_MESSAGE_ERROR", + }, + ], + }, + ], + }, + }); + }); + it("captures errors within links", async () => { const query = gql` query Foo { @@ -356,6 +417,66 @@ describe("error handling with class", () => { expect(called).toBe(true); }); + it("handles protocol errors", async () => { + expect.assertions(3); + const query = gql` + query Foo { + foo { + bar + } + } + `; + + const errorLink = new ErrorLink(({ operation, protocolErrors }) => { + expect(operation.operationName).toBe("Foo"); + expect(protocolErrors).toEqual([ + { + message: "cannot read message from websocket", + extensions: [{ code: "WEBSOCKET_MESSAGE_ERROR" }], + }, + ]); + }); + + const mockLink = new ApolloLink((_operation) => { + return new Observable((observer) => { + observer.next({ + data: null, + extensions: { + [PROTOCOL_ERRORS_SYMBOL]: [ + { + message: "cannot read message from websocket", + extensions: [ + { + code: "WEBSOCKET_MESSAGE_ERROR", + }, + ], + }, + ], + }, + }); + }); + }); + + const link = errorLink.concat(mockLink); + const stream = new ObservableStream(execute(link, { query })); + + await expect(stream).toEmitValue({ + data: null, + extensions: { + [PROTOCOL_ERRORS_SYMBOL]: [ + { + message: "cannot read message from websocket", + extensions: [ + { + code: "WEBSOCKET_MESSAGE_ERROR", + }, + ], + }, + ], + }, + }); + }); + it("captures errors within links", async () => { const query = gql` { diff --git a/src/link/error/index.ts b/src/link/error/index.ts index bf9494c5dfa..af8d9b173cd 100644 --- a/src/link/error/index.ts +++ b/src/link/error/index.ts @@ -1,5 +1,13 @@ -import type { FormattedExecutionResult, GraphQLFormattedError } from "graphql"; +import type { + FormattedExecutionResult, + GraphQLErrorExtensions, + GraphQLFormattedError, +} from "graphql"; +import { + graphQLResultHasProtocolErrors, + PROTOCOL_ERRORS_SYMBOL, +} from "../../errors/index.js"; import type { NetworkError } from "../../errors/index.js"; import { Observable } from "../../utilities/index.js"; import type { Operation, FetchResult, NextLink } from "../core/index.js"; @@ -8,6 +16,10 @@ import { ApolloLink } from "../core/index.js"; export interface ErrorResponse { graphQLErrors?: ReadonlyArray; networkError?: NetworkError; + protocolErrors?: ReadonlyArray<{ + message: string; + extensions?: GraphQLErrorExtensions[]; + }>; response?: FormattedExecutionResult; operation: Operation; forward: NextLink; @@ -42,16 +54,24 @@ export function onError(errorHandler: ErrorHandler): ApolloLink { operation, forward, }); + } else if (graphQLResultHasProtocolErrors(result)) { + retriedResult = errorHandler({ + protocolErrors: result.extensions[PROTOCOL_ERRORS_SYMBOL], + response: result, + operation, + forward, + }); + } - if (retriedResult) { - retriedSub = retriedResult.subscribe({ - next: observer.next.bind(observer), - error: observer.error.bind(observer), - complete: observer.complete.bind(observer), - }); - return; - } + if (retriedResult) { + retriedSub = retriedResult.subscribe({ + next: observer.next.bind(observer), + error: observer.error.bind(observer), + complete: observer.complete.bind(observer), + }); + return; } + observer.next(result); }, error: (networkError) => { From 7b4338c6705c1992e9bf4ae287bf19182e7fae51 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 17 Jan 2025 00:35:50 -0700 Subject: [PATCH 02/27] Add test to ensure retry can happen after protocol errors --- src/link/error/__tests__/index.ts | 51 +++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/link/error/__tests__/index.ts b/src/link/error/__tests__/index.ts index 38460e05a8e..ecf28a116cd 100644 --- a/src/link/error/__tests__/index.ts +++ b/src/link/error/__tests__/index.ts @@ -586,6 +586,18 @@ describe("support for request retrying", () => { message: "some other error", }; + const PROTOCOL_ERROR = { + data: null, + extensions: { + [PROTOCOL_ERRORS_SYMBOL]: [ + { + message: "cannot read message from websocket", + extensions: [{ code: "WEBSOCKET_MESSAGE_ERROR" }], + }, + ], + }, + }; + it("returns the retried request when forward(operation) is called", async () => { let errorHandlerCalled = false; @@ -671,6 +683,45 @@ describe("support for request retrying", () => { await expect(stream).toComplete(); }); + it("supports retrying when the initial request had protocol errors", async () => { + let errorHandlerCalled = false; + + let timesCalled = 0; + const mockHttpLink = new ApolloLink((operation) => { + return new Observable((observer) => { + // simulate the first request being an error + if (timesCalled === 0) { + timesCalled++; + observer.next(PROTOCOL_ERROR); + observer.complete(); + } else { + observer.next(GOOD_RESPONSE); + observer.complete(); + } + }); + }); + + const errorLink = new ErrorLink( + ({ protocolErrors, operation, forward }) => { + if (protocolErrors) { + errorHandlerCalled = true; + expect(protocolErrors).toEqual( + PROTOCOL_ERROR.extensions[PROTOCOL_ERRORS_SYMBOL] + ); + return forward(operation); + } + } + ); + + const link = errorLink.concat(mockHttpLink); + + const stream = new ObservableStream(execute(link, { query: QUERY })); + + await expect(stream).toEmitValue(GOOD_RESPONSE); + expect(errorHandlerCalled).toBe(true); + await expect(stream).toComplete(); + }); + it("returns errors from retried requests", async () => { let errorHandlerCalled = false; From e51ef3e4a3750343337382862e9559beb5fd0cb1 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 17 Jan 2025 00:37:59 -0700 Subject: [PATCH 03/27] Add changeset --- .changeset/giant-peas-lie.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .changeset/giant-peas-lie.md diff --git a/.changeset/giant-peas-lie.md b/.changeset/giant-peas-lie.md new file mode 100644 index 00000000000..9caaab73e33 --- /dev/null +++ b/.changeset/giant-peas-lie.md @@ -0,0 +1,13 @@ +--- +"@apollo/client": patch +--- + +Make `protocolErrors` from multipart subscriptions available to the error link with the `protocolErrors` property. + +```js +const errorLink = onError(({ protocolErrors }) => { + if (protocolErrors) { + console.log(protocolErrors); + } +}); +``` From 034dd47afab5408536255c08d18ada0149123fed Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Fri, 17 Jan 2025 00:46:17 -0700 Subject: [PATCH 04/27] Add protocolErrors to docs --- docs/source/api/link/apollo-link-error.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/source/api/link/apollo-link-error.md b/docs/source/api/link/apollo-link-error.md index 4b1052d8a85..5a646e81a67 100644 --- a/docs/source/api/link/apollo-link-error.md +++ b/docs/source/api/link/apollo-link-error.md @@ -11,13 +11,22 @@ Use the `onError` link to perform custom logic when a [GraphQL or network error] import { onError } from "@apollo/client/link/error"; // Log any GraphQL errors or network error that occurred -const errorLink = onError(({ graphQLErrors, networkError }) => { +const errorLink = onError(({ graphQLErrors, networkError, protocolErrors }) => { if (graphQLErrors) graphQLErrors.forEach(({ message, locations, path }) => console.log( `[GraphQL error]: Message: ${message}, Location: ${locations}, Path: ${path}` ) ); + + if (protocolErrors) { + protocolErrors.forEach(({ message, extensions }) => { + console.log( + `[Protocol error]: Message: ${message}, Extensions: ${JSON.stringify(extensions)}` + ); + }); + } + if (networkError) console.log(`[Network error]: ${networkError}`); }); ``` From e54cfdac0b71bdf3a7c1a97c18e5207b3f3da014 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 17 Jan 2025 14:45:25 +0100 Subject: [PATCH 05/27] add tests for incremental subscription & `@defer` with errorLink --- package-lock.json | 12 +++ package.json | 1 + src/link/core/types.ts | 2 +- src/link/error/__tests__/index.ts | 130 +++++++++++++++++++------ src/testing/internal/incremental.ts | 142 ++++++++++++++++++++++++++++ src/testing/internal/index.ts | 5 + 6 files changed, 260 insertions(+), 32 deletions(-) create mode 100644 src/testing/internal/incremental.ts diff --git a/package-lock.json b/package-lock.json index bb622e36c3e..9f6243c240b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -78,6 +78,7 @@ "fetch-mock": "9.11.0", "glob": "8.1.0", "graphql": "16.9.0", + "graphql-17-alpha2": "npm:graphql@17.0.0-alpha.2", "graphql-ws": "5.16.0", "jest": "29.7.0", "jest-environment-jsdom": "29.7.0", @@ -7795,6 +7796,17 @@ "node": "^12.22.0 || ^14.16.0 || ^16.0.0 || >=17.0.0" } }, + "node_modules/graphql-17-alpha2": { + "name": "graphql", + "version": "17.0.0-alpha.2", + "resolved": "https://registry.npmjs.org/graphql/-/graphql-17.0.0-alpha.2.tgz", + "integrity": "sha512-aRAd/BQ5hSO0+l7x+sHBfJVUp2JUOjPTE/iwJ3BhtYNH/MC7n4gjlZbKvnBVFZZAczyMS3vezS4teEZivoqIzw==", + "dev": true, + "license": "MIT", + "engines": { + "node": "^14.19.0 || ^16.10.0 || >=18.0.0" + } + }, "node_modules/graphql-tag": { "version": "2.12.6", "resolved": "https://registry.npmjs.org/graphql-tag/-/graphql-tag-2.12.6.tgz", diff --git a/package.json b/package.json index c4652499024..e8b5d4db9b4 100644 --- a/package.json +++ b/package.json @@ -161,6 +161,7 @@ "fetch-mock": "9.11.0", "glob": "8.1.0", "graphql": "16.9.0", + "graphql-17-alpha2": "npm:graphql@17.0.0-alpha.2", "graphql-ws": "5.16.0", "jest": "29.7.0", "jest-environment-jsdom": "29.7.0", diff --git a/src/link/core/types.ts b/src/link/core/types.ts index c596ecac0c2..766d6512db4 100644 --- a/src/link/core/types.ts +++ b/src/link/core/types.ts @@ -53,7 +53,7 @@ export interface ApolloPayloadResult< payload: SingleExecutionResult | ExecutionPatchResult | null; // Transport layer errors (as distinct from GraphQL or NetworkErrors), // these are fatal errors that will include done: true. - errors?: ReadonlyArray; + errors?: ReadonlyArray; } export type ExecutionPatchResult< diff --git a/src/link/error/__tests__/index.ts b/src/link/error/__tests__/index.ts index ecf28a116cd..11b5c8add9a 100644 --- a/src/link/error/__tests__/index.ts +++ b/src/link/error/__tests__/index.ts @@ -7,6 +7,10 @@ import { Observable } from "../../../utilities/observables/Observable"; import { onError, ErrorLink } from "../"; import { ObservableStream } from "../../../testing/internal"; import { PROTOCOL_ERRORS_SYMBOL } from "../../../errors"; +import { + mockDeferStream, + mockMultipartSubscriptionStream, +} from "../../../testing/internal/incremental"; describe("error handling", () => { it("has an easy way to handle GraphQL errors", async () => { @@ -72,12 +76,16 @@ describe("error handling", () => { expect(called).toBe(true); }); - it("handles protocol errors", async () => { - expect.assertions(3); + it.failing("handles protocol errors (@defer)", async () => { + // TODO: this test doesn't execute the `errorHandler` yet. Should be 4, is 2. + fail(); + expect.assertions(4); const query = gql` query Foo { foo { - bar + ... @defer { + bar + } } } `; @@ -86,46 +94,106 @@ describe("error handling", () => { expect(operation.operationName).toBe("Foo"); expect(protocolErrors).toEqual([ { - message: "cannot read message from websocket", - extensions: [{ code: "WEBSOCKET_MESSAGE_ERROR" }], + message: "could not read data", + extensions: { + code: "INCREMENTAL_ERROR", + }, }, ]); }); - const mockLink = new ApolloLink((_operation) => { - return new Observable((observer) => { - observer.next({ - data: null, - extensions: { - [PROTOCOL_ERRORS_SYMBOL]: [ - { - message: "cannot read message from websocket", - extensions: [ - { - code: "WEBSOCKET_MESSAGE_ERROR", - }, - ], + const { httpLink, enqueueInitialChunk, enqueueProtocolErrorChunk } = + mockDeferStream(); + const link = errorLink.concat(httpLink); + const stream = new ObservableStream(execute(link, { query })); + + enqueueInitialChunk({ + hasNext: true, + data: {}, + }); + + enqueueProtocolErrorChunk([ + { + message: "could not read data", + extensions: { + code: "INCREMENTAL_ERROR", + }, + }, + ]); + await expect(stream).toEmitValue({ + data: {}, + hasNext: true, + }); + + await expect(stream).toEmitValue({ + hasNext: true, + incremental: [ + { + errors: [ + { + message: "could not read data", + extensions: { + code: "INCREMENTAL_ERROR", }, - ], - }, - }); - }); + }, + ], + }, + ], }); + }); - const link = errorLink.concat(mockLink); - const stream = new ObservableStream(execute(link, { query })); + it("handles protocol errors (multipart subscription)", async () => { + expect.assertions(4); + const sampleSubscription = gql` + subscription MySubscription { + aNewDieWasCreated { + die { + roll + sides + color + } + } + } + `; + + const errorLink = onError((args) => { + const { operation, protocolErrors } = args; + expect(operation.operationName).toBe("MySubscription"); + expect(protocolErrors).toEqual([ + { + message: "Error field", + extensions: { code: "INTERNAL_SERVER_ERROR" }, + }, + ]); + }); + + const { httpLink, enqueuePayloadResult, enqueueErrorResult } = + mockMultipartSubscriptionStream(); + const link = errorLink.concat(httpLink); + const stream = new ObservableStream( + execute(link, { query: sampleSubscription }) + ); + + enqueuePayloadResult({ + data: { aNewDieWasCreated: { die: { color: "red", roll: 1, sides: 4 } } }, + }); + + enqueueErrorResult([ + { message: "Error field", extensions: { code: "INTERNAL_SERVER_ERROR" } }, + ]); + + await expect(stream).toEmitValue({ + data: { aNewDieWasCreated: { die: { color: "red", roll: 1, sides: 4 } } }, + }); await expect(stream).toEmitValue({ - data: null, extensions: { [PROTOCOL_ERRORS_SYMBOL]: [ { - message: "cannot read message from websocket", - extensions: [ - { - code: "WEBSOCKET_MESSAGE_ERROR", - }, - ], + extensions: { + code: "INTERNAL_SERVER_ERROR", + }, + message: "Error field", }, ], }, diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts new file mode 100644 index 00000000000..0f8759989a7 --- /dev/null +++ b/src/testing/internal/incremental.ts @@ -0,0 +1,142 @@ +import { HttpLink } from "../../link/http/index.js"; +import type { + GraphQLFormattedError, + InitialIncrementalExecutionResult, + SubsequentIncrementalExecutionResult, +} from "graphql-17-alpha2"; +import type { GraphQLError } from "graphql"; +import { + ReadableStream as NodeReadableStream, + TextEncoderStream, + TransformStream, +} from "node:stream/web"; +import type { ApolloPayloadResult } from "../../core/index.js"; + +const hasNextSymbol = Symbol("hasNext"); + +export function mockIncrementalStream({ + responseHeaders, +}: { + responseHeaders: Headers; +}) { + let streamController: ReadableStreamDefaultController< + Chunks & { [hasNextSymbol]: boolean } + >; + let sentInitialChunk = false; + const stream = new NodeReadableStream({ + start(c) { + streamController = c; + }, + }) + .pipeThrough( + new TransformStream({ + transform: (chunk, controller) => { + controller.enqueue( + (!sentInitialChunk ? "\r\n---\r\n" : "") + + "content-type: application/json; charset=utf-8\r\n\r\n" + + JSON.stringify(chunk) + + (chunk[hasNextSymbol] ? "\r\n---\r\n" : "\r\n-----\r\n") + ); + sentInitialChunk = true; + }, + }) + ) + .pipeThrough(new TextEncoderStream()); + + const httpLink = new HttpLink({ + fetch(input, init) { + return Promise.resolve( + new Response( + stream satisfies NodeReadableStream as ReadableStream, + { + status: 200, + headers: responseHeaders, + } + ) + ); + }, + }); + return { + httpLink, + streamController: streamController!, + }; +} + +export function mockDeferStream< + TData = Record, + TExtensions = Record, +>() { + const { httpLink, streamController } = mockIncrementalStream< + | InitialIncrementalExecutionResult + | SubsequentIncrementalExecutionResult + >({ + responseHeaders: new Headers({ + "Content-Type": 'multipart/mixed; boundary="-"; deferSpec=20220824', + }), + }); + return { + httpLink, + streamController: streamController!, + enqueueInitialChunk( + chunk: InitialIncrementalExecutionResult + ) { + streamController.enqueue({ ...chunk, [hasNextSymbol]: chunk.hasNext }); + if (!chunk.hasNext) streamController.close(); + }, + enqueueSubsequentChunk( + chunk: SubsequentIncrementalExecutionResult + ) { + streamController.enqueue({ ...chunk, [hasNextSymbol]: chunk.hasNext }); + if (!chunk.hasNext) streamController.close(); + }, + enqueueProtocolErrorChunk(errors: GraphQLFormattedError[]) { + streamController.enqueue({ + hasNext: true, + [hasNextSymbol]: true, + incremental: [ + { + // eslint-disable-next-line @typescript-eslint/ban-types + errors: errors as GraphQLError[], + }, + ], + } satisfies SubsequentIncrementalExecutionResult & { + [hasNextSymbol]: boolean; + }); + }, + }; +} + +export function mockMultipartSubscriptionStream< + TData = Record, + TExtensions = Record, +>() { + const { httpLink, streamController } = mockIncrementalStream< + ApolloPayloadResult + >({ + responseHeaders: new Headers({ + "Content-Type": "multipart/mixed", + }), + }); + + // send initial empty chunk back + streamController.enqueue({} as any); + + return { + httpLink, + streamController: streamController!, + enqueuePayloadResult( + payload: ApolloPayloadResult["payload"], + hasNext = true + ) { + streamController.enqueue({ payload, [hasNextSymbol]: hasNext }); + if (!hasNext) streamController.close(); + }, + enqueueErrorResult( + errors: ApolloPayloadResult["errors"], + payload: ApolloPayloadResult["payload"] = null + ) { + streamController.enqueue({ payload, errors, [hasNextSymbol]: false }); + streamController.close(); + }, + }; +} diff --git a/src/testing/internal/index.ts b/src/testing/internal/index.ts index cf38a14db79..2d422f519a9 100644 --- a/src/testing/internal/index.ts +++ b/src/testing/internal/index.ts @@ -28,3 +28,8 @@ export { export { actAsync } from "./rtl/actAsync.js"; export { renderAsync } from "./rtl/renderAsync.js"; export { renderHookAsync } from "./rtl/renderHookAsync.js"; +export { + mockIncrementalStream, + mockDeferStream, + mockMultipartSubscriptionStream, +} from "./incremental.js"; From 6bc334645285bd9eafcea729a2f6c496ef5a75f2 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 12:37:43 -0700 Subject: [PATCH 06/27] Add comments for each error type --- src/link/error/index.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/link/error/index.ts b/src/link/error/index.ts index af8d9b173cd..e77e7b27f8d 100644 --- a/src/link/error/index.ts +++ b/src/link/error/index.ts @@ -14,8 +14,20 @@ import type { Operation, FetchResult, NextLink } from "../core/index.js"; import { ApolloLink } from "../core/index.js"; export interface ErrorResponse { + /** + * Errors returned in the `errors` property of the GraphQL response. + */ graphQLErrors?: ReadonlyArray; + /** + * Errors thrown during a network request. This is usually an error thrown + * during a `fetch` call or an error while parsing the response from the + * network. + */ networkError?: NetworkError; + /** + * Fatal transport-level errors from multipart subscriptions. + * See the [multipart subscription protocol](https://www.apollographql.com/docs/graphos/routing/operations/subscriptions/multipart-protocol#message-and-error-format) for more information. + */ protocolErrors?: ReadonlyArray<{ message: string; extensions?: GraphQLErrorExtensions[]; From 23f4d7e82567809c9808674d820a05c9b974eda7 Mon Sep 17 00:00:00 2001 From: jerelmiller <565661+jerelmiller@users.noreply.github.com> Date: Tue, 21 Jan 2025 19:41:22 +0000 Subject: [PATCH 07/27] Clean up Prettier, Size-limit, and Api-Extractor --- .api-reports/api-report-core.api.md | 2 +- .api-reports/api-report-link_core.api.md | 2 +- .api-reports/api-report-link_error.api.md | 8 +++++--- .api-reports/api-report-utilities.api.md | 2 +- .api-reports/api-report.api.md | 2 +- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/.api-reports/api-report-core.api.md b/.api-reports/api-report-core.api.md index 75937521116..d02b35069d0 100644 --- a/.api-reports/api-report-core.api.md +++ b/.api-reports/api-report-core.api.md @@ -259,7 +259,7 @@ export class ApolloLink { // @public (undocumented) export interface ApolloPayloadResult, TExtensions = Record> { // (undocumented) - errors?: ReadonlyArray; + errors?: ReadonlyArray; // (undocumented) payload: SingleExecutionResult | ExecutionPatchResult | null; } diff --git a/.api-reports/api-report-link_core.api.md b/.api-reports/api-report-link_core.api.md index 32f6e56f608..1e3bdb755ba 100644 --- a/.api-reports/api-report-link_core.api.md +++ b/.api-reports/api-report-link_core.api.md @@ -43,7 +43,7 @@ export class ApolloLink { // @public (undocumented) export interface ApolloPayloadResult, TExtensions = Record> { // (undocumented) - errors?: ReadonlyArray; + errors?: ReadonlyArray; // (undocumented) payload: SingleExecutionResult | ExecutionPatchResult | null; } diff --git a/.api-reports/api-report-link_error.api.md b/.api-reports/api-report-link_error.api.md index 92cbae62eb5..bf6e3185840 100644 --- a/.api-reports/api-report-link_error.api.md +++ b/.api-reports/api-report-link_error.api.md @@ -6,6 +6,7 @@ import type { DocumentNode } from 'graphql'; import type { FormattedExecutionResult } from 'graphql'; +import type { GraphQLErrorExtensions } from 'graphql'; import type { GraphQLFormattedError } from 'graphql'; import { Observable } from 'zen-observable-ts'; import type { Observer } from 'zen-observable-ts'; @@ -80,14 +81,15 @@ export class ErrorLink extends ApolloLink { export interface ErrorResponse { // (undocumented) forward: NextLink; - // (undocumented) graphQLErrors?: ReadonlyArray; // Warning: (ae-forgotten-export) The symbol "NetworkError" needs to be exported by the entry point index.d.ts - // - // (undocumented) networkError?: NetworkError; // (undocumented) operation: Operation; + protocolErrors?: ReadonlyArray<{ + message: string; + extensions?: GraphQLErrorExtensions[]; + }>; // (undocumented) response?: FormattedExecutionResult; } diff --git a/.api-reports/api-report-utilities.api.md b/.api-reports/api-report-utilities.api.md index 6f944d1a205..7681702a523 100644 --- a/.api-reports/api-report-utilities.api.md +++ b/.api-reports/api-report-utilities.api.md @@ -313,7 +313,7 @@ class ApolloLink { // @public (undocumented) interface ApolloPayloadResult, TExtensions = Record> { // (undocumented) - errors?: ReadonlyArray; + errors?: ReadonlyArray; // Warning: (ae-forgotten-export) The symbol "SingleExecutionResult" needs to be exported by the entry point index.d.ts // Warning: (ae-forgotten-export) The symbol "ExecutionPatchResult" needs to be exported by the entry point index.d.ts // diff --git a/.api-reports/api-report.api.md b/.api-reports/api-report.api.md index f56f037b9df..2369d1d233a 100644 --- a/.api-reports/api-report.api.md +++ b/.api-reports/api-report.api.md @@ -282,7 +282,7 @@ export class ApolloLink { // @public (undocumented) export interface ApolloPayloadResult, TExtensions = Record> { // (undocumented) - errors?: ReadonlyArray; + errors?: ReadonlyArray; // (undocumented) payload: SingleExecutionResult | ExecutionPatchResult | null; } From 1fbf96c3b80293c88284851b03209d719c41de99 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 12:39:34 -0700 Subject: [PATCH 08/27] Remove string from union on ApolloPayloadResult --- src/link/core/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/link/core/types.ts b/src/link/core/types.ts index 766d6512db4..32fe69728fe 100644 --- a/src/link/core/types.ts +++ b/src/link/core/types.ts @@ -53,7 +53,7 @@ export interface ApolloPayloadResult< payload: SingleExecutionResult | ExecutionPatchResult | null; // Transport layer errors (as distinct from GraphQL or NetworkErrors), // these are fatal errors that will include done: true. - errors?: ReadonlyArray; + errors?: ReadonlyArray; } export type ExecutionPatchResult< From 7549224c99ca91b5d84799130d818d3720e63700 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 12:43:01 -0700 Subject: [PATCH 09/27] Add changeset for change to payload result --- .changeset/mighty-shoes-clap.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/mighty-shoes-clap.md diff --git a/.changeset/mighty-shoes-clap.md b/.changeset/mighty-shoes-clap.md new file mode 100644 index 00000000000..5c155848405 --- /dev/null +++ b/.changeset/mighty-shoes-clap.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Remove the `string` type from the array union for the `errors` field on the `ApolloPayloadResult` type. Errors were never plain strings and always in the GraphQL error format. From 5df6452a7a5917a992f74c89fdd9abdccea39473 Mon Sep 17 00:00:00 2001 From: jerelmiller <565661+jerelmiller@users.noreply.github.com> Date: Tue, 21 Jan 2025 19:45:38 +0000 Subject: [PATCH 10/27] Clean up Prettier, Size-limit, and Api-Extractor --- .api-reports/api-report-core.api.md | 2 +- .api-reports/api-report-link_core.api.md | 2 +- .api-reports/api-report-utilities.api.md | 2 +- .api-reports/api-report.api.md | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.api-reports/api-report-core.api.md b/.api-reports/api-report-core.api.md index d02b35069d0..241e1933484 100644 --- a/.api-reports/api-report-core.api.md +++ b/.api-reports/api-report-core.api.md @@ -259,7 +259,7 @@ export class ApolloLink { // @public (undocumented) export interface ApolloPayloadResult, TExtensions = Record> { // (undocumented) - errors?: ReadonlyArray; + errors?: ReadonlyArray; // (undocumented) payload: SingleExecutionResult | ExecutionPatchResult | null; } diff --git a/.api-reports/api-report-link_core.api.md b/.api-reports/api-report-link_core.api.md index 1e3bdb755ba..0171eeadfd8 100644 --- a/.api-reports/api-report-link_core.api.md +++ b/.api-reports/api-report-link_core.api.md @@ -43,7 +43,7 @@ export class ApolloLink { // @public (undocumented) export interface ApolloPayloadResult, TExtensions = Record> { // (undocumented) - errors?: ReadonlyArray; + errors?: ReadonlyArray; // (undocumented) payload: SingleExecutionResult | ExecutionPatchResult | null; } diff --git a/.api-reports/api-report-utilities.api.md b/.api-reports/api-report-utilities.api.md index 7681702a523..5397c853318 100644 --- a/.api-reports/api-report-utilities.api.md +++ b/.api-reports/api-report-utilities.api.md @@ -313,7 +313,7 @@ class ApolloLink { // @public (undocumented) interface ApolloPayloadResult, TExtensions = Record> { // (undocumented) - errors?: ReadonlyArray; + errors?: ReadonlyArray; // Warning: (ae-forgotten-export) The symbol "SingleExecutionResult" needs to be exported by the entry point index.d.ts // Warning: (ae-forgotten-export) The symbol "ExecutionPatchResult" needs to be exported by the entry point index.d.ts // diff --git a/.api-reports/api-report.api.md b/.api-reports/api-report.api.md index 2369d1d233a..68b3262ac60 100644 --- a/.api-reports/api-report.api.md +++ b/.api-reports/api-report.api.md @@ -282,7 +282,7 @@ export class ApolloLink { // @public (undocumented) export interface ApolloPayloadResult, TExtensions = Record> { // (undocumented) - errors?: ReadonlyArray; + errors?: ReadonlyArray; // (undocumented) payload: SingleExecutionResult | ExecutionPatchResult | null; } From 11f22f25acd57ca5da2c8b314f1b4b34510f814c Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 12:46:48 -0700 Subject: [PATCH 11/27] Pass TData and TExtensions in ApolloPayloadResult --- src/link/core/types.ts | 5 ++++- src/testing/internal/incremental.ts | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/link/core/types.ts b/src/link/core/types.ts index 32fe69728fe..22a83844a46 100644 --- a/src/link/core/types.ts +++ b/src/link/core/types.ts @@ -50,7 +50,10 @@ export interface ApolloPayloadResult< TData = Record, TExtensions = Record, > { - payload: SingleExecutionResult | ExecutionPatchResult | null; + payload: + | SingleExecutionResult + | ExecutionPatchResult + | null; // Transport layer errors (as distinct from GraphQL or NetworkErrors), // these are fatal errors that will include done: true. errors?: ReadonlyArray; diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts index 0f8759989a7..96bff3e713f 100644 --- a/src/testing/internal/incremental.ts +++ b/src/testing/internal/incremental.ts @@ -125,7 +125,7 @@ export function mockMultipartSubscriptionStream< httpLink, streamController: streamController!, enqueuePayloadResult( - payload: ApolloPayloadResult["payload"], + payload: ApolloPayloadResult["payload"], hasNext = true ) { streamController.enqueue({ payload, [hasNextSymbol]: hasNext }); @@ -133,7 +133,7 @@ export function mockMultipartSubscriptionStream< }, enqueueErrorResult( errors: ApolloPayloadResult["errors"], - payload: ApolloPayloadResult["payload"] = null + payload: ApolloPayloadResult["payload"] = null ) { streamController.enqueue({ payload, errors, [hasNextSymbol]: false }); streamController.close(); From 5ce2c00061a311fd884e8d589e8e9f9dc76a7941 Mon Sep 17 00:00:00 2001 From: jerelmiller <565661+jerelmiller@users.noreply.github.com> Date: Tue, 21 Jan 2025 19:50:02 +0000 Subject: [PATCH 12/27] Clean up Prettier, Size-limit, and Api-Extractor --- .api-reports/api-report-core.api.md | 2 +- .api-reports/api-report-link_core.api.md | 6 +++--- .api-reports/api-report-utilities.api.md | 2 +- .api-reports/api-report.api.md | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.api-reports/api-report-core.api.md b/.api-reports/api-report-core.api.md index 241e1933484..4bc22faf6ef 100644 --- a/.api-reports/api-report-core.api.md +++ b/.api-reports/api-report-core.api.md @@ -261,7 +261,7 @@ export interface ApolloPayloadResult, TExtensions = // (undocumented) errors?: ReadonlyArray; // (undocumented) - payload: SingleExecutionResult | ExecutionPatchResult | null; + payload: SingleExecutionResult | ExecutionPatchResult | null; } // @public (undocumented) diff --git a/.api-reports/api-report-link_core.api.md b/.api-reports/api-report-link_core.api.md index 0171eeadfd8..173736f0ee9 100644 --- a/.api-reports/api-report-link_core.api.md +++ b/.api-reports/api-report-link_core.api.md @@ -44,8 +44,10 @@ export class ApolloLink { export interface ApolloPayloadResult, TExtensions = Record> { // (undocumented) errors?: ReadonlyArray; + // Warning: (ae-forgotten-export) The symbol "DefaultContext" needs to be exported by the entry point index.d.ts + // // (undocumented) - payload: SingleExecutionResult | ExecutionPatchResult | null; + payload: SingleExecutionResult | ExecutionPatchResult | null; } // @public (undocumented) @@ -106,8 +108,6 @@ export const from: typeof ApolloLink.from; // @public (undocumented) export interface GraphQLRequest> { - // Warning: (ae-forgotten-export) The symbol "DefaultContext" needs to be exported by the entry point index.d.ts - // // (undocumented) context?: DefaultContext; // (undocumented) diff --git a/.api-reports/api-report-utilities.api.md b/.api-reports/api-report-utilities.api.md index 5397c853318..2040d463195 100644 --- a/.api-reports/api-report-utilities.api.md +++ b/.api-reports/api-report-utilities.api.md @@ -318,7 +318,7 @@ interface ApolloPayloadResult, TExtensions = Record< // Warning: (ae-forgotten-export) The symbol "ExecutionPatchResult" needs to be exported by the entry point index.d.ts // // (undocumented) - payload: SingleExecutionResult | ExecutionPatchResult | null; + payload: SingleExecutionResult | ExecutionPatchResult | null; } // @public (undocumented) diff --git a/.api-reports/api-report.api.md b/.api-reports/api-report.api.md index 68b3262ac60..5d15453120a 100644 --- a/.api-reports/api-report.api.md +++ b/.api-reports/api-report.api.md @@ -284,7 +284,7 @@ export interface ApolloPayloadResult, TExtensions = // (undocumented) errors?: ReadonlyArray; // (undocumented) - payload: SingleExecutionResult | ExecutionPatchResult | null; + payload: SingleExecutionResult | ExecutionPatchResult | null; } // Warning: (ae-forgotten-export) The symbol "ApolloProviderProps" needs to be exported by the entry point index.d.ts From 17b7ed2c9c66a53f82d86a913bf7f40435a907dd Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 12:56:40 -0700 Subject: [PATCH 13/27] Update changesets --- .changeset/giant-peas-lie.md | 2 +- .changeset/mighty-shoes-clap.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/giant-peas-lie.md b/.changeset/giant-peas-lie.md index 9caaab73e33..27e4096c99c 100644 --- a/.changeset/giant-peas-lie.md +++ b/.changeset/giant-peas-lie.md @@ -2,7 +2,7 @@ "@apollo/client": patch --- -Make `protocolErrors` from multipart subscriptions available to the error link with the `protocolErrors` property. +Make fatal [tranport-level errors](https://www.apollographql.com/docs/graphos/routing/operations/subscriptions/multipart-protocol#message-and-error-format) from multipart subscriptions available to the error link with the `protocolErrors` property. ```js const errorLink = onError(({ protocolErrors }) => { diff --git a/.changeset/mighty-shoes-clap.md b/.changeset/mighty-shoes-clap.md index 5c155848405..da087271edb 100644 --- a/.changeset/mighty-shoes-clap.md +++ b/.changeset/mighty-shoes-clap.md @@ -2,4 +2,4 @@ "@apollo/client": patch --- -Remove the `string` type from the array union for the `errors` field on the `ApolloPayloadResult` type. Errors were never plain strings and always in the GraphQL error format. +Fix the array type for the `errors` field on the `ApolloPayloadResult` type. This type was always in the shape of the GraphQL error format, per the [multipart subscriptions protocol](https://www.apollographql.com/docs/graphos/routing/operations/subscriptions/multipart-protocol#message-and-error-format) and never a plain string or a JavaScript error object. From d5d4a775d5c1513a08e25299a09399455c4ccb16 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 13:00:32 -0700 Subject: [PATCH 14/27] Update docs --- docs/source/api/link/apollo-link-error.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/source/api/link/apollo-link-error.md b/docs/source/api/link/apollo-link-error.md index 5a646e81a67..138c9eaf1f5 100644 --- a/docs/source/api/link/apollo-link-error.md +++ b/docs/source/api/link/apollo-link-error.md @@ -10,7 +10,7 @@ Use the `onError` link to perform custom logic when a [GraphQL or network error] ```js import { onError } from "@apollo/client/link/error"; -// Log any GraphQL errors or network error that occurred +// Log any GraphQL errors, protocol errors, or network error that occurred const errorLink = onError(({ graphQLErrors, networkError, protocolErrors }) => { if (graphQLErrors) graphQLErrors.forEach(({ message, locations, path }) => @@ -109,6 +109,20 @@ A network error that occurred while attempting to execute the operation, if any. + + + +###### `protocolErrors` + +`ReadonlyArray<{ message: string; extensions?: GraphQLErrorExtensions[]; }>` + + + +Fatal transport-level errors from multipart subscriptions. See the [multipart subscription protocol](https://www.apollographql.com/docs/graphos/routing/operations/subscriptions/multipart-protocol#message-and-error-format) for more information. + + + + From e16cd54fbccf70bdd380f4c024950537298d1294 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 13:38:04 -0700 Subject: [PATCH 15/27] Update class-based test to use mock subscription helpers --- src/link/error/__tests__/index.ts | 68 +++++++++++++++---------------- 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/src/link/error/__tests__/index.ts b/src/link/error/__tests__/index.ts index 11b5c8add9a..ddb651dbad5 100644 --- a/src/link/error/__tests__/index.ts +++ b/src/link/error/__tests__/index.ts @@ -485,60 +485,56 @@ describe("error handling with class", () => { expect(called).toBe(true); }); - it("handles protocol errors", async () => { - expect.assertions(3); - const query = gql` - query Foo { - foo { - bar + it("handles protocol errors (multipart subscription)", async () => { + expect.assertions(4); + const subscription = gql` + subscription MySubscription { + aNewDieWasCreated { + die { + roll + sides + color + } } } `; + const { httpLink, enqueuePayloadResult, enqueueErrorResult } = + mockMultipartSubscriptionStream(); + const errorLink = new ErrorLink(({ operation, protocolErrors }) => { - expect(operation.operationName).toBe("Foo"); + expect(operation.operationName).toBe("MySubscription"); expect(protocolErrors).toEqual([ { - message: "cannot read message from websocket", - extensions: [{ code: "WEBSOCKET_MESSAGE_ERROR" }], + message: "Error field", + extensions: { code: "INTERNAL_SERVER_ERROR" }, }, ]); }); - const mockLink = new ApolloLink((_operation) => { - return new Observable((observer) => { - observer.next({ - data: null, - extensions: { - [PROTOCOL_ERRORS_SYMBOL]: [ - { - message: "cannot read message from websocket", - extensions: [ - { - code: "WEBSOCKET_MESSAGE_ERROR", - }, - ], - }, - ], - }, - }); - }); + const link = errorLink.concat(httpLink); + const stream = new ObservableStream(execute(link, { query: subscription })); + + enqueuePayloadResult({ + data: { aNewDieWasCreated: { die: { color: "red", roll: 1, sides: 4 } } }, }); - const link = errorLink.concat(mockLink); - const stream = new ObservableStream(execute(link, { query })); + enqueueErrorResult([ + { message: "Error field", extensions: { code: "INTERNAL_SERVER_ERROR" } }, + ]); + + await expect(stream).toEmitValue({ + data: { aNewDieWasCreated: { die: { color: "red", roll: 1, sides: 4 } } }, + }); await expect(stream).toEmitValue({ - data: null, extensions: { [PROTOCOL_ERRORS_SYMBOL]: [ { - message: "cannot read message from websocket", - extensions: [ - { - code: "WEBSOCKET_MESSAGE_ERROR", - }, - ], + extensions: { + code: "INTERNAL_SERVER_ERROR", + }, + message: "Error field", }, ], }, From 266c642a4eda25c999bd2486f2852426dd7a9c49 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 18:11:20 -0700 Subject: [PATCH 16/27] Create new streams for each request and better handle retries --- src/testing/internal/incremental.ts | 139 ++++++++++++++++++---------- 1 file changed, 88 insertions(+), 51 deletions(-) diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts index 96bff3e713f..83c93b71e6a 100644 --- a/src/testing/internal/incremental.ts +++ b/src/testing/internal/incremental.ts @@ -19,35 +19,48 @@ export function mockIncrementalStream({ }: { responseHeaders: Headers; }) { - let streamController: ReadableStreamDefaultController< + type StreamController = ReadableStreamDefaultController< Chunks & { [hasNextSymbol]: boolean } >; let sentInitialChunk = false; - const stream = new NodeReadableStream({ - start(c) { - streamController = c; - }, - }) - .pipeThrough( - new TransformStream({ - transform: (chunk, controller) => { - controller.enqueue( - (!sentInitialChunk ? "\r\n---\r\n" : "") + - "content-type: application/json; charset=utf-8\r\n\r\n" + - JSON.stringify(chunk) + - (chunk[hasNextSymbol] ? "\r\n---\r\n" : "\r\n-----\r\n") - ); - sentInitialChunk = true; - }, - }) - ) - .pipeThrough(new TextEncoderStream()); + let resolve!: (streamController: StreamController) => void; + let promise!: Promise; + + createPromise(); + + function createPromise() { + promise = new Promise((res) => { + resolve = res; + }); + } + + function createStream() { + return new NodeReadableStream({ + start(c) { + resolve(c); + }, + }) + .pipeThrough( + new TransformStream({ + transform: (chunk, controller) => { + controller.enqueue( + (!sentInitialChunk ? "\r\n---\r\n" : "") + + "content-type: application/json; charset=utf-8\r\n\r\n" + + JSON.stringify(chunk) + + (chunk[hasNextSymbol] ? "\r\n---\r\n" : "\r\n-----\r\n") + ); + sentInitialChunk = true; + }, + }) + ) + .pipeThrough(new TextEncoderStream()); + } const httpLink = new HttpLink({ fetch(input, init) { return Promise.resolve( new Response( - stream satisfies NodeReadableStream as ReadableStream, + createStream() satisfies NodeReadableStream as ReadableStream, { status: 200, headers: responseHeaders, @@ -56,9 +69,39 @@ export function mockIncrementalStream({ ); }, }); + + async function close() { + const streamController = await promise; + streamController.close(); + sentInitialChunk = false; + createPromise(); + } + + async function enqueue( + chunk: Chunks, + hasNext: boolean, + { timeout = 100 }: { timeout?: number } = {} + ) { + const streamController = await Promise.race([ + promise, + new Promise((_, reject) => { + setTimeout(() => { + reject("Timeout waiting for creation of ReadableStream controller"); + }, timeout); + }), + ]); + + streamController.enqueue({ ...chunk, [hasNextSymbol]: hasNext }); + + if (!hasNext) { + await close(); + } + } + return { httpLink, - streamController: streamController!, + enqueue, + close, }; } @@ -66,7 +109,7 @@ export function mockDeferStream< TData = Record, TExtensions = Record, >() { - const { httpLink, streamController } = mockIncrementalStream< + const { httpLink, enqueue } = mockIncrementalStream< | InitialIncrementalExecutionResult | SubsequentIncrementalExecutionResult >({ @@ -76,32 +119,29 @@ export function mockDeferStream< }); return { httpLink, - streamController: streamController!, enqueueInitialChunk( chunk: InitialIncrementalExecutionResult ) { - streamController.enqueue({ ...chunk, [hasNextSymbol]: chunk.hasNext }); - if (!chunk.hasNext) streamController.close(); + enqueue(chunk, chunk.hasNext); }, enqueueSubsequentChunk( chunk: SubsequentIncrementalExecutionResult ) { - streamController.enqueue({ ...chunk, [hasNextSymbol]: chunk.hasNext }); - if (!chunk.hasNext) streamController.close(); + enqueue(chunk, chunk.hasNext); }, enqueueProtocolErrorChunk(errors: GraphQLFormattedError[]) { - streamController.enqueue({ - hasNext: true, - [hasNextSymbol]: true, - incremental: [ - { - // eslint-disable-next-line @typescript-eslint/ban-types - errors: errors as GraphQLError[], - }, - ], - } satisfies SubsequentIncrementalExecutionResult & { - [hasNextSymbol]: boolean; - }); + enqueue( + { + hasNext: true, + incremental: [ + { + // eslint-disable-next-line @typescript-eslint/ban-types + errors: errors as GraphQLError[], + }, + ], + } satisfies SubsequentIncrementalExecutionResult, + true + ); }, }; } @@ -110,7 +150,7 @@ export function mockMultipartSubscriptionStream< TData = Record, TExtensions = Record, >() { - const { httpLink, streamController } = mockIncrementalStream< + const { httpLink, enqueue } = mockIncrementalStream< ApolloPayloadResult >({ responseHeaders: new Headers({ @@ -118,25 +158,22 @@ export function mockMultipartSubscriptionStream< }), }); - // send initial empty chunk back - streamController.enqueue({} as any); - return { httpLink, - streamController: streamController!, - enqueuePayloadResult( + async enqueueInitial() { + await enqueue({} as any, true); + }, + async enqueuePayloadResult( payload: ApolloPayloadResult["payload"], hasNext = true ) { - streamController.enqueue({ payload, [hasNextSymbol]: hasNext }); - if (!hasNext) streamController.close(); + await enqueue({ payload }, hasNext); }, - enqueueErrorResult( + async enqueueErrorResult( errors: ApolloPayloadResult["errors"], payload: ApolloPayloadResult["payload"] = null ) { - streamController.enqueue({ payload, errors, [hasNextSymbol]: false }); - streamController.close(); + await enqueue({ payload, errors }, false); }, }; } From cb5894f3c2ad9981f490a9ad7430040156d5826a Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 18:22:23 -0700 Subject: [PATCH 17/27] Add process queue to allow for synchronous enqueue --- src/testing/internal/incremental.ts | 79 ++++++++++++++++------------- 1 file changed, 44 insertions(+), 35 deletions(-) diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts index 83c93b71e6a..d1b8f35f453 100644 --- a/src/testing/internal/incremental.ts +++ b/src/testing/internal/incremental.ts @@ -19,25 +19,38 @@ export function mockIncrementalStream({ }: { responseHeaders: Headers; }) { + const CLOSE = Symbol(); type StreamController = ReadableStreamDefaultController< Chunks & { [hasNextSymbol]: boolean } >; + let streamController: ReadableStreamDefaultController< + Chunks & { [hasNextSymbol]: boolean } + > | null = null; let sentInitialChunk = false; - let resolve!: (streamController: StreamController) => void; - let promise!: Promise; - createPromise(); + const queue: Array<(Chunks & { [hasNextSymbol]: boolean }) | typeof CLOSE> = + []; + + function processQueue(streamController: StreamController) { + if (!streamController) { + throw new Error("Cannot process queue. Stream controller not created."); + } - function createPromise() { - promise = new Promise((res) => { - resolve = res; - }); + let chunk; + while ((chunk = queue.shift())) { + if (chunk === CLOSE) { + streamController.close(); + } else { + streamController.enqueue(chunk); + } + } } function createStream() { return new NodeReadableStream({ start(c) { - resolve(c); + streamController = c; + processQueue(c); }, }) .pipeThrough( @@ -70,31 +83,28 @@ export function mockIncrementalStream({ }, }); - async function close() { - const streamController = await promise; - streamController.close(); + function close() { + if (streamController) { + streamController.close(); + } else { + queue.push(CLOSE); + } + + streamController = null; sentInitialChunk = false; - createPromise(); } - async function enqueue( - chunk: Chunks, - hasNext: boolean, - { timeout = 100 }: { timeout?: number } = {} - ) { - const streamController = await Promise.race([ - promise, - new Promise((_, reject) => { - setTimeout(() => { - reject("Timeout waiting for creation of ReadableStream controller"); - }, timeout); - }), - ]); - - streamController.enqueue({ ...chunk, [hasNextSymbol]: hasNext }); + function enqueue(chunk: Chunks, hasNext: boolean) { + const payload = { ...chunk, [hasNextSymbol]: hasNext }; + + if (streamController) { + streamController.enqueue(payload); + } else { + queue.push(payload); + } if (!hasNext) { - await close(); + close(); } } @@ -158,22 +168,21 @@ export function mockMultipartSubscriptionStream< }), }); + enqueue({} as any, true); + return { httpLink, - async enqueueInitial() { - await enqueue({} as any, true); - }, - async enqueuePayloadResult( + enqueuePayloadResult( payload: ApolloPayloadResult["payload"], hasNext = true ) { - await enqueue({ payload }, hasNext); + enqueue({ payload }, hasNext); }, - async enqueueErrorResult( + enqueueErrorResult( errors: ApolloPayloadResult["errors"], payload: ApolloPayloadResult["payload"] = null ) { - await enqueue({ payload, errors }, false); + enqueue({ payload, errors }, false); }, }; } From bb99ec4ba3a474029edcee7140b8675d0a85466b Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 18:50:34 -0700 Subject: [PATCH 18/27] Remove unneeded check --- src/testing/internal/incremental.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts index d1b8f35f453..353e75036ba 100644 --- a/src/testing/internal/incremental.ts +++ b/src/testing/internal/incremental.ts @@ -32,10 +32,6 @@ export function mockIncrementalStream({ []; function processQueue(streamController: StreamController) { - if (!streamController) { - throw new Error("Cannot process queue. Stream controller not created."); - } - let chunk; while ((chunk = queue.shift())) { if (chunk === CLOSE) { From fefd26e1460ac89daa358b4178d847c90762bdaa Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 18:53:03 -0700 Subject: [PATCH 19/27] Use stream http link to test retry --- src/link/error/__tests__/index.ts | 70 +++++++++++++++++-------------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/src/link/error/__tests__/index.ts b/src/link/error/__tests__/index.ts index ddb651dbad5..9aebd528161 100644 --- a/src/link/error/__tests__/index.ts +++ b/src/link/error/__tests__/index.ts @@ -650,18 +650,6 @@ describe("support for request retrying", () => { message: "some other error", }; - const PROTOCOL_ERROR = { - data: null, - extensions: { - [PROTOCOL_ERRORS_SYMBOL]: [ - { - message: "cannot read message from websocket", - extensions: [{ code: "WEBSOCKET_MESSAGE_ERROR" }], - }, - ], - }, - }; - it("returns the retried request when forward(operation) is called", async () => { let errorHandlerCalled = false; @@ -750,38 +738,56 @@ describe("support for request retrying", () => { it("supports retrying when the initial request had protocol errors", async () => { let errorHandlerCalled = false; - let timesCalled = 0; - const mockHttpLink = new ApolloLink((operation) => { - return new Observable((observer) => { - // simulate the first request being an error - if (timesCalled === 0) { - timesCalled++; - observer.next(PROTOCOL_ERROR); - observer.complete(); - } else { - observer.next(GOOD_RESPONSE); - observer.complete(); - } - }); - }); + const { httpLink, enqueuePayloadResult, enqueueErrorResult } = + mockMultipartSubscriptionStream(); const errorLink = new ErrorLink( ({ protocolErrors, operation, forward }) => { if (protocolErrors) { errorHandlerCalled = true; - expect(protocolErrors).toEqual( - PROTOCOL_ERROR.extensions[PROTOCOL_ERRORS_SYMBOL] - ); + expect(protocolErrors).toEqual([ + { + message: "cannot read message from websocket", + extensions: { + code: "WEBSOCKET_MESSAGE_ERROR", + }, + }, + ]); return forward(operation); } } ); - const link = errorLink.concat(mockHttpLink); + const link = errorLink.concat(httpLink); + const stream = new ObservableStream( + execute(link, { + query: gql` + subscription Foo { + foo { + bar + } + } + `, + }) + ); - const stream = new ObservableStream(execute(link, { query: QUERY })); + enqueuePayloadResult({ data: { foo: { bar: true } } }); - await expect(stream).toEmitValue(GOOD_RESPONSE); + await expect(stream).toEmitValue({ data: { foo: { bar: true } } }); + + enqueueErrorResult([ + { + message: "cannot read message from websocket", + extensions: { + code: "WEBSOCKET_MESSAGE_ERROR", + }, + }, + ]); + + enqueuePayloadResult({ data: { foo: { bar: true } } }, false); + + // Ensure the error result is not emitted but rather the retried result + await expect(stream).toEmitValue({ data: { foo: { bar: true } } }); expect(errorHandlerCalled).toBe(true); await expect(stream).toComplete(); }); From d5f24693400496e60b7602922006b4b98fe8e032 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 18:57:53 -0700 Subject: [PATCH 20/27] Rename enqueueErrorResult with enqueueProtocolErrors --- src/link/error/__tests__/index.ts | 15 +++++++++------ src/testing/internal/incremental.ts | 7 ++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/link/error/__tests__/index.ts b/src/link/error/__tests__/index.ts index 9aebd528161..acc9cbc3338 100644 --- a/src/link/error/__tests__/index.ts +++ b/src/link/error/__tests__/index.ts @@ -167,7 +167,7 @@ describe("error handling", () => { ]); }); - const { httpLink, enqueuePayloadResult, enqueueErrorResult } = + const { httpLink, enqueuePayloadResult, enqueueProtocolErrors } = mockMultipartSubscriptionStream(); const link = errorLink.concat(httpLink); const stream = new ObservableStream( @@ -178,7 +178,7 @@ describe("error handling", () => { data: { aNewDieWasCreated: { die: { color: "red", roll: 1, sides: 4 } } }, }); - enqueueErrorResult([ + enqueueProtocolErrors([ { message: "Error field", extensions: { code: "INTERNAL_SERVER_ERROR" } }, ]); @@ -499,8 +499,11 @@ describe("error handling with class", () => { } `; - const { httpLink, enqueuePayloadResult, enqueueErrorResult } = - mockMultipartSubscriptionStream(); + const { + httpLink, + enqueuePayloadResult, + enqueueProtocolErrors: enqueueErrorResult, + } = mockMultipartSubscriptionStream(); const errorLink = new ErrorLink(({ operation, protocolErrors }) => { expect(operation.operationName).toBe("MySubscription"); @@ -738,7 +741,7 @@ describe("support for request retrying", () => { it("supports retrying when the initial request had protocol errors", async () => { let errorHandlerCalled = false; - const { httpLink, enqueuePayloadResult, enqueueErrorResult } = + const { httpLink, enqueuePayloadResult, enqueueProtocolErrors } = mockMultipartSubscriptionStream(); const errorLink = new ErrorLink( @@ -775,7 +778,7 @@ describe("support for request retrying", () => { await expect(stream).toEmitValue({ data: { foo: { bar: true } } }); - enqueueErrorResult([ + enqueueProtocolErrors([ { message: "cannot read message from websocket", extensions: { diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts index 353e75036ba..d558f3ea2ac 100644 --- a/src/testing/internal/incremental.ts +++ b/src/testing/internal/incremental.ts @@ -174,11 +174,8 @@ export function mockMultipartSubscriptionStream< ) { enqueue({ payload }, hasNext); }, - enqueueErrorResult( - errors: ApolloPayloadResult["errors"], - payload: ApolloPayloadResult["payload"] = null - ) { - enqueue({ payload, errors }, false); + enqueueProtocolErrors(errors: ApolloPayloadResult["errors"]) { + enqueue({ payload: null, errors }, false); }, }; } From b1e73a25cb57d4e8ba37bd6a80dbf41e220d54b0 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 18:58:04 -0700 Subject: [PATCH 21/27] Add enqueueHeartbeat --- src/testing/internal/incremental.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts index d558f3ea2ac..9e8f1c92e40 100644 --- a/src/testing/internal/incremental.ts +++ b/src/testing/internal/incremental.ts @@ -168,6 +168,9 @@ export function mockMultipartSubscriptionStream< return { httpLink, + enqueueHeartbeat() { + enqueue({} as any, true); + }, enqueuePayloadResult( payload: ApolloPayloadResult["payload"], hasNext = true From 40cd6f09d61ee9384c297b4011fe4845c636971c Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 18:59:21 -0700 Subject: [PATCH 22/27] Refactor initial chunk to use enqueueHeartbeat --- src/testing/internal/incremental.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts index 9e8f1c92e40..51f6ca328b6 100644 --- a/src/testing/internal/incremental.ts +++ b/src/testing/internal/incremental.ts @@ -164,13 +164,15 @@ export function mockMultipartSubscriptionStream< }), }); - enqueue({} as any, true); + enqueueHeartbeat(); + + function enqueueHeartbeat() { + enqueue({} as any, true); + } return { httpLink, - enqueueHeartbeat() { - enqueue({} as any, true); - }, + enqueueHeartbeat, enqueuePayloadResult( payload: ApolloPayloadResult["payload"], hasNext = true From 27d2eac987357eba9b2ffac1c8f505203a80be46 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 19:00:42 -0700 Subject: [PATCH 23/27] Rename enqueueProtocolErrorChunk to enqueueErrorChunk in defer mock --- src/link/error/__tests__/index.ts | 4 ++-- src/testing/internal/incremental.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/link/error/__tests__/index.ts b/src/link/error/__tests__/index.ts index acc9cbc3338..8c1456019c1 100644 --- a/src/link/error/__tests__/index.ts +++ b/src/link/error/__tests__/index.ts @@ -102,7 +102,7 @@ describe("error handling", () => { ]); }); - const { httpLink, enqueueInitialChunk, enqueueProtocolErrorChunk } = + const { httpLink, enqueueInitialChunk, enqueueErrorChunk } = mockDeferStream(); const link = errorLink.concat(httpLink); const stream = new ObservableStream(execute(link, { query })); @@ -112,7 +112,7 @@ describe("error handling", () => { data: {}, }); - enqueueProtocolErrorChunk([ + enqueueErrorChunk([ { message: "could not read data", extensions: { diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts index 51f6ca328b6..3b4d6f9df6e 100644 --- a/src/testing/internal/incremental.ts +++ b/src/testing/internal/incremental.ts @@ -135,7 +135,7 @@ export function mockDeferStream< ) { enqueue(chunk, chunk.hasNext); }, - enqueueProtocolErrorChunk(errors: GraphQLFormattedError[]) { + enqueueErrorChunk(errors: GraphQLFormattedError[]) { enqueue( { hasNext: true, From c7dde9eb6c11782744f52102ff799284f241c680 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 19:04:21 -0700 Subject: [PATCH 24/27] Remove alias --- src/link/error/__tests__/index.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/link/error/__tests__/index.ts b/src/link/error/__tests__/index.ts index 8c1456019c1..9fe9cf257af 100644 --- a/src/link/error/__tests__/index.ts +++ b/src/link/error/__tests__/index.ts @@ -499,11 +499,8 @@ describe("error handling with class", () => { } `; - const { - httpLink, - enqueuePayloadResult, - enqueueProtocolErrors: enqueueErrorResult, - } = mockMultipartSubscriptionStream(); + const { httpLink, enqueuePayloadResult, enqueueProtocolErrors } = + mockMultipartSubscriptionStream(); const errorLink = new ErrorLink(({ operation, protocolErrors }) => { expect(operation.operationName).toBe("MySubscription"); @@ -522,7 +519,7 @@ describe("error handling with class", () => { data: { aNewDieWasCreated: { die: { color: "red", roll: 1, sides: 4 } } }, }); - enqueueErrorResult([ + enqueueProtocolErrors([ { message: "Error field", extensions: { code: "INTERNAL_SERVER_ERROR" } }, ]); From fe6281c6f4a3363fb5c70d1d529fd17e9ead7e93 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 19:08:27 -0700 Subject: [PATCH 25/27] Minor refactor to avoid multiple ifs --- src/testing/internal/incremental.ts | 33 +++++++++++++++-------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts index 3b4d6f9df6e..2b234791750 100644 --- a/src/testing/internal/incremental.ts +++ b/src/testing/internal/incremental.ts @@ -20,9 +20,6 @@ export function mockIncrementalStream({ responseHeaders: Headers; }) { const CLOSE = Symbol(); - type StreamController = ReadableStreamDefaultController< - Chunks & { [hasNextSymbol]: boolean } - >; let streamController: ReadableStreamDefaultController< Chunks & { [hasNextSymbol]: boolean } > | null = null; @@ -31,7 +28,11 @@ export function mockIncrementalStream({ const queue: Array<(Chunks & { [hasNextSymbol]: boolean }) | typeof CLOSE> = []; - function processQueue(streamController: StreamController) { + function processQueue() { + if (!streamController) { + throw new Error("Cannot process queue without stream controller"); + } + let chunk; while ((chunk = queue.shift())) { if (chunk === CLOSE) { @@ -46,7 +47,7 @@ export function mockIncrementalStream({ return new NodeReadableStream({ start(c) { streamController = c; - processQueue(c); + processQueue(); }, }) .pipeThrough( @@ -79,25 +80,25 @@ export function mockIncrementalStream({ }, }); - function close() { + function queueNext( + event: (Chunks & { [hasNextSymbol]: boolean }) | typeof CLOSE + ) { + queue.push(event); + if (streamController) { - streamController.close(); - } else { - queue.push(CLOSE); + processQueue(); } + } + + function close() { + queueNext(CLOSE); streamController = null; sentInitialChunk = false; } function enqueue(chunk: Chunks, hasNext: boolean) { - const payload = { ...chunk, [hasNextSymbol]: hasNext }; - - if (streamController) { - streamController.enqueue(payload); - } else { - queue.push(payload); - } + queueNext({ ...chunk, [hasNextSymbol]: hasNext }); if (!hasNext) { close(); From 4775b82a539e142364c9b7596ad20e7e7cb9e360 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 19:09:48 -0700 Subject: [PATCH 26/27] Minor tweak to avoid repeating same type --- src/testing/internal/incremental.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts index 2b234791750..5e237b9fc44 100644 --- a/src/testing/internal/incremental.ts +++ b/src/testing/internal/incremental.ts @@ -19,14 +19,12 @@ export function mockIncrementalStream({ }: { responseHeaders: Headers; }) { + type Payload = Chunks & { [hasNextSymbol]: boolean }; const CLOSE = Symbol(); - let streamController: ReadableStreamDefaultController< - Chunks & { [hasNextSymbol]: boolean } - > | null = null; + let streamController: ReadableStreamDefaultController | null = null; let sentInitialChunk = false; - const queue: Array<(Chunks & { [hasNextSymbol]: boolean }) | typeof CLOSE> = - []; + const queue: Array = []; function processQueue() { if (!streamController) { @@ -80,9 +78,7 @@ export function mockIncrementalStream({ }, }); - function queueNext( - event: (Chunks & { [hasNextSymbol]: boolean }) | typeof CLOSE - ) { + function queueNext(event: Payload | typeof CLOSE) { queue.push(event); if (streamController) { From d2f62adcb6e8bde99d12393d0c5159fbcc269e4a Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Tue, 21 Jan 2025 19:17:43 -0700 Subject: [PATCH 27/27] Fix lint issue due to upgrade of eslint --- src/testing/internal/incremental.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/testing/internal/incremental.ts b/src/testing/internal/incremental.ts index 5e237b9fc44..b03aebe7bfa 100644 --- a/src/testing/internal/incremental.ts +++ b/src/testing/internal/incremental.ts @@ -138,7 +138,7 @@ export function mockDeferStream< hasNext: true, incremental: [ { - // eslint-disable-next-line @typescript-eslint/ban-types + // eslint-disable-next-line @typescript-eslint/no-restricted-types errors: errors as GraphQLError[], }, ],