Fix networkStatus incorrectly reported as ready when using errorPolicy: 'all' with errors (#12362)

Author: jerelmiller

.changeset/khaki-cheetahs-lick.md

@@ -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.

.size-limits.json

@@ -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
 }

src/core/ObservableQuery.ts

@@ -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 &&

src/core/__tests__/ObservableQuery.ts

@@ -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,
       });
@@ -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],
@@ -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,
       });
     });
 
@@ -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();
     });

src/react/hooks/__tests__/useLazyQuery.test.tsx

@@ -1533,6 +1533,8 @@ describe("useLazyQuery Hook", () => {
         variables: {},
       });
     }
+
+    await expect(takeSnapshot).not.toRerender();
   });
 
   it("the promise should not cause an unhandled rejection", async () => {

src/react/hooks/__tests__/useQuery.test.tsx

@@ -3971,7 +3971,7 @@ describe("useQuery Hook", () => {
           errors: [{ message: "error" }],
           called: true,
           loading: false,
-          networkStatus: NetworkStatus.ready,
+          networkStatus: NetworkStatus.error,
           previousData: undefined,
           variables: {},
         });
@@ -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: {},
         });
@@ -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: {},
         });
@@ -12025,7 +12025,7 @@ describe("useQuery Hook", () => {
         ],
         called: true,
         loading: false,
-        networkStatus: NetworkStatus.ready,
+        networkStatus: NetworkStatus.error,
         previousData: {
           hero: {
             heroFriends: [
@@ -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: {},
         });