diff --git a/.api-reports/api-report-core.api.md b/.api-reports/api-report-core.api.md index 1f59611f36c..6d3f7743208 100644 --- a/.api-reports/api-report-core.api.md +++ b/.api-reports/api-report-core.api.md @@ -173,7 +173,7 @@ export class ApolloClient implements DataProxy_2 { setLocalStateFragmentMatcher(fragmentMatcher: FragmentMatcher): void; setResolvers(resolvers: Resolvers | Resolvers[]): void; stop(): void; - subscribe(options: SubscriptionOptions): Observable>>; + subscribe(options: SubscriptionOptions): Observable>>; // (undocumented) readonly typeDefs: ApolloClientOptions["typeDefs"]; // (undocumented) @@ -1823,7 +1823,7 @@ class QueryManager { // (undocumented) readonly ssrMode: boolean; // (undocumented) - startGraphQLSubscription(options: SubscriptionOptions): Observable>; + startGraphQLSubscription(options: SubscriptionOptions): Observable>; stop(): void; // (undocumented) stopQuery(queryId: string): void; @@ -2109,6 +2109,13 @@ class Stump extends Layer { removeLayer(): this; } +// @public (undocumented) +export interface SubscribeResult { + data: TData | undefined; + error?: ErrorLike; + extensions?: Record; +} + // @public (undocumented) export interface SubscribeToMoreFunction { // (undocumented) @@ -2328,8 +2335,8 @@ interface WriteContext extends ReadMergeModifyContext { // src/cache/inmemory/types.ts:133:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:128:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:129:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts -// src/core/QueryManager.ts:185:5 - (ae-forgotten-export) The symbol "MutationStoreValue" needs to be exported by the entry point index.d.ts -// src/core/QueryManager.ts:456:7 - (ae-forgotten-export) The symbol "UpdateQueries" needs to be exported by the entry point index.d.ts +// src/core/QueryManager.ts:187:5 - (ae-forgotten-export) The symbol "MutationStoreValue" needs to be exported by the entry point index.d.ts +// src/core/QueryManager.ts:458:7 - (ae-forgotten-export) The symbol "UpdateQueries" needs to be exported by the entry point index.d.ts // src/link/http/selectHttpOptionsAndBody.ts:128:1 - (ae-forgotten-export) The symbol "HttpQueryOptions" needs to be exported by the entry point index.d.ts // (No @packageDocumentation comment for this package) diff --git a/.api-reports/api-report-react.api.md b/.api-reports/api-report-react.api.md index 0a3cbc57bd6..61c650aa55c 100644 --- a/.api-reports/api-report-react.api.md +++ b/.api-reports/api-report-react.api.md @@ -146,7 +146,8 @@ class ApolloClient_2 implements DataProxy { setResolvers(resolvers: Resolvers | Resolvers[]): void; stop(): void; // Warning: (ae-forgotten-export) The symbol "SubscriptionOptions" needs to be exported by the entry point index.d.ts - subscribe(options: SubscriptionOptions): Observable>>; + // Warning: (ae-forgotten-export) The symbol "SubscribeResult" needs to be exported by the entry point index.d.ts + subscribe(options: SubscriptionOptions): Observable>>; // Warning: (ae-forgotten-export) The symbol "ApolloClientOptions" needs to be exported by the entry point index.d.ts // // (undocumented) @@ -870,7 +871,7 @@ class QueryManager { // (undocumented) readonly ssrMode: boolean; // (undocumented) - startGraphQLSubscription(options: SubscriptionOptions): Observable>; + startGraphQLSubscription(options: SubscriptionOptions): Observable>; stop(): void; // (undocumented) stopQuery(queryId: string): void; @@ -995,6 +996,13 @@ export type SkipToken = typeof skipToken; // @public (undocumented) export const skipToken: unique symbol; +// @public (undocumented) +interface SubscribeResult { + data: TData | undefined; + error?: ErrorLike; + extensions?: Record; +} + // @public (undocumented) interface SubscribeToMoreOptions { // (undocumented) @@ -1464,13 +1472,7 @@ export namespace useReadQuery { export type UseReadQueryResult = useReadQuery_2.Result; // @public -export function useSubscription(subscription: DocumentNode | TypedDocumentNode_2, options?: useSubscription.Options, NoInfer_2>): { - restart: () => void; - loading: boolean; - data?: TData | undefined; - error?: ErrorLike_2; - variables?: TVariables | undefined; -}; +export function useSubscription(subscription: DocumentNode | TypedDocumentNode_2, options?: useSubscription.Options, NoInfer_2>): useSubscription.Result; // @public (undocumented) export namespace useSubscription { @@ -1479,14 +1481,16 @@ export namespace useSubscription { // (undocumented) client: ApolloClient; // (undocumented) - data: Result; + data: OnDataResult; } // (undocumented) + export type OnDataResult = Omit, "restart">; + // (undocumented) export interface OnSubscriptionDataOptions { // (undocumented) client: ApolloClient; // (undocumented) - subscriptionData: Result; + subscriptionData: OnDataResult; } // (undocumented) export interface Options { @@ -1512,6 +1516,8 @@ export namespace useSubscription { data?: MaybeMasked; error?: ErrorLike_2; loading: boolean; + // (undocumented) + restart: () => void; // @internal (undocumented) variables?: TVariables; } @@ -1655,8 +1661,8 @@ interface WatchQueryOptions_2(options: SubscriptionOptions): Observable>>; + // Warning: (ae-forgotten-export) The symbol "SubscribeResult" needs to be exported by the entry point index.d.ts + subscribe(options: SubscriptionOptions): Observable>>; // Warning: (ae-forgotten-export) The symbol "ApolloClientOptions" needs to be exported by the entry point index.d.ts // // (undocumented) @@ -686,7 +687,7 @@ class QueryManager { // (undocumented) readonly ssrMode: boolean; // (undocumented) - startGraphQLSubscription(options: SubscriptionOptions): Observable>; + startGraphQLSubscription(options: SubscriptionOptions): Observable>; stop(): void; // (undocumented) stopQuery(queryId: string): void; @@ -796,6 +797,13 @@ export type SkipToken = typeof skipToken; // @public (undocumented) export const skipToken: unique symbol; +// @public (undocumented) +interface SubscribeResult { + data: TData | undefined; + error?: ErrorLike; + extensions?: Record; +} + // @public (undocumented) interface SubscribeToMoreOptions { // (undocumented) @@ -1235,13 +1243,7 @@ export namespace useReadQuery { } // @public -export function useSubscription(subscription: DocumentNode | TypedDocumentNode, options?: useSubscription.Options, NoInfer_2>): { - restart: () => void; - loading: boolean; - data?: TData | undefined; - error?: ErrorLike_2; - variables?: TVariables | undefined; -}; +export function useSubscription(subscription: DocumentNode | TypedDocumentNode, options?: useSubscription.Options, NoInfer_2>): useSubscription.Result; // @public (undocumented) export namespace useSubscription { @@ -1250,14 +1252,16 @@ export namespace useSubscription { // (undocumented) client: ApolloClient; // (undocumented) - data: Result; + data: OnDataResult; } // (undocumented) + export type OnDataResult = Omit, "restart">; + // (undocumented) export interface OnSubscriptionDataOptions { // (undocumented) client: ApolloClient; // (undocumented) - subscriptionData: Result; + subscriptionData: OnDataResult; } // (undocumented) export interface Options { @@ -1283,6 +1287,8 @@ export namespace useSubscription { data?: MaybeMasked; error?: ErrorLike_2; loading: boolean; + // (undocumented) + restart: () => void; // @internal (undocumented) variables?: TVariables; } @@ -1417,8 +1423,8 @@ interface WatchQueryOptions_2(options: SubscriptionOptions): Observable>>; + subscribe(options: SubscriptionOptions): Observable>>; // (undocumented) readonly typeDefs: ApolloClientOptions["typeDefs"]; // (undocumented) @@ -1939,7 +1939,7 @@ class QueryManager { // (undocumented) readonly ssrMode: boolean; // (undocumented) - startGraphQLSubscription(options: SubscriptionOptions): Observable>; + startGraphQLSubscription(options: SubscriptionOptions): Observable>; stop(): void; // (undocumented) stopQuery(queryId: string): void; @@ -2233,6 +2233,13 @@ class Stump extends Layer { removeLayer(): this; } +// @public (undocumented) +export interface SubscribeResult { + data: TData | undefined; + error?: ErrorLike; + extensions?: Record; +} + // @public (undocumented) export interface SubscribeToMoreFunction { // (undocumented) @@ -2470,8 +2477,8 @@ interface WriteContext extends ReadMergeModifyContext { // src/cache/inmemory/types.ts:133:3 - (ae-forgotten-export) The symbol "KeyFieldsFunction" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:128:5 - (ae-forgotten-export) The symbol "QueryManager" needs to be exported by the entry point index.d.ts // src/core/ObservableQuery.ts:129:5 - (ae-forgotten-export) The symbol "QueryInfo" needs to be exported by the entry point index.d.ts -// src/core/QueryManager.ts:185:5 - (ae-forgotten-export) The symbol "MutationStoreValue" needs to be exported by the entry point index.d.ts -// src/core/QueryManager.ts:456:7 - (ae-forgotten-export) The symbol "UpdateQueries" needs to be exported by the entry point index.d.ts +// src/core/QueryManager.ts:187:5 - (ae-forgotten-export) The symbol "MutationStoreValue" needs to be exported by the entry point index.d.ts +// src/core/QueryManager.ts:458:7 - (ae-forgotten-export) The symbol "UpdateQueries" needs to be exported by the entry point index.d.ts // src/link/http/selectHttpOptionsAndBody.ts:128:1 - (ae-forgotten-export) The symbol "HttpQueryOptions" needs to be exported by the entry point index.d.ts // (No @packageDocumentation comment for this package) diff --git a/.changeset/forty-shrimps-fry.md b/.changeset/forty-shrimps-fry.md new file mode 100644 index 00000000000..6e89634d4f1 --- /dev/null +++ b/.changeset/forty-shrimps-fry.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": major +--- + +Subscriptions now emit a `SubscribeResult` instead of a `FetchResult`. As a result, the `errors` field has been removed in favor of `error`. diff --git a/.changeset/real-teachers-peel.md b/.changeset/real-teachers-peel.md new file mode 100644 index 00000000000..93debc83be1 --- /dev/null +++ b/.changeset/real-teachers-peel.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": major +--- + +Unify error behavior on subscriptions for GraphQL errors and network errors by ensuring network errors are subject to the `errorPolicy`. Network errors that terminate the connection will now be emitted on the `error` property passed to the `next` callback followed by a call to the `complete` callback. diff --git a/.changeset/tricky-tables-shave.md b/.changeset/tricky-tables-shave.md new file mode 100644 index 00000000000..75c7875b106 --- /dev/null +++ b/.changeset/tricky-tables-shave.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": major +--- + +GraphQL errors or network errors emitted while using an `errorPolicy` of `ignore` in subscriptions will no longer emit a result if there is no `data` emitted along with the error. diff --git a/.changeset/warm-ties-sit.md b/.changeset/warm-ties-sit.md new file mode 100644 index 00000000000..b9ed57eeb73 --- /dev/null +++ b/.changeset/warm-ties-sit.md @@ -0,0 +1,7 @@ +--- +"@apollo/client": major +--- + +Subscriptions no longer emit errors in the `error` callback and instead provide errors on the `error` property on the result passed to the `next` callback. As a result, errors will no longer automatically terminate the connection allowing additional results to be emitted when the connection stays open. + +When an error terminates the downstream connection, a `next` event will be emitted with an `error` property followed by a `complete` event instead. diff --git a/.size-limits.json b/.size-limits.json index 3e9346fd7d7..45975c5bc61 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,6 +1,6 @@ { - "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 42184, - "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 37665, - "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 32637, - "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 27537 + "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (CJS)": 42313, + "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production) (CJS)": 37697, + "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\"": 32739, + "import { ApolloClient, InMemoryCache, HttpLink } from \"@apollo/client\" (production)": 27589 } diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index 9dc87ad1fe0..4101185eac8 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -6253,7 +6253,7 @@ describe("custom document transforms", () => { }); describe("unconventional errors", () => { - test("wraps error mesage in Error type when erroring with a string", async () => { + test("wraps error message in Error type when erroring with a string", async () => { const query = gql` query { hello @@ -6277,7 +6277,7 @@ describe("unconventional errors", () => { const stream = new ObservableStream(client.watchQuery({ query })); - await expect(stream).toEmitApolloQueryResult({ + await expect(stream).toEmitStrictTyped({ data: undefined, error: expectedError, loading: false, @@ -6304,7 +6304,10 @@ describe("unconventional errors", () => { }); const subscriptionStream = new ObservableStream(subscription); - await expect(subscriptionStream).toEmitError(expectedError); + await expect(subscriptionStream).toEmitStrictTyped({ + data: undefined, + error: expectedError, + }); }); test("wraps unconventional types in UnconventionalError", async () => { @@ -6332,7 +6335,7 @@ describe("unconventional errors", () => { const stream = new ObservableStream(client.watchQuery({ query })); - await expect(stream).toEmitApolloQueryResult({ + await expect(stream).toEmitStrictTyped({ data: undefined, error: expectedError, loading: false, @@ -6359,7 +6362,10 @@ describe("unconventional errors", () => { }); const subscriptionStream = new ObservableStream(subscription); - await expect(subscriptionStream).toEmitError(expectedError); + await expect(subscriptionStream).toEmitStrictTyped({ + data: undefined, + error: expectedError, + }); } }); }); diff --git a/src/__tests__/dataMasking.ts b/src/__tests__/dataMasking.ts index e747d14abe9..a6b083b7ff0 100644 --- a/src/__tests__/dataMasking.ts +++ b/src/__tests__/dataMasking.ts @@ -4388,11 +4388,10 @@ describe("client.subscribe", () => { }, }); - const error = await stream.takeError(); - - expect(error).toEqual( - new CombinedGraphQLErrors([{ message: "Something went wrong" }]) - ); + await expect(stream).toEmitStrictTyped({ + data: undefined, + error: new CombinedGraphQLErrors([{ message: "Something went wrong" }]), + }); }); test("handles errors returned from the subscription when errorPolicy is `all`", async () => { @@ -4433,10 +4432,10 @@ describe("client.subscribe", () => { }, }); - const { data, errors } = await stream.takeNext(); - - expect(data).toEqual({ addedComment: null }); - expect(errors).toEqual([{ message: "Something went wrong" }]); + await expect(stream).toEmitStrictTyped({ + data: { addedComment: null }, + error: new CombinedGraphQLErrors([{ message: "Something went wrong" }]), + }); }); test("masks partial data for errors returned from the subscription when errorPolicy is `all`", async () => { @@ -4482,10 +4481,10 @@ describe("client.subscribe", () => { }, }); - const { data, errors } = await stream.takeNext(); - - expect(data).toEqual({ addedComment: { __typename: "Comment", id: 1 } }); - expect(errors).toEqual([{ message: "Could not get author" }]); + await expect(stream).toEmitStrictTyped({ + data: { addedComment: { __typename: "Comment", id: 1 } }, + error: new CombinedGraphQLErrors([{ message: "Could not get author" }]), + }); }); test("warns and returns masked result when used with no-cache fetch policy", async () => { diff --git a/src/__tests__/graphqlSubscriptions.ts b/src/__tests__/graphqlSubscriptions.ts index b15b0c19fa5..86479baa63f 100644 --- a/src/__tests__/graphqlSubscriptions.ts +++ b/src/__tests__/graphqlSubscriptions.ts @@ -1,4 +1,3 @@ -import { GraphQLError } from "graphql"; import { gql } from "graphql-tag"; import { InMemoryCache } from "@apollo/client/cache"; @@ -6,11 +5,14 @@ import { ApolloClient } from "@apollo/client/core"; import { CombinedGraphQLErrors, CombinedProtocolErrors, - PROTOCOL_ERRORS_SYMBOL, } from "@apollo/client/errors"; -import { mockObservableLink } from "@apollo/client/testing"; +import { MockSubscriptionLink } from "@apollo/client/testing"; -import { ObservableStream, spyOnConsole } from "../testing/internal/index.js"; +import { + mockMultipartSubscriptionStream, + ObservableStream, + spyOnConsole, +} from "../testing/internal/index.js"; describe("GraphQL Subscriptions", () => { const results = [ @@ -51,7 +53,7 @@ describe("GraphQL Subscriptions", () => { }); it("should start a subscription on network interface and unsubscribe", async () => { - const link = mockObservableLink(); + const link = new MockSubscriptionLink(); // This test calls directly through Apollo Client const client = new ApolloClient({ link, @@ -61,13 +63,13 @@ describe("GraphQL Subscriptions", () => { const stream = new ObservableStream(client.subscribe(defaultOptions)); link.simulateResult(results[0]); - await expect(stream).toEmitFetchResult(results[0].result); + await expect(stream).toEmitStrictTyped(results[0].result); stream.unsubscribe(); }); it("should subscribe with default values", async () => { - const link = mockObservableLink(); + const link = new MockSubscriptionLink(); // This test calls directly through Apollo Client const client = new ApolloClient({ link, @@ -78,13 +80,13 @@ describe("GraphQL Subscriptions", () => { link.simulateResult(results[0]); - await expect(stream).toEmitFetchResult(results[0].result); + await expect(stream).toEmitStrictTyped(results[0].result); stream.unsubscribe(); }); it("should multiplex subscriptions", async () => { - const link = mockObservableLink(); + const link = new MockSubscriptionLink(); const client = new ApolloClient({ link, cache: new InMemoryCache(), @@ -96,12 +98,12 @@ describe("GraphQL Subscriptions", () => { link.simulateResult(results[0]); - await expect(stream1).toEmitFetchResult(results[0].result); - await expect(stream2).toEmitFetchResult(results[0].result); + await expect(stream1).toEmitStrictTyped(results[0].result); + await expect(stream2).toEmitStrictTyped(results[0].result); }); it("should receive multiple results for a subscription", async () => { - const link = mockObservableLink(); + const link = new MockSubscriptionLink(); const client = new ApolloClient({ link, cache: new InMemoryCache(), @@ -113,15 +115,15 @@ describe("GraphQL Subscriptions", () => { link.simulateResult(results[i]); } - await expect(stream).toEmitFetchResult(results[0].result); - await expect(stream).toEmitFetchResult(results[1].result); - await expect(stream).toEmitFetchResult(results[2].result); - await expect(stream).toEmitFetchResult(results[3].result); + await expect(stream).toEmitStrictTyped(results[0].result); + await expect(stream).toEmitStrictTyped(results[1].result); + await expect(stream).toEmitStrictTyped(results[2].result); + await expect(stream).toEmitStrictTyped(results[3].result); await expect(stream).not.toEmitAnything(); }); it("should not cache subscription data if a `no-cache` fetch policy is used", async () => { - const link = mockObservableLink(); + const link = new MockSubscriptionLink(); const cache = new InMemoryCache(); const client = new ApolloClient({ link, @@ -139,8 +141,8 @@ describe("GraphQL Subscriptions", () => { expect(cache.extract()).toEqual({}); }); - it("should throw an error if the result has errors on it", async () => { - const link = mockObservableLink(); + it("emits an error if the result has GraphQL errors", async () => { + const link = new MockSubscriptionLink(); const client = new ApolloClient({ link, cache: new InMemoryCache(), @@ -162,13 +164,14 @@ describe("GraphQL Subscriptions", () => { }, ], path: ["result"], - } as any, + }, ], }, }); - await expect(stream).toEmitError( - new CombinedGraphQLErrors([ + await expect(stream).toEmitStrictTyped({ + data: undefined, + error: new CombinedGraphQLErrors([ { message: "This is an error", locations: [ @@ -179,8 +182,77 @@ describe("GraphQL Subscriptions", () => { ], path: ["result"], }, - ]) - ); + ]), + }); + }); + + it("can continue receiving results after GraphQL errors when the connection remains open", async () => { + const link = new MockSubscriptionLink(); + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + + const obs = client.subscribe(options); + const stream = new ObservableStream(obs); + + link.simulateResult({ + result: { + data: null, + errors: [ + { + message: "This is an error", + locations: [ + { + column: 3, + line: 2, + }, + ], + path: ["result"], + }, + ], + }, + }); + + await expect(stream).toEmitStrictTyped({ + data: undefined, + error: new CombinedGraphQLErrors([ + { + message: "This is an error", + locations: [ + { + column: 3, + line: 2, + }, + ], + path: ["result"], + }, + ]), + }); + + link.simulateResult(results[0]); + + await expect(stream).toEmitStrictTyped({ data: results[0].result.data }); + }); + + it("emits a result with error if the result has network errors", async () => { + const link = new MockSubscriptionLink(); + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + + const obs = client.subscribe(options); + const stream = new ObservableStream(obs); + + link.simulateResult({ error: new Error("Oops") }); + + await expect(stream).toEmitStrictTyped({ + data: undefined, + error: new Error("Oops"), + }); + + await expect(stream).toComplete(); }); it('returns errors in next result when `errorPolicy` is "all"', async () => { @@ -191,7 +263,7 @@ describe("GraphQL Subscriptions", () => { } } `; - const link = mockObservableLink(); + const link = new MockSubscriptionLink(); const client = new ApolloClient({ link, cache: new InMemoryCache(), @@ -208,21 +280,43 @@ describe("GraphQL Subscriptions", () => { { result: { data: null, - errors: [new GraphQLError("This is an error")], + errors: [{ message: "This is an error" }], }, }, true ); - await expect(stream).toEmitFetchResult({ - data: null, - errors: [new GraphQLError("This is an error")], + await expect(stream).toEmitStrictTyped({ + data: undefined, + error: new CombinedGraphQLErrors([{ message: "This is an error" }]), }); await expect(stream).toComplete(); }); - it('throws protocol errors when `errorPolicy` is "all"', async () => { + it("emits a result with error and completes when the result has network errors with `errorPolicy: 'all'`", async () => { + const link = new MockSubscriptionLink(); + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + + const obs = client.subscribe({ ...options, errorPolicy: "all" }); + const stream = new ObservableStream(obs); + + link.simulateResult({ error: new Error("Oops") }); + + await expect(stream).toEmitStrictTyped({ + data: undefined, + error: new Error("Oops"), + }); + await expect(stream).toComplete(); + }); + + it('emits protocol errors when `errorPolicy` is "all"', async () => { + const { httpLink, enqueueProtocolErrors } = + mockMultipartSubscriptionStream(); + const query = gql` subscription UserInfo($name: String) { user(name: $name) { @@ -230,9 +324,8 @@ describe("GraphQL Subscriptions", () => { } } `; - const link = mockObservableLink(); const client = new ApolloClient({ - link, + link: httpLink, cache: new InMemoryCache(), }); @@ -246,38 +339,31 @@ describe("GraphQL Subscriptions", () => { // Silence expected warning about missing field for cache write using _consoleSpy = spyOnConsole("warn"); - link.simulateResult( + enqueueProtocolErrors([ { - result: { - data: null, - extensions: { - [PROTOCOL_ERRORS_SYMBOL]: new CombinedProtocolErrors([ - { - message: "cannot read message from websocket", - extensions: { - code: "WEBSOCKET_MESSAGE_ERROR", - }, - }, - ]), - }, + message: "cannot read message from websocket", + extensions: { + code: "WEBSOCKET_MESSAGE_ERROR", }, }, - true - ); + ]); - await expect(stream).toEmitError( - new CombinedProtocolErrors([ + await expect(stream).toEmitStrictTyped({ + data: undefined, + error: new CombinedProtocolErrors([ { message: "cannot read message from websocket", extensions: { code: "WEBSOCKET_MESSAGE_ERROR", }, }, - ]) - ); + ]), + }); + + await expect(stream).toComplete(); }); - it('strips errors in next result when `errorPolicy` is "ignore"', async () => { + it('does not emit anything for GraphQL errors with no data in next result when `errorPolicy` is "ignore"', async () => { const query = gql` subscription UserInfo($name: String) { user(name: $name) { @@ -285,7 +371,7 @@ describe("GraphQL Subscriptions", () => { } } `; - const link = mockObservableLink(); + const link = new MockSubscriptionLink(); const client = new ApolloClient({ link, cache: new InMemoryCache(), @@ -298,21 +384,20 @@ describe("GraphQL Subscriptions", () => { }); const stream = new ObservableStream(obs); - link.simulateResult( - { - result: { - data: null, - errors: [new GraphQLError("This is an error")], - }, + link.simulateResult({ + result: { + data: null, + errors: [{ message: "This is an error" }], }, - true - ); + }); + + await expect(stream).not.toEmitAnything(); - await expect(stream).toEmitFetchResult({ data: null }); + link.simulateComplete(); await expect(stream).toComplete(); }); - it('throws protocol errors when `errorPolicy` is "ignore"', async () => { + it('does not emit anything for network errors with no data in next result when `errorPolicy` is "ignore"', async () => { const query = gql` subscription UserInfo($name: String) { user(name: $name) { @@ -320,7 +405,7 @@ describe("GraphQL Subscriptions", () => { } } `; - const link = mockObservableLink(); + const link = new MockSubscriptionLink(); const client = new ApolloClient({ link, cache: new InMemoryCache(), @@ -333,42 +418,50 @@ describe("GraphQL Subscriptions", () => { }); const stream = new ObservableStream(obs); + link.simulateResult({ error: new Error("Oops") }); + + await expect(stream).toComplete(); + }); + + it('does not emit anything and completes observable for protocolErrors when `errorPolicy` is "ignore"', async () => { + const { httpLink, enqueueProtocolErrors } = + mockMultipartSubscriptionStream(); + const query = gql` + subscription UserInfo($name: String) { + user(name: $name) { + name + } + } + `; + const client = new ApolloClient({ + link: httpLink, + cache: new InMemoryCache(), + }); + + const obs = client.subscribe({ + query, + variables: { name: "Iron Man" }, + errorPolicy: "ignore", + }); + const stream = new ObservableStream(obs); + // Silence expected warning about missing field for cache write using _consoleSpy = spyOnConsole("warn"); - link.simulateResult( + enqueueProtocolErrors([ { - result: { - data: null, - extensions: { - [PROTOCOL_ERRORS_SYMBOL]: new CombinedProtocolErrors([ - { - message: "cannot read message from websocket", - extensions: { - code: "WEBSOCKET_MESSAGE_ERROR", - }, - }, - ]), - }, + message: "cannot read message from websocket", + extensions: { + code: "WEBSOCKET_MESSAGE_ERROR", }, }, - true - ); + ]); - await expect(stream).toEmitError( - new CombinedProtocolErrors([ - { - message: "cannot read message from websocket", - extensions: { - code: "WEBSOCKET_MESSAGE_ERROR", - }, - }, - ]) - ); + await expect(stream).toComplete(); }); it("should call complete handler when the subscription completes", async () => { - const link = mockObservableLink(); + const link = new MockSubscriptionLink(); const client = new ApolloClient({ link, cache: new InMemoryCache(), @@ -382,7 +475,7 @@ describe("GraphQL Subscriptions", () => { }); it("should pass a context object through the link execution chain", async () => { - const link = mockObservableLink(); + const link = new MockSubscriptionLink(); const client = new ApolloClient({ cache: new InMemoryCache(), link, @@ -392,17 +485,19 @@ describe("GraphQL Subscriptions", () => { link.simulateResult(results[0]); - await expect(stream).toEmitFetchResult(results[0].result); + await expect(stream).toEmitStrictTyped(results[0].result); expect(link.operation?.getContext().someVar).toEqual( options.context.someVar ); }); - it("should throw an error if the result has protocolErrors on it", async () => { - const link = mockObservableLink(); + it("emits an error if the result has protocolErrors on it", async () => { + const { httpLink, enqueueProtocolErrors } = + mockMultipartSubscriptionStream(); + const client = new ApolloClient({ - link, + link: httpLink, cache: new InMemoryCache(), }); @@ -412,31 +507,25 @@ describe("GraphQL Subscriptions", () => { // Silence expected warning about missing field for cache write using _consoleSpy = spyOnConsole("warn"); - link.simulateResult({ - result: { - data: null, + enqueueProtocolErrors([ + { + message: "cannot read message from websocket", extensions: { - [PROTOCOL_ERRORS_SYMBOL]: new CombinedProtocolErrors([ - { - message: "cannot read message from websocket", - extensions: { - code: "WEBSOCKET_MESSAGE_ERROR", - }, - }, - ]), + code: "WEBSOCKET_MESSAGE_ERROR", }, }, - }); + ]); - await expect(stream).toEmitError( - new CombinedProtocolErrors([ + await expect(stream).toEmitStrictTyped({ + data: undefined, + error: new CombinedProtocolErrors([ { message: "cannot read message from websocket", extensions: { code: "WEBSOCKET_MESSAGE_ERROR", }, }, - ]) - ); + ]), + }); }); }); diff --git a/src/core/ApolloClient.ts b/src/core/ApolloClient.ts index e5ae4905a99..b421a6d59c4 100644 --- a/src/core/ApolloClient.ts +++ b/src/core/ApolloClient.ts @@ -7,7 +7,7 @@ import type { WatchFragmentOptions, WatchFragmentResult, } from "@apollo/client/cache"; -import type { FetchResult, GraphQLRequest } from "@apollo/client/link/core"; +import type { GraphQLRequest } from "@apollo/client/link/core"; import { ApolloLink, execute } from "@apollo/client/link/core"; import type { UriFunction } from "@apollo/client/link/http"; import { HttpLink } from "@apollo/client/link/http"; @@ -37,6 +37,7 @@ import type { RefetchQueriesOptions, RefetchQueriesResult, Resolvers, + SubscribeResult, } from "./types.js"; import type { MutationOptions, @@ -511,7 +512,7 @@ export class ApolloClient implements DataProxy { TVariables extends OperationVariables = OperationVariables, >( options: SubscriptionOptions - ): Observable>> { + ): Observable>> { const id = this.queryManager.generateQueryId(); return this.queryManager.startGraphQLSubscription(options).pipe( diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 8a8c548d8a9..d377715c93e 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -649,7 +649,19 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`, }) .subscribe({ next: (subscriptionData) => { - const { updateQuery } = options; + const { updateQuery, onError } = options; + const { error } = subscriptionData; + + if (error) { + if (onError) { + onError(error); + } else { + invariant.error("Unhandled GraphQL subscription error", error); + } + + return; + } + if (updateQuery) { this.updateQuery((previous, updateOptions) => updateQuery(previous, { @@ -661,13 +673,6 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`, ); } }, - error: (err: any) => { - if (options.onError) { - options.onError(err); - return; - } - invariant.error("Unhandled GraphQL subscription error", err); - }, }); this.subscriptions.add(subscription); diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index 40ebae7c430..9f379230b7c 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -5,6 +5,7 @@ import { catchError, concat, EMPTY, + filter, from, lastValueFrom, map, @@ -87,6 +88,7 @@ import type { MutationUpdaterFunction, OnQueryUpdated, OperationVariables, + SubscribeResult, } from "./types.js"; import type { ErrorPolicy, @@ -1040,7 +1042,7 @@ export class QueryManager { public startGraphQLSubscription( options: SubscriptionOptions - ): Observable> { + ): Observable> { let { query, variables } = options; const { fetchPolicy, @@ -1059,14 +1061,14 @@ export class QueryManager { variables, extensions ).pipe( - map((result) => { + map((rawResult): SubscribeResult => { if (fetchPolicy !== "no-cache") { // the subscription interface should handle not sending us results we no longer subscribe to. // XXX I don't think we ever send in an object with errors, but we might in the future... - if (shouldWriteResult(result, errorPolicy)) { + if (shouldWriteResult(rawResult, errorPolicy)) { this.cache.write({ query, - result: result.data, + result: rawResult.data, dataId: "ROOT_SUBSCRIPTION", variables: variables, }); @@ -1075,29 +1077,43 @@ export class QueryManager { this.broadcastQueries(); } - const hasErrors = graphQLResultHasError(result); - const hasProtocolErrors = graphQLResultHasProtocolErrors(result); + const result: SubscribeResult = { + data: rawResult.data ?? undefined, + }; - if (hasErrors && errorPolicy === "none") { - throw new CombinedGraphQLErrors(result.errors!); + if (graphQLResultHasError(rawResult)) { + result.error = new CombinedGraphQLErrors(rawResult.errors!); + } else if (graphQLResultHasProtocolErrors(rawResult)) { + result.error = rawResult.extensions[PROTOCOL_ERRORS_SYMBOL]; + // Don't emit protocol errors added by HttpLink + delete rawResult.extensions[PROTOCOL_ERRORS_SYMBOL]; + } + + if ( + rawResult.extensions && + Object.keys(rawResult.extensions).length + ) { + result.extensions = rawResult.extensions; } - if (hasProtocolErrors) { - // `errorPolicy` is a mechanism for handling GraphQL errors, according - // to our documentation, so we throw protocol errors regardless of the - // set error policy. - throw result.extensions[PROTOCOL_ERRORS_SYMBOL]; + if (result.error && errorPolicy === "none") { + result.data = undefined; } if (errorPolicy === "ignore") { - delete result.errors; + delete result.error; } return result; }), catchError((error) => { - throw maybeWrapError(error); - }) + if (errorPolicy === "ignore") { + return of({ data: undefined } as SubscribeResult); + } + + return of({ data: undefined, error: maybeWrapError(error) }); + }), + filter((result) => !!(result.data || result.error)) ); if (this.getDocumentInfo(query).hasClientExports) { @@ -1105,7 +1121,7 @@ export class QueryManager { .addExportedVariables(query, variables, context) .then(makeObservable); - return new Observable>((observer) => { + return new Observable>((observer) => { let sub: Subscription | null = null; observablePromise.then( (observable) => (sub = observable.subscribe(observer)), diff --git a/src/core/types.ts b/src/core/types.ts index 05b52d2540f..ebf3195b4ce 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -244,3 +244,14 @@ export interface MutateResult { /** {@inheritDoc @apollo/client!MutationResultDocumentation#extensions:member} */ extensions?: Record; } + +export interface SubscribeResult { + /** {@inheritDoc @apollo/client!MutationResultDocumentation#data:member} */ + data: TData | undefined; + + /** {@inheritDoc @apollo/client!MutationResultDocumentation#error:member} */ + error?: ErrorLike; + + /** {@inheritDoc @apollo/client!MutationResultDocumentation#extensions:member} */ + extensions?: Record; +} diff --git a/src/react/hooks/__tests__/useSubscription.test.tsx b/src/react/hooks/__tests__/useSubscription.test.tsx index cbe2408b76d..dd93ddab95d 100644 --- a/src/react/hooks/__tests__/useSubscription.test.tsx +++ b/src/react/hooks/__tests__/useSubscription.test.tsx @@ -4,7 +4,7 @@ import { renderHookToSnapshotStream, } from "@testing-library/react-render-stream"; import { expectTypeOf } from "expect-type"; -import { GraphQLError } from "graphql"; +import { GraphQLFormattedError } from "graphql"; import { gql } from "graphql-tag"; import React from "react"; import { ErrorBoundary } from "react-error-boundary"; @@ -177,6 +177,112 @@ describe("useSubscription Hook", () => { ); }); + it("can continue to receive new results after an error", async () => { + const subscription = gql` + subscription { + car { + make + } + } + `; + + const results = ["Audi", "BMW", "Mercedes", "Hyundai"].map((make) => ({ + result: { data: { car: { make } } }, + })); + + const errorResult = { + result: { data: { car: { make: null } }, errors: [{ message: "test" }] }, + }; + + const link = new MockSubscriptionLink(); + const client = new ApolloClient({ + link, + cache: new Cache(), + }); + + const onError = jest.fn(); + const onData = jest.fn(); + const onComplete = jest.fn(); + using _disabledAct = disableActEnvironment(); + const { takeSnapshot } = await renderHookToSnapshotStream( + () => useSubscription(subscription, { onError, onData, onComplete }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + await expect(takeSnapshot()).resolves.toEqualStrictTyped({ + data: undefined, + error: undefined, + loading: true, + variables: undefined, + }); + + link.simulateResult(results[0]); + + await expect(takeSnapshot()).resolves.toEqualStrictTyped({ + data: results[0].result.data, + error: undefined, + loading: false, + variables: undefined, + }); + + expect(onData).toHaveBeenCalledTimes(1); + expect(onData).toHaveBeenLastCalledWith({ + client, + data: { + data: results[0].result.data, + error: undefined, + loading: false, + variables: undefined, + }, + }); + expect(onError).toHaveBeenCalledTimes(0); + expect(onComplete).toHaveBeenCalledTimes(0); + + link.simulateResult(errorResult); + + await expect(takeSnapshot()).resolves.toEqualStrictTyped({ + data: undefined, + error: new CombinedGraphQLErrors([{ message: "test" }]), + loading: false, + variables: undefined, + }); + + expect(onData).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenLastCalledWith( + new CombinedGraphQLErrors([{ message: "test" }]) + ); + expect(onComplete).toHaveBeenCalledTimes(0); + + link.simulateResult(results[1]); + + await expect(takeSnapshot()).resolves.toEqualStrictTyped({ + data: results[1].result.data, + error: undefined, + loading: false, + variables: undefined, + }); + + expect(onData).toHaveBeenCalledTimes(2); + expect(onData).toHaveBeenLastCalledWith({ + client, + data: { + data: results[1].result.data, + error: undefined, + loading: false, + variables: undefined, + }, + }); + expect(onError).toHaveBeenCalledTimes(1); + expect(onComplete).toHaveBeenCalledTimes(0); + + await expect(takeSnapshot).not.toRerender(); + }); + it("should call onComplete after subscription is complete", async () => { const subscription = gql` subscription { @@ -730,8 +836,6 @@ describe("useSubscription Hook", () => { }); it("should handle immediate completions gracefully", async () => { - using consoleSpy = spyOnConsole("error"); - const subscription = gql` subscription { car { @@ -764,27 +868,22 @@ describe("useSubscription Hook", () => { }); // Simulating the behavior of HttpLink, which calls next and complete in sequence. - link.simulateResult({ result: { data: null } }, /* complete */ true); + link.simulateResult( + { result: { data: { car: { __typename: "Car", make: "Audi" } } } }, + /* complete */ true + ); await expect(takeSnapshot()).resolves.toEqualStrictTyped({ - data: null, + data: { car: { __typename: "Car", make: "Audi" } }, error: undefined, loading: false, variables: undefined, }); await expect(takeSnapshot).not.toRerender(); - - expect(consoleSpy.error).toHaveBeenCalledTimes(1); - expect(consoleSpy.error.mock.calls[0]).toStrictEqual([ - "Missing field '%s' while writing result %o", - "car", - {}, - ]); }); it("should handle immediate completions with multiple subscriptions gracefully", async () => { - using consoleSpy = spyOnConsole("error"); const subscription = gql` subscription { car { @@ -839,14 +938,17 @@ describe("useSubscription Hook", () => { } // Simulating the behavior of HttpLink, which calls next and complete in sequence. - link.simulateResult({ result: { data: null } }, /* complete */ true); + link.simulateResult( + { result: { data: { car: { __typename: "Car", make: "Audi" } } } }, + /* complete */ true + ); if (IS_REACT_17) { { const { sub1, sub2, sub3 } = await takeSnapshot(); expect(sub1).toEqualStrictTyped({ - data: null, + data: { car: { __typename: "Car", make: "Audi" } }, error: undefined, loading: false, variables: undefined, @@ -871,14 +973,14 @@ describe("useSubscription Hook", () => { const { sub1, sub2, sub3 } = await takeSnapshot(); expect(sub1).toEqualStrictTyped({ - data: null, + data: { car: { __typename: "Car", make: "Audi" } }, error: undefined, loading: false, variables: undefined, }); expect(sub2).toEqualStrictTyped({ - data: null, + data: { car: { __typename: "Car", make: "Audi" } }, error: undefined, loading: false, variables: undefined, @@ -897,21 +999,21 @@ describe("useSubscription Hook", () => { const { sub1, sub2, sub3 } = await takeSnapshot(); expect(sub1).toEqualStrictTyped({ - data: null, + data: { car: { __typename: "Car", make: "Audi" } }, error: undefined, loading: false, variables: undefined, }); expect(sub2).toEqualStrictTyped({ - data: null, + data: { car: { __typename: "Car", make: "Audi" } }, error: undefined, loading: false, variables: undefined, }); expect(sub3).toEqualStrictTyped({ - data: null, + data: { car: { __typename: "Car", make: "Audi" } }, error: undefined, loading: false, variables: undefined, @@ -919,23 +1021,6 @@ describe("useSubscription Hook", () => { } await expect(takeSnapshot).not.toRerender(); - - expect(consoleSpy.error).toHaveBeenCalledTimes(3); - expect(consoleSpy.error.mock.calls[0]).toStrictEqual([ - "Missing field '%s' while writing result %o", - "car", - {}, - ]); - expect(consoleSpy.error.mock.calls[1]).toStrictEqual([ - "Missing field '%s' while writing result %o", - "car", - {}, - ]); - expect(consoleSpy.error.mock.calls[2]).toStrictEqual([ - "Missing field '%s' while writing result %o", - "car", - {}, - ]); }); test("should warn when using 'onSubscriptionData' and 'onData' together", () => { @@ -1640,7 +1725,7 @@ followed by new in-flight setup", async () => { }); describe("protocol error", () => { - it.each([undefined, "none", "all", "ignore"] as const)( + it.each([undefined, "none", "all"] as const)( "`errorPolicy: '%s'`: returns `{ error }`, calls `onError`", async (errorPolicy) => { const { httpLink, enqueueProtocolErrors } = @@ -1659,11 +1744,17 @@ followed by new in-flight setup", async () => { const onData = jest.fn(); const onError = jest.fn(); + const onComplete = jest.fn(); using _disabledAct = disableActEnvironment(); const { takeSnapshot } = await renderHookToSnapshotStream( () => - useSubscription(subscription, { errorPolicy, onError, onData }), + useSubscription(subscription, { + errorPolicy, + onError, + onData, + onComplete, + }), { wrapper: ({ children }) => ( {children} @@ -1696,8 +1787,61 @@ followed by new in-flight setup", async () => { expect(onError).toHaveBeenCalledTimes(1); expect(onError).toHaveBeenCalledWith(expectedError); expect(onData).toHaveBeenCalledTimes(0); + expect(onComplete).toHaveBeenCalledTimes(1); } ); + + it("`errorPolicy: 'ignore'`: does not rerender, calls `onComplete`", async () => { + const { httpLink, enqueueProtocolErrors } = + mockMultipartSubscriptionStream(); + + const subscription: TypedDocumentNode<{ totalLikes: number }, {}> = gql` + subscription ($id: ID!) { + totalLikes + } + `; + const client = new ApolloClient({ + link: httpLink, + cache: new Cache(), + }); + + const onData = jest.fn(); + const onError = jest.fn(); + const onComplete = jest.fn(); + + using _disabledAct = disableActEnvironment(); + const { takeSnapshot } = await renderHookToSnapshotStream( + () => + useSubscription(subscription, { + errorPolicy: "ignore", + onError, + onData, + onComplete, + }), + { + wrapper: ({ children }) => ( + {children} + ), + } + ); + + await expect(takeSnapshot()).resolves.toEqualStrictTyped({ + data: undefined, + error: undefined, + loading: true, + variables: undefined, + }); + + enqueueProtocolErrors([ + { message: "Socket closed with event -1: I'm a test!" }, + ]); + + await expect(takeSnapshot).not.toRerender(); + + expect(onError).toHaveBeenCalledTimes(0); + expect(onData).toHaveBeenCalledTimes(0); + expect(onComplete).toHaveBeenCalledTimes(1); + }); }); }); }); @@ -2010,7 +2154,7 @@ describe("`restart` callback", () => { }); } - const error = new GraphQLError("error"); + const error: GraphQLFormattedError = { message: "error" }; link.simulateResult({ result: { errors: [error] }, }); @@ -2026,7 +2170,7 @@ describe("`restart` callback", () => { } await expect(takeSnapshot).not.toRerender({ timeout: 20 }); - expect(onUnsubscribe).toHaveBeenCalledTimes(1); + expect(onUnsubscribe).toHaveBeenCalledTimes(0); expect(onSubscribe).toHaveBeenCalledTimes(1); getCurrentSnapshot().restart(); @@ -2042,6 +2186,7 @@ describe("`restart` callback", () => { } await waitFor(() => expect(onSubscribe).toHaveBeenCalledTimes(2)); + await tick(); expect(onUnsubscribe).toHaveBeenCalledTimes(1); link.simulateResult({ result: { data: { totalLikes: 2 } } }); @@ -2138,38 +2283,36 @@ describe("ignoreResults", () => { }); link.simulateResult(results[0]); + await tick(); - await waitFor(() => { - expect(onData).toHaveBeenCalledTimes(1); - expect(onData).toHaveBeenLastCalledWith({ - client, - data: { - data: results[0].result.data, - error: undefined, - loading: false, - variables: undefined, - }, - }); - expect(onError).toHaveBeenCalledTimes(0); - expect(onComplete).toHaveBeenCalledTimes(0); + expect(onData).toHaveBeenCalledTimes(1); + expect(onData).toHaveBeenLastCalledWith({ + client, + data: { + data: results[0].result.data, + error: undefined, + loading: false, + variables: undefined, + }, }); + expect(onError).toHaveBeenCalledTimes(0); + expect(onComplete).toHaveBeenCalledTimes(0); link.simulateResult(results[1], true); + await tick(); - await waitFor(() => { - expect(onData).toHaveBeenCalledTimes(2); - expect(onData).toHaveBeenLastCalledWith({ - client, - data: { - data: results[1].result.data, - error: undefined, - loading: false, - variables: undefined, - }, - }); - expect(onError).toHaveBeenCalledTimes(0); - expect(onComplete).toHaveBeenCalledTimes(1); + expect(onData).toHaveBeenCalledTimes(2); + expect(onData).toHaveBeenLastCalledWith({ + client, + data: { + data: results[1].result.data, + error: undefined, + loading: false, + variables: undefined, + }, }); + expect(onError).toHaveBeenCalledTimes(0); + expect(onComplete).toHaveBeenCalledTimes(1); await expect(takeSnapshot).not.toRerender(); }); @@ -2211,31 +2354,29 @@ describe("ignoreResults", () => { }); link.simulateResult(results[0]); + await tick(); - await waitFor(() => { - expect(onData).toHaveBeenCalledTimes(1); - expect(onData).toHaveBeenLastCalledWith({ - client, - data: { - data: results[0].result.data, - error: undefined, - loading: false, - variables: undefined, - }, - }); - expect(onError).toHaveBeenCalledTimes(0); - expect(onComplete).toHaveBeenCalledTimes(0); + expect(onData).toHaveBeenCalledTimes(1); + expect(onData).toHaveBeenLastCalledWith({ + client, + data: { + data: results[0].result.data, + error: undefined, + loading: false, + variables: undefined, + }, }); + expect(onError).toHaveBeenCalledTimes(0); + expect(onComplete).toHaveBeenCalledTimes(0); const error = new Error("test"); link.simulateResult({ error }); + await tick(); - await waitFor(() => { - expect(onData).toHaveBeenCalledTimes(1); - expect(onError).toHaveBeenCalledTimes(1); - expect(onError).toHaveBeenLastCalledWith(error); - expect(onComplete).toHaveBeenCalledTimes(0); - }); + expect(onData).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenLastCalledWith(error); + expect(onComplete).toHaveBeenCalledTimes(1); await expect(takeSnapshot).not.toRerender(); }); diff --git a/src/react/hooks/useSubscription.ts b/src/react/hooks/useSubscription.ts index ceac64e932c..4cb1f6aa35d 100644 --- a/src/react/hooks/useSubscription.ts +++ b/src/react/hooks/useSubscription.ts @@ -10,10 +10,9 @@ import type { ErrorLike, ErrorPolicy, FetchPolicy, - FetchResult, OperationVariables, + SubscribeResult, } from "@apollo/client/core"; -import { CombinedGraphQLErrors } from "@apollo/client/errors"; import type { MaybeMasked } from "@apollo/client/masking"; import { DocumentType, verifyDocumentType } from "@apollo/client/react/parser"; import type { NoInfer } from "@apollo/client/utilities"; @@ -87,6 +86,8 @@ export declare namespace useSubscription { /** {@inheritDoc @apollo/client!SubscriptionResultDocumentation#error:member} */ error?: ErrorLike; + restart: () => void; + // This was added by the legacy useSubscription type, and is tested in unit // tests, but probably shouldn’t be added to the result. /** @@ -96,17 +97,24 @@ export declare namespace useSubscription { variables?: TVariables; } + export type OnDataResult = Omit, "restart">; + export interface OnDataOptions { client: ApolloClient; - data: Result; + data: OnDataResult; } export interface OnSubscriptionDataOptions { client: ApolloClient; - subscriptionData: Result; + subscriptionData: OnDataResult; } } +type StateResult = Omit< + useSubscription.Result, + "restart" +>; + /** * > Refer to the [Subscriptions](https://www.apollographql.com/docs/react/data/subscriptions/) section for a more in-depth overview of `useSubscription`. * @@ -194,7 +202,7 @@ export function useSubscription< >( subscription: DocumentNode | TypedDocumentNode, options: useSubscription.Options, NoInfer> = {} -) { +): useSubscription.Result { const hasIssuedDeprecationWarningRef = React.useRef(false); const client = useApolloClient(options.client); verifyDocumentType(subscription, DocumentType.Subscription); @@ -276,9 +284,7 @@ export function useSubscription< }); const fallbackLoading = !skip && !ignoreResults; - const fallbackResult = React.useMemo< - useSubscription.Result - >( + const fallbackResult = React.useMemo>( () => ({ loading: fallbackLoading, error: void 0, @@ -301,7 +307,7 @@ export function useSubscription< ignoreResultsRef.current = ignoreResults; }); - const ret = useSyncExternalStore>( + const ret = useSyncExternalStore>( React.useCallback( (update) => { if (!observable) { @@ -312,22 +318,18 @@ export function useSubscription< const variables = observable.__.variables; const client = observable.__.client; const subscription = observable.subscribe({ - next(fetchResult) { + next(value) { if (subscriptionStopped) { return; } - const result = { + const result: StateResult = { loading: false, - // TODO: fetchResult.data can be null but SubscriptionResult.data - // expects TData | undefined only - data: fetchResult.data!, - error: - fetchResult.errors ? - new CombinedGraphQLErrors(fetchResult.errors) - : undefined, + data: value.data, + error: value.error, variables, }; + observable.__.setResult(result); if (!ignoreResultsRef.current) update(); @@ -345,18 +347,6 @@ export function useSubscription< }); } }, - error(error) { - if (!subscriptionStopped) { - observable.__.setResult({ - loading: false, - data: void 0, - error, - variables, - }); - if (!ignoreResultsRef.current) update(); - optionsRef.current.onError?.(error); - } - }, complete() { if (!subscriptionStopped) { if (optionsRef.current.onComplete) { @@ -426,15 +416,15 @@ function createSubscription< data: void 0, error: void 0, variables, - } as useSubscription.Result, - setResult(result: useSubscription.Result) { + } as StateResult, + setResult(result: StateResult) { __.result = result; }, }; - let observable: Observable>> | null = null; + let observable: Observable>> | null = null; return Object.assign( - new Observable>>((observer) => { + new Observable>>((observer) => { // lazily start the subscription when the first observer subscribes // to get around strict mode if (!observable) { diff --git a/src/testing/matchers/index.d.ts b/src/testing/matchers/index.d.ts index 2598c70c828..be5efa33143 100644 --- a/src/testing/matchers/index.d.ts +++ b/src/testing/matchers/index.d.ts @@ -59,6 +59,7 @@ interface ApolloCustomMatchers { (options?: TakeOptions) => Promise : { error: "matcher needs to be called on an ObservableStream instance" }; + /** @deprecated Use `toEmitStrictTyped` instead */ toEmitApolloQueryResult: T extends ObservableStream ? QueryResult extends ApolloQueryResult ? (value: ApolloQueryResult, options?: TakeOptions) => Promise @@ -73,6 +74,7 @@ interface ApolloCustomMatchers { (error?: any, options?: TakeOptions) => Promise : { error: "matcher needs to be called on an ObservableStream instance" }; + /** @deprecated Use `toEmitStrictTyped` instead */ toEmitFetchResult: T extends ObservableStream> ? (value: FetchResult, options?: TakeOptions) => Promise : { @@ -87,14 +89,17 @@ interface ApolloCustomMatchers { (options?: TakeOptions) => Promise : { error: "matcher needs to be called on an ObservableStream instance" }; + /** @deprecated Use `toEmitStrictTyped` instead */ toEmitValue: T extends ObservableStream ? (value: any, options?: TakeOptions) => Promise : { error: "matcher needs to be called on an ObservableStream instance" }; + /** @deprecated Use `toEmitStrictTyped` instead */ toEmitValueStrict: T extends ObservableStream ? (value: any, options?: TakeOptions) => Promise : { error: "matcher needs to be called on an ObservableStream instance" }; + /** @deprecated Use `toEmitStrictTyped` instead */ toEmitMatchedValue: T extends ObservableStream ? (value: any, options?: TakeOptions) => Promise : { error: "matcher needs to be called on an ObservableStream instance" }; @@ -133,6 +138,13 @@ interface ApolloCustomMatchers { (expected: FetchResult) => R : { error: "matchers needs to be called on a FetchResult" }; + toEmitStrictTyped: T extends ObservableStream ? + ( + expected: FilterUnserializableProperties, + options?: TakeOptions + ) => Promise + : { error: "toEmitStrictTyped needs to be called on an ObservableStream" }; + toEqualStrictTyped: T extends Promise ? (expected: FilterUnserializableProperties) => R : (expected: FilterUnserializableProperties) => R; diff --git a/src/testing/matchers/index.ts b/src/testing/matchers/index.ts index 721f9814fcc..2808d570456 100644 --- a/src/testing/matchers/index.ts +++ b/src/testing/matchers/index.ts @@ -9,6 +9,7 @@ import { toEmitError } from "./toEmitError.js"; import { toEmitFetchResult } from "./toEmitFetchResult.js"; import { toEmitMatchedValue } from "./toEmitMatchedValue.js"; import { toEmitNext } from "./toEmitNext.js"; +import { toEmitStrictTyped } from "./toEmitStrictTyped.js"; import { toEmitValue } from "./toEmitValue.js"; import { toEmitValueStrict } from "./toEmitValueStrict.js"; import { toEqualApolloQueryResult } from "./toEqualApolloQueryResult.js"; @@ -27,6 +28,7 @@ expect.extend({ toEmitFetchResult, toEmitMatchedValue, toEmitNext, + toEmitStrictTyped, toEmitValue, toEmitValueStrict, toEqualApolloQueryResult, diff --git a/src/testing/matchers/toEmitStrictTyped.ts b/src/testing/matchers/toEmitStrictTyped.ts new file mode 100644 index 00000000000..21a214a9ae7 --- /dev/null +++ b/src/testing/matchers/toEmitStrictTyped.ts @@ -0,0 +1,69 @@ +import { iterableEquality } from "@jest/expect-utils"; +import type { MatcherFunction } from "expect"; + +import type { ObservableStream } from "../internal/index.js"; +import type { TakeOptions } from "../internal/ObservableStream.js"; + +import { getSerializableProperties } from "./utils/getSerializableProperties.js"; + +export const toEmitStrictTyped: MatcherFunction< + [value: any, options?: TakeOptions] +> = async function (actual, expected, options) { + const stream = actual as ObservableStream; + const hint = this.utils.matcherHint( + this.isNot ? ".not.toEmitStrictTyped" : "toEmitStrictTyped", + "stream", + "expected", + { isNot: this.isNot } + ); + + try { + const value = await stream.takeNext(options); + const serializableProperties = getSerializableProperties(value); + + const pass = this.equals( + serializableProperties, + expected, + // https://github.com/jestjs/jest/blob/22029ba06b69716699254bb9397f2b3bc7b3cf3b/packages/expect/src/matchers.ts#L62-L67 + [...this.customTesters, iterableEquality], + true + ); + + return { + pass, + message: () => { + if (pass) { + return ( + hint + + "\n\nExpected stream not to emit a fetch result equal to expected but it did." + ); + } + + return ( + hint + + "\n\n" + + this.utils.printDiffOrStringify( + expected, + serializableProperties, + "Expected", + "Received", + true + ) + ); + }, + }; + } catch (error) { + if ( + error instanceof Error && + error.message === "Timeout waiting for next event" + ) { + return { + pass: false, + message: () => + hint + "\n\nExpected stream to emit a value but it did not.", + }; + } else { + throw error; + } + } +}; diff --git a/src/testing/matchers/toEqualStrictTyped.ts b/src/testing/matchers/toEqualStrictTyped.ts index 0873642c6c4..6f543c313bd 100644 --- a/src/testing/matchers/toEqualStrictTyped.ts +++ b/src/testing/matchers/toEqualStrictTyped.ts @@ -1,33 +1,7 @@ import { iterableEquality } from "@jest/expect-utils"; import type { MatcherFunction } from "expect"; -import { ApolloClient, ObservableQuery } from "@apollo/client/core"; -import { isPlainObject } from "@apollo/client/utilities"; - -function isKnownClassInstance(value: unknown) { - return [ApolloClient, ObservableQuery].some((c) => value instanceof c); -} - -function getSerializableProperties(obj: unknown): any { - if (Array.isArray(obj)) { - return obj.map((item) => getSerializableProperties(item)); - } - - if (isPlainObject(obj)) { - return Object.entries(obj).reduce( - (memo, [key, value]) => { - if (typeof value === "function" || isKnownClassInstance(value)) { - return memo; - } - - return { ...memo, [key]: value }; - }, - {} as Record - ); - } - - return obj; -} +import { getSerializableProperties } from "./utils/getSerializableProperties.js"; export const toEqualStrictTyped: MatcherFunction<[value: any]> = function ( actual, diff --git a/src/testing/matchers/utils/getSerializableProperties.ts b/src/testing/matchers/utils/getSerializableProperties.ts new file mode 100644 index 00000000000..deedd727017 --- /dev/null +++ b/src/testing/matchers/utils/getSerializableProperties.ts @@ -0,0 +1,27 @@ +import { ApolloClient, ObservableQuery } from "@apollo/client/core"; +import { isPlainObject } from "@apollo/client/utilities"; + +function isKnownClassInstance(value: unknown) { + return [ApolloClient, ObservableQuery].some((c) => value instanceof c); +} + +export function getSerializableProperties(obj: unknown): any { + if (Array.isArray(obj)) { + return obj.map((item) => getSerializableProperties(item)); + } + + if (isPlainObject(obj)) { + return Object.entries(obj).reduce( + (memo, [key, value]) => { + if (typeof value === "function" || isKnownClassInstance(value)) { + return memo; + } + + return { ...memo, [key]: value }; + }, + {} as Record + ); + } + + return obj; +}