Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useQuery: networkError overwrites graphQLErrors, and vice versa on refetch #8157

Closed
mogzol opened this issue May 6, 2021 · 4 comments
Closed
Assignees
Labels
🔍 investigate Investigate further

Comments

@mogzol
Copy link

mogzol commented May 6, 2021

Intended outcome:

I have a query which might cause the server to return a 401 status, with additional GraphQL errors in the body. I want to be able to get both the network status code, and the graphQL errors out of the result.

Actual outcome:

When I run the query, initially result.error contains the graphQLErrors field, but the networkError field is null. The component then immediately re-renders and now the networkError field is set, but the graphQLErrors field is null.

This alone would be manageable (although still odd behaviour). I can wait for the second render then get the network status from networkError.statusCode and get the GraphQL errors out of networkError.result.errors.

However, if I ever refetch the query, then the result will only ever have graphQLErrors, and networkError is always null, even though the server is returning a 401.

How to reproduce the issue:

Here's a really simple example component. QUERY here needs to be a query that causes the server to return an error status, and also includes GraphQL errors in the body.

export function TestComponent() {
  const { loading, networkStatus, error, refetch } = useQuery(QUERY);
  console.log({ loading, networkStatus, error: { ...error } });

  return (
    <button onClick={() => {
        console.log("refetch!");
        refetch();
      }}
    >
      refetch
    </button>
  );
}

After loading this component, I see this in my console:

image

Note that initially once done loading, there's a non-empty graphQLErrors array, but no networkError, then immediately after that there's an empty graphQLErrors array, but now there is a networkError.

If I click the refetch button, I only get a single output to the console, it goes back to having graphQLErrors, but no networkError:

image

This means that after re-fetching, I can no longer check the network status code of the response.

Versions

  System:
    OS: macOS 11.3
  Binaries:
    Node: 14.16.1 - /usr/local/bin/node
    npm: 7.10.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 90.0.4430.93
    Firefox: 88.0
    Safari: 14.1
  npmPackages:
    @apollo/client: ^3.3.16 => 3.3.16 
@brainkim brainkim self-assigned this Aug 13, 2021
@brainkim brainkim mentioned this issue Aug 13, 2021
14 tasks
@mogzol
Copy link
Author

mogzol commented Jan 25, 2022

So I just tested this again on Apollo 3.5.8 and it's a bit better, although still strange. Now when refetching it behaves the same as when initially fetching, which is good, but it still does the odd behaviour of initially setting error.graphQLErrors, then immediately re-rendering, clearing error.graphQLErrors, and setting error.networkError.

Here's a codesandbox to reproduce this: https://codesandbox.io/s/apollo-network-error-demo-k5elw?file=/client/App.jsx

Open the console tab after the page loads, you'll see that once the query finishes, the component first re-renders with graphQLErrors but with no networkError, and then immediately re-renders with networkError but no graphQLErrors. Refetching the query now has the same behaviour (previously in apollo 3.3.16 networkError was never set after refetching).

I would expect the component to only re-render once after the query finishes, and for both graphQLErrors and networkError to be set on the error object, not just one or the other. Is the current behaviour intentional?

@MrDoomBringer
Copy link
Contributor

I believe this issue is happening because of the two server responses you're getting, one has a "data" field and the other does not.

That would make this one of many issues related to #9870, so I plan to close this as a duplicate to try and keep discussion in a single place. If you think this issue is unrelated, please let me know and we can keep this open. Thanks a ton :)

@mogzol
Copy link
Author

mogzol commented Jun 30, 2022

I don't think it's because one response has a "data" field and the other doesn't, both responses are identical. I believe this is caused by the response having both GraphQL errors and a network status error (401). So error.graphQLErrors gets set because of the GraphQL errors in the response, but then the component immediately re-renders and sets error.networkError instead because the response also has status 401.

In this scenario I'd expect both graphQLErrors and networkError to be set, rather than only one or the other.

If you still believe this is related to #9870 though then I am fine with closing this.

@MrDoomBringer MrDoomBringer self-assigned this Jul 5, 2022
@MrDoomBringer
Copy link
Contributor

Hi @mogzol! Thanks so much for the fast response.

Yep, you're right - there's only the one server response that results in two renders. However, the underlying issue appears to be the same as in #9870 - Apollo Client's handling/presenting of network-code errors separately from GraphQL errors.

These seem to me to be two sides of the same coin, so I'll be updating #9870 with insights from this issue - while this behavior might not necessarily be unintended, it's definitely confusing for users and in need of change.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🔍 investigate Investigate further
Projects
None yet
Development

No branches or pull requests

4 participants