Intended outcome:

If a query throws an error, recover, either by refetch-ing (if intermittent, this could work) or resetStore-ing and changing something (in my case logging in / thus changing an auth header).

Actual outcome:

react-apollo will not process the successful result of the re-run query (gory details below).

How to reproduce the issue:

Here is a reproduction: https://codesandbox.io/s/qv1q541v2j

Note this points at: https://launchpad.graphql.com/vm3qvkz743 a really simple launch pad that throws an error 50% of the time.

To reproduce the problem in its pure form:

1. Load https://qv1q541v2j.codesandbox.io/ , ensuring it gets the error the first time (ie refresh the page until it does)
2. Hit the refresh button. Notice that even if the graphql network request returns actual data (it will 50% of the time), it never updates the component.

I can make a less random reproduction, this just seemed an easy way to show what's going on.

Version

• [email protected]
• [email protected] [NOTE this doesn't happen with older versions, see gory details below)

Gory details

Why does this happen?

The reason is the confluence of two things:

1. This PR which added an optimization to ignore query results if the previous query result was complete

2. If an query is in the store as an error, a refetch will not inform listeners when it begins, due to this line:

https://github.com/apollographql/apollo-client/blob/09b26bd8d14abcc9f3f7c4648d56b66df83ef658/packages/apollo-client/src/core/QueryManager.ts#L469-L472

So what happens is:
a) first query returns, error. Picked up by RA, renders.
b) refetch happens (either directly or via resetStore). Due to (a) being an error and 2. above, RA is never informed.
c) refetch query returns. In 1. above, RA compares it to the first query, which has networkStatus === 7, so it throws it away.

I'm happy to submit a PR with a fix, if someone can give me some guidance on where to fix it. @jbaxleyiii do you think the optimization (1.) is perhaps broken in other places and should be removed? I won't pretend to fully understand all the code paths that lead to next() being called, but I suspect other similar ones could have been missed.