From 5db1659dc07e3de697894fc1c6f00a151d068291 Mon Sep 17 00:00:00 2001
From: Jerel Miller <jerelmiller@gmail.com>
Date: Mon, 5 Aug 2024 11:28:14 -0600
Subject: [PATCH] Fix issue where multiple fetches might report `data` if
 result contained errors (#11984)

---
 .changeset/fluffy-impalas-cross.md          |  5 ++
 .size-limits.json                           |  4 +-
 src/core/QueryManager.ts                    | 13 +++-
 src/react/hooks/__tests__/useQuery.test.tsx | 81 ++++++++++++++++++++-
 4 files changed, 98 insertions(+), 5 deletions(-)
 create mode 100644 .changeset/fluffy-impalas-cross.md

diff --git a/.changeset/fluffy-impalas-cross.md b/.changeset/fluffy-impalas-cross.md
new file mode 100644
index 00000000000..55f412a7428
--- /dev/null
+++ b/.changeset/fluffy-impalas-cross.md
@@ -0,0 +1,5 @@
+---
+"@apollo/client": patch
+---
+
+Fix an issue where multiple fetches with results that returned errors would sometimes set the `data` property with an `errorPolicy` of `none`.
diff --git a/.size-limits.json b/.size-limits.json
index 0e52c9ae9c7..e5433a23665 100644
--- a/.size-limits.json
+++ b/.size-limits.json
@@ -1,4 +1,4 @@
 {
-  "dist/apollo-client.min.cjs": 40243,
-  "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 33041
+  "dist/apollo-client.min.cjs": 40252,
+  "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 33052
 }
diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts
index 6ba645285c8..ea91c0abdea 100644
--- a/src/core/QueryManager.ts
+++ b/src/core/QueryManager.ts
@@ -1176,11 +1176,12 @@ export class QueryManager<TStore> {
       (result) => {
         const graphQLErrors = getGraphQLErrorsFromResult(result);
         const hasErrors = graphQLErrors.length > 0;
+        const { errorPolicy } = options;
 
         // If we interrupted this request by calling getResultsFromLink again
         // with the same QueryInfo object, we ignore the old results.
         if (requestId >= queryInfo.lastRequestId) {
-          if (hasErrors && options.errorPolicy === "none") {
+          if (hasErrors && errorPolicy === "none") {
             // Throwing here effectively calls observer.error.
             throw queryInfo.markError(
               new ApolloError({
@@ -1206,7 +1207,15 @@ export class QueryManager<TStore> {
           networkStatus: NetworkStatus.ready,
         };
 
-        if (hasErrors && options.errorPolicy !== "ignore") {
+        // In the case we start multiple network requests simulatenously, we
+        // want to ensure we properly set `data` if we're reporting on an old
+        // result which will not be caught by the conditional above that ends up
+        // throwing the markError result.
+        if (hasErrors && errorPolicy === "none") {
+          aqr.data = void 0 as TData;
+        }
+
+        if (hasErrors && errorPolicy !== "ignore") {
           aqr.errors = graphQLErrors;
           aqr.networkStatus = NetworkStatus.error;
         }
diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx
index 7c87e28a92f..01a3d35e50f 100644
--- a/src/react/hooks/__tests__/useQuery.test.tsx
+++ b/src/react/hooks/__tests__/useQuery.test.tsx
@@ -1,5 +1,5 @@
 import React, { Fragment, ReactNode, useEffect, useRef, useState } from "react";
-import { DocumentNode, GraphQLError } from "graphql";
+import { DocumentNode, GraphQLError, GraphQLFormattedError } from "graphql";
 import gql from "graphql-tag";
 import { act } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
@@ -10081,6 +10081,85 @@ describe("useQuery Hook", () => {
       }
     );
   });
+
+  // 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`
+      query {
+        user {
+          id
+          name
+        }
+      }
+    `;
+
+    const graphQLError: GraphQLFormattedError = { message: "Cannot get name" };
+
+    const mocks = [
+      {
+        request: { query },
+        result: {
+          data: { user: { __typename: "User", id: "1", name: null } },
+          errors: [graphQLError],
+        },
+        delay: 10,
+        maxUsageCount: Number.POSITIVE_INFINITY,
+      },
+    ];
+
+    const ProfiledHook = profileHook(() =>
+      useQuery(query, { notifyOnNetworkStatusChange: true })
+    );
+
+    render(<ProfiledHook />, {
+      wrapper: ({ children }) => (
+        <MockedProvider mocks={mocks}>{children}</MockedProvider>
+      ),
+    });
+
+    {
+      const { loading, data, error } = await ProfiledHook.takeSnapshot();
+
+      expect(loading).toBe(true);
+      expect(data).toBeUndefined();
+      expect(error).toBeUndefined();
+    }
+
+    {
+      const { loading, data, error } = await ProfiledHook.takeSnapshot();
+
+      expect(loading).toBe(false);
+      expect(data).toBeUndefined();
+      expect(error).toEqual(new ApolloError({ graphQLErrors: [graphQLError] }));
+    }
+
+    const { refetch } = ProfiledHook.getCurrentSnapshot();
+
+    refetch().catch(() => {});
+    refetch().catch(() => {});
+
+    {
+      const { loading, networkStatus, data, error } =
+        await ProfiledHook.takeSnapshot();
+
+      expect(loading).toBe(true);
+      expect(data).toBeUndefined();
+      expect(networkStatus).toBe(NetworkStatus.refetch);
+      expect(error).toBeUndefined();
+    }
+
+    {
+      const { loading, networkStatus, data, error } =
+        await ProfiledHook.takeSnapshot();
+
+      expect(loading).toBe(false);
+      expect(data).toBeUndefined();
+      expect(networkStatus).toBe(NetworkStatus.error);
+      expect(error).toEqual(new ApolloError({ graphQLErrors: [graphQLError] }));
+    }
+
+    await expect(ProfiledHook).not.toRerender({ timeout: 200 });
+  });
 });
 
 describe.skip("Type Tests", () => {