From e609156c4989def88ae1a28b2e0f0378077a5528 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Tue, 6 Aug 2024 10:23:21 +0200 Subject: [PATCH] Fix a potential crash when calling `clearStore` while a query was running. (#11989) * Fix a potential crash when calling `clearStore` while a query was running. * size-limits * Update src/react/hooks/__tests__/useLazyQuery.test.tsx Co-authored-by: Jerel Miller * fix up merge * Clean up Prettier, Size-limit, and Api-Extractor --------- Co-authored-by: Jerel Miller Co-authored-by: phryneas --- .changeset/pink-guests-vanish.md | 11 ++++ .size-limits.json | 4 +- src/core/ObservableQuery.ts | 8 ++- .../hooks/__tests__/useLazyQuery.test.tsx | 64 +++++++++++++++++++ src/react/hooks/__tests__/useQuery.test.tsx | 48 ++++++++++++++ src/utilities/observables/Concast.ts | 2 +- 6 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 .changeset/pink-guests-vanish.md diff --git a/.changeset/pink-guests-vanish.md b/.changeset/pink-guests-vanish.md new file mode 100644 index 00000000000..df9044e07ea --- /dev/null +++ b/.changeset/pink-guests-vanish.md @@ -0,0 +1,11 @@ +--- +"@apollo/client": patch +--- + +Fix a potential crash when calling `clearStore` while a query was running. + +Previously, calling `client.clearStore()` while a query was running had one of these results: +* `useQuery` would stay in a `loading: true` state. +* `useLazyQuery` would stay in a `loading: true` state, but also crash with a `"Cannot read property 'data' of undefined"` error. + +Now, in both cases, the hook will enter an error state with a `networkError`, and the promise returned by the `useLazyQuery` `execute` function will return a result in an error state. diff --git a/.size-limits.json b/.size-limits.json index e5433a23665..161c5d01b7a 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 40252, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 33052 + "dist/apollo-client.min.cjs": 40271, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 33058 } diff --git a/src/core/ObservableQuery.ts b/src/core/ObservableQuery.ts index 1289a9735ef..c7bd4dec582 100644 --- a/src/core/ObservableQuery.ts +++ b/src/core/ObservableQuery.ts @@ -17,7 +17,7 @@ import { fixObservableSubclass, getQueryDefinition, } from "../utilities/index.js"; -import type { ApolloError } from "../errors/index.js"; +import { ApolloError, isApolloError } from "../errors/index.js"; import type { QueryManager } from "./QueryManager.js"; import type { ApolloQueryResult, @@ -974,6 +974,12 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`, }, error: (error) => { if (equal(this.variables, variables)) { + // Coming from `getResultsFromLink`, `error` here should always be an `ApolloError`. + // However, calling `concast.cancel` can inject another type of error, so we have to + // wrap it again here. + if (!isApolloError(error)) { + error = new ApolloError({ networkError: error }); + } finishWaitingForOwnResult(); this.reportError(error, variables); } diff --git a/src/react/hooks/__tests__/useLazyQuery.test.tsx b/src/react/hooks/__tests__/useLazyQuery.test.tsx index df38ea6adc0..e96dc5d09b0 100644 --- a/src/react/hooks/__tests__/useLazyQuery.test.tsx +++ b/src/react/hooks/__tests__/useLazyQuery.test.tsx @@ -25,6 +25,7 @@ import { import { useLazyQuery } from "../useLazyQuery"; import { QueryResult } from "../../types/types"; import { profileHook } from "../../../testing/internal"; +import { InvariantError } from "../../../utilities/globals"; describe("useLazyQuery Hook", () => { const helloQuery: TypedDocumentNode<{ @@ -1922,6 +1923,69 @@ describe("useLazyQuery Hook", () => { expect(options.fetchPolicy).toBe(defaultFetchPolicy); }); }); + + // regression for https://github.com/apollographql/apollo-client/issues/11988 + test("calling `clearStore` while a lazy query is running puts the hook into an error state and resolves the promise with an error result", async () => { + const link = new MockSubscriptionLink(); + let requests = 0; + link.onSetup(() => requests++); + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + const ProfiledHook = profileHook(() => useLazyQuery(helloQuery)); + render(, { + wrapper: ({ children }) => ( + {children} + ), + }); + + { + const [, result] = await ProfiledHook.takeSnapshot(); + expect(result.loading).toBe(false); + expect(result.data).toBeUndefined(); + } + const execute = ProfiledHook.getCurrentSnapshot()[0]; + + const promise = execute(); + expect(requests).toBe(1); + + { + const [, result] = await ProfiledHook.takeSnapshot(); + expect(result.loading).toBe(true); + expect(result.data).toBeUndefined(); + } + + client.clearStore(); + + const executionResult = await promise; + expect(executionResult.data).toBeUndefined(); + expect(executionResult.loading).toBe(true); + expect(executionResult.error).toEqual( + new ApolloError({ + networkError: new InvariantError( + "Store reset while query was in flight (not completed in link chain)" + ), + }) + ); + + { + const [, result] = await ProfiledHook.takeSnapshot(); + expect(result.loading).toBe(false); + expect(result.data).toBeUndefined(); + expect(result.error).toEqual( + new ApolloError({ + networkError: new InvariantError( + "Store reset while query was in flight (not completed in link chain)" + ), + }) + ); + } + + link.simulateResult({ result: { data: { hello: "Greetings" } } }, true); + await expect(ProfiledHook).not.toRerender({ timeout: 50 }); + expect(requests).toBe(1); + }); }); describe.skip("Type Tests", () => { diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index 01a3d35e50f..d521a96a0c3 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -10082,6 +10082,54 @@ describe("useQuery Hook", () => { ); }); + test("calling `clearStore` while a query is running puts the hook into an error state", async () => { + const query = gql` + query { + hello + } + `; + + const link = new MockSubscriptionLink(); + let requests = 0; + link.onSetup(() => requests++); + const client = new ApolloClient({ + link, + cache: new InMemoryCache(), + }); + const ProfiledHook = profileHook(() => useQuery(query)); + render(, { + wrapper: ({ children }) => ( + {children} + ), + }); + + expect(requests).toBe(1); + { + const result = await ProfiledHook.takeSnapshot(); + expect(result.loading).toBe(true); + expect(result.data).toBeUndefined(); + } + + client.clearStore(); + + { + const result = await ProfiledHook.takeSnapshot(); + expect(result.loading).toBe(false); + expect(result.data).toBeUndefined(); + expect(result.error).toEqual( + new ApolloError({ + networkError: new InvariantError( + "Store reset while query was in flight (not completed in link chain)" + ), + }) + ); + } + + link.simulateResult({ result: { data: { hello: "Greetings" } } }, true); + await expect(ProfiledHook).not.toRerender({ timeout: 50 }); + expect(requests).toBe(1); + }); + // https://github.com/apollographql/apollo-client/issues/11938 it("does not emit `data` on previous fetch when a 2nd fetch is kicked off and the result returns an error when errorPolicy is none", async () => { const query = gql` diff --git a/src/utilities/observables/Concast.ts b/src/utilities/observables/Concast.ts index 73c36520f8b..476b6dd639e 100644 --- a/src/utilities/observables/Concast.ts +++ b/src/utilities/observables/Concast.ts @@ -256,7 +256,7 @@ export class Concast extends Observable { public cancel = (reason: any) => { this.reject(reason); this.sources = []; - this.handlers.complete(); + this.handlers.error(reason); }; }