useQuery results gets out of sync after a network error

[magicmark]

I'm having trouble nailing down exactly what's going or how to describe it, but here goes:

Whenever there's a network error, the data attribute returned from useQuery "remembers" the previous result. And then things get a little weird, and useQuery appears to return the results from the previous network request?

This sounds a little crazy, and maybe this isn't accurately describing what's actually happening - repros are worth a 10000 words, so here's something I threw together to demonstrate the issue:

Runnable Codesandbox link: https://codesandbox.io/s/silly-jones-tcpez?file=/components/MakeQuery.js

Code

import { gql, useQuery, useApolloClient } from "@apollo/client";
import { useState, useEffect } from "react";

const GET_GREETING = gql`
  query {
    greeting
  }
`;

export default function MakeQuery() {
  const [networkError, setNetworkError] = useState(false);
  const [manualQueryResult, setManualQueryResult] = useState();
  const apolloClient = useApolloClient();

  const queryOptions = {
    fetchPolicy: "no-cache",
    context: {
      headers: {
        "x-network-error": networkError === true ? "1" : "0"
      }
    }
  };

  const { data, error, loading } = useQuery(GET_GREETING, queryOptions);
  useEffect(() => {
    apolloClient
      .query({
        ...queryOptions,
        query: GET_GREETING
      })
      .then((...results) => {
        setManualQueryResult(results);
      })
      .catch((...results) => {
        setManualQueryResult(results);
      });
  }, [apolloClient, networkError, queryOptions]);

  return (
    <div>
      <label htmlFor="errorToggle">Toggle network error</label>
      <input
        id="errorToggle"
        type="checkbox"
        name="errorToggle"
        checked={networkError}
        onChange={(event) => {
          setNetworkError(event.target.checked);
        }}
      />

      <h2>useQuery results:</h2>
      <pre>{JSON.stringify({ data, error, loading }, null, 2)}</pre>

      <h2>apolloClient.query results:</h2>
      <pre>{JSON.stringify(manualQueryResult, null, 2)}</pre>
    </div>
  );
}

My repro shows a component using both useQuery and the raw apolloClient.query method on the client instance. (The apolloClient.query result appears to show the truth, and what I'd expect useQuery to also show.

1. Here what the page looks like to start with - so far so good, both versions show a successful query response

   

2. Let's click the 'toggle network error' option to trigger a state change and re-render the component, and have the server return a 500:

   

   The manual query approach appears to show the right thing. useQuery shows some....accidentally cached data?

3. Now let's untoggle the network error...

   We should go back to the state shown in (1), but....

   

   🤔 🤔 🤔 looks like useQuery is now showing what it should've shown before? We're now in a state where useQuery is one step behind what it should actually be displaying. You can keep clicking the toggle - useQuery and apolloClient.query are now permanently out of sync.

It's very possible I'm misunderstanding how state is working here, and I'm doing something dumb - fwiw this came up trying to build https://apollo-visualizer.vercel.app/, in case anyone is interested in the use case. More generally though, I could imagine this causing problems when a network comes back online, or for retries or something.

Is this a bug? Or am I being dumb? Thanks!

Versions

  "dependencies": {
    "@apollo/client": "3.2.0",
    "@graphql-tools/schema": "6.2.3",
    "graphql": "15.3.0",
    "json-stringify-safe": "5.0.1",
    "next": "latest",
    "react": "^16.13.1",
    "react-dom": "^16.13.1"
  },

sandbox@sse-sandbox-tcpez:/sandbox$ npx envinfo@latest --preset apollo
npx: installed 1 in 0.682s

  System:
    OS: Linux 5.4 Debian GNU/Linux 10 (buster) 10 (buster)
  Binaries:
    Node: 10.21.0 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.4 - /usr/local/bin/npm
  npmPackages:
    @apollo/client: 3.2.0 => 3.2.0

[magicmark]

FWIW I'm working around this for now by recreating the apollo client instance on each re-rerender, to throw away all state

Here's a version of the minimal repro with the workaround applied: https://codesandbox.io/s/peaceful-lovelace-bfdgj?file=/components/MakeQuery.js - the UI displays what is expected, the two results stay in sync.

[hwillson]

@magicmark Can you double check your repro? It's working for me in each case:

Initial:

Toggle 1:

Toggle 2:

[magicmark]

@hwillson I think you might be viewing the second link I posted (which demonstrates a workaround) - check out the first link, in my initial message, which demonstrates the issue described - https://codesandbox.io/s/silly-jones-tcpez?file=/components/MakeQuery.js

[hwillson]

Oops! Sorry, I looked before coffee - I'll take a closer look.

[hwillson]

@magicmark If you drop in a quick console.log('render'); at the top of your MakeQuery component, and watch your browser console, I think you'll see the issue. Your component is trapped in an infinite re-render loop. This is happening because of the queryOptions object you're creating on each render, and setting this as a dependency on the useEffect hook that triggers the apolloClient.query call. Since the queryOptions object is new on each render, the query useEffect hook is called on each render, which is causing the results to be messed up. If you remove queryOptions from your useEffect deps array, or look into using something like useMemo with your queryOptions creation, this problem should go away.

[magicmark]

@hwillson Ah, good catch! Yes, this is a problem with my repro :( By removing queryOptions from the useEffect deps list as you suggest, we get the two versions back "in sync".

However.....this brings me back to the original eyebrow-raiser that I started creating the repro for:

Here's a version that avoids the infinite rerender loop https://codesandbox.io/s/staging-sun-9ij4e?file=/components/MakeQuery.js

Why does useQuery return "data" from a network error result? Notice the explicit "no-cache" policy.

Is this expected?

[hwillson]

Sorry for the delay here @magicmark - the issue you've outlined in #7045 (comment) is being tracked in #6905. Thanks!