Skip to content

Commit

Permalink
Fix networkStatus incorrectly reported as ready when using `error…
Browse files Browse the repository at this point in the history
…Policy: 'all'` with errors (#12362)
  • Loading branch information
jerelmiller authored Mar 7, 2025
1 parent 50f15a3 commit f6d387c
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 11 deletions.
7 changes: 7 additions & 0 deletions .changeset/khaki-cheetahs-lick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@apollo/client": patch
---

Fixes an issue where calling `observableQuery.getCurrentResult()` when the `errorPolicy` was set to `all` would return the `networkStatus` as `NetworkStatus.ready` when there were errors returned in the result. This has been corrected to report `NetworkStatus.error`.

This bug also affected the `useQuery` and `useLazyQuery` hooks and may affect you if you check for `networkStatus` in your component.
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": 42243,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34437
"dist/apollo-client.min.cjs": 42260,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34450
}
11 changes: 11 additions & 0 deletions src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,17 @@ export class ObservableQuery<
result.partial = true;
}

// We need to check for both both `error` and `errors` field because there
// are cases where sometimes `error` is set, but not `errors` and
// vice-versa. This will be updated in the next major version when
// `errors` is deprecated in favor of `error`.
if (
result.networkStatus === NetworkStatus.ready &&
(result.error || result.errors)
) {
result.networkStatus = NetworkStatus.error;
}

if (
__DEV__ &&
!diff.complete &&
Expand Down
12 changes: 8 additions & 4 deletions src/core/__tests__/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1003,7 +1003,7 @@ describe("ObservableQuery", () => {
data: undefined,
errors: [error],
loading: false,
networkStatus: NetworkStatus.ready,
networkStatus: NetworkStatus.error,
// TODO: This is not present on the emitted result so this should match
partial: true,
});
Expand Down Expand Up @@ -2279,6 +2279,7 @@ describe("ObservableQuery", () => {
const result = await observable.result();
const currentResult = observable.getCurrentResult();

// TODO: This should include an `error` property, not just `errors`
expect(result).toEqualApolloQueryResult({
data: dataOne,
errors: [error],
Expand All @@ -2289,9 +2290,7 @@ describe("ObservableQuery", () => {
data: dataOne,
errors: [error],
loading: false,
// TODO: The networkStatus returned here is different than the one
// returned from `observable.result()`. These should match
networkStatus: NetworkStatus.ready,
networkStatus: NetworkStatus.error,
});
});

Expand Down Expand Up @@ -2497,6 +2496,11 @@ describe("ObservableQuery", () => {
loading: false,
networkStatus: NetworkStatus.ready,
});
expect(observable.getCurrentResult()).toEqualApolloQueryResult({
data: dataTwo,
loading: false,
networkStatus: NetworkStatus.ready,
});

await expect(stream).not.toEmitAnything();
});
Expand Down
2 changes: 2 additions & 0 deletions src/react/hooks/__tests__/useLazyQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,8 @@ describe("useLazyQuery Hook", () => {
variables: {},
});
}

await expect(takeSnapshot).not.toRerender();
});

it("the promise should not cause an unhandled rejection", async () => {
Expand Down
10 changes: 5 additions & 5 deletions src/react/hooks/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3971,7 +3971,7 @@ describe("useQuery Hook", () => {
errors: [{ message: "error" }],
called: true,
loading: false,
networkStatus: NetworkStatus.ready,
networkStatus: NetworkStatus.error,
previousData: undefined,
variables: {},
});
Expand Down Expand Up @@ -4039,7 +4039,7 @@ describe("useQuery Hook", () => {
errors: [{ message: 'Could not fetch "hello"' }],
called: true,
loading: false,
networkStatus: NetworkStatus.ready,
networkStatus: NetworkStatus.error,
previousData: undefined,
variables: {},
});
Expand Down Expand Up @@ -4103,7 +4103,7 @@ describe("useQuery Hook", () => {
errors: [{ message: 'Could not fetch "hello"' }],
called: true,
loading: false,
networkStatus: NetworkStatus.ready,
networkStatus: NetworkStatus.error,
previousData: undefined,
variables: {},
});
Expand Down Expand Up @@ -12025,7 +12025,7 @@ describe("useQuery Hook", () => {
],
called: true,
loading: false,
networkStatus: NetworkStatus.ready,
networkStatus: NetworkStatus.error,
previousData: {
hero: {
heroFriends: [
Expand Down Expand Up @@ -13506,7 +13506,7 @@ describe("useQuery Hook", () => {
errors: [{ message: "Couldn't get name" }],
called: true,
loading: false,
networkStatus: NetworkStatus.ready,
networkStatus: NetworkStatus.error,
previousData: undefined,
variables: {},
});
Expand Down

0 comments on commit f6d387c

Please sign in to comment.