Skip to content

Commit

Permalink
Fix a potential crash when calling clearStore while a query was run…
Browse files Browse the repository at this point in the history
…ning. (#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 <[email protected]>

* fix up merge

* Clean up Prettier, Size-limit, and Api-Extractor

---------

Co-authored-by: Jerel Miller <[email protected]>
Co-authored-by: phryneas <[email protected]>
  • Loading branch information
3 people authored Aug 6, 2024
1 parent 8aa627f commit e609156
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 4 deletions.
11 changes: 11 additions & 0 deletions .changeset/pink-guests-vanish.md
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 2 additions & 2 deletions .size-limits.json
Original file line number Diff line number Diff line change
@@ -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
}
8 changes: 7 additions & 1 deletion src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
Expand Down
64 changes: 64 additions & 0 deletions src/react/hooks/__tests__/useLazyQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<{
Expand Down Expand Up @@ -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(<ProfiledHook />, {
wrapper: ({ children }) => (
<ApolloProvider client={client}>{children}</ApolloProvider>
),
});

{
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", () => {
Expand Down
48 changes: 48 additions & 0 deletions src/react/hooks/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<ProfiledHook />, {
wrapper: ({ children }) => (
<ApolloProvider client={client}>{children}</ApolloProvider>
),
});

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`
Expand Down
2 changes: 1 addition & 1 deletion src/utilities/observables/Concast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ export class Concast<T> extends Observable<T> {
public cancel = (reason: any) => {
this.reject(reason);
this.sources = [];
this.handlers.complete();
this.handlers.error(reason);
};
}

Expand Down

0 comments on commit e609156

Please sign in to comment.