Apollo-client: Race-condition in refetchQueries (cache-first query completes before network-only query)

[ctavan]

Intended outcome:

Using refetchQueries on a mutation should guarantee to update the store from the network query result.

Actual outcome:

If the application triggers the same query, that was supposed to be refetched, shortly after the mutation has run (which by default will happen with the cache-first policy), this query will typically hit the cache and complete before the network-only query issued by refetchQueries will finish.

Since the refetch query has a lower requestId its result will not be propagated to the store, see: https://github.com/apollographql/apollo-client/blob/d420cbdbb3e6637ee2e8e8c211e3d28850b93c6e/src/queries/store.ts#L135

How to reproduce the issue:

See: https://github.com/ctavan/react-apollo-error-template/tree/reproduce-refetch-query-race

I think the issue is best explained when looking at the demo app / screenshot:

The steps to reproduce are:

1. Load the page -> List query is run and populates the store with 3 people.
2. Go to "create" which unmounts the List and mounts the Create component. Clicking the button will fire a mutation which adds a new person.
3. The mutation has refetchQueries: ['People'], specified.
   
   
   
   • As is visible from the console output which simulates the server behavior, apollo-client starts a network request to refetch the query right after the mutation network request has resolved (and even before the mutate() promise resolves, as indicated by Mutation done). In the example this gets lastRequestId = 4:
   
   

• At this point the success callback of the resolved mutation redirects back to the List component which re-triggers the list-query there, lastRequestId = 5:

• Shortly after that, the network request for the refetch resolves and as can be seen from the server output there were actually 4 elements returned.
• However the store still only holds 3 elements for the people query as can be seen from the Apollo client dev toolbar. I assume this is because the cache-first which was executed synchronously resolved before the network-only refetch query and the network-only query was therefore discarded.

My assumption would be that refetchQueries guarantees to sync the current store with the result returned from the server and that intermediate cache-first queries cannot interfere with this.

So, is this a bug or a feature? :)

[helfer]

Hi @ctavan, thanks for the very detailed bug report! I think this is definitely not a desired feature, so if you would be able to provide us with a reproduction using react-apollo-error-template, we should be able to fix this pretty quickly (or you could help us fix it by making a PR with a failing test!).

[fc]

@helfer it looks like he already provided a reproduction react-apollo-error-template (refer to the link in his post).

[ctavan]

Yes indeed @fc @helfer: The reproduction is here: https://github.com/ctavan/react-apollo-error-template/tree/reproduce-refetch-query-race

[aroth]

Also noticing this.

[jbaxleyiii]

@ctavan thank you for the great report! I'll take a look!

[stale[bot]]

This issue has been automatically marked as stale becuase it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Apollo Client!

[stale[bot]]

This issue has been automatically closed because it has not had recent activity after being marked as stale. If you belive this issue is still a problem or should be reopened, please reopen it! Thank you for your contributions to Apollo Client!

[ctavan]

The issue still persists and I believe this issue should not be closed.

[WiktorKa]

Is there any update on this issue?

[tsvetann]

...and in 2018 the issue is still alive :)

[SebastianMuniz]

I'm having the same issue! Please fix or provide some light about it.

[oduncan]

Running Into the same issue, It's as if Apollo was only intended for SPAs? What is the current work around?

[SebastianMuniz]

The workaround for this is to have a unique collection of records in a store and when you send a mutation you request all inserted fields and merge that into your store collection

[oduncan]

I am still learning, can you point me to a resource?

[SebastianMuniz]

I'm sorry I'cant. It's the solution i implemented myself for a particular case. Im using an app state container, in this case Mobx, I have a list of object record in the store, when i start my app I make a query to the backend asking for the list of records.
Then every time I make a mutation to add or subtract an object form the list, inside the mutation I select all the field of the object I'm adding. So when the object its inserted i get back the response with the object itself. So then i merge it to the coleection of record i got in the store.
I dont know if its more clear for you, hope so.

[oduncan]

Thank you.

[nickmarlou]

The issue is still here and with update mutation it's really wierd

[mortenab]

Also seeing this issue - any plans to fix it?

[alexgorbatchev]

I think the label applied incorrectly to the issue :)

[vincentdesmares]

I can still reproduce it. I had to replace most of my query names by an object to solve the rendering issue:

{ 
  refetchQueries: [{
    query: myGraphQlQuery,
    variables: {
        id: 2
    }
  }]
}

As said previously, when using a string, the queries are fetched (they appear in the network tab) but the props are sometimes not updated.

I'm not sure the refetchQuery as a String are supported anymore as it doesn't appear in the official doc.

Yet, there they have some tests and a few doc-strings reference them.

It would be nice if the maintainers could clearly state if this feature is supported or not. If yes, this ticket must be re-open, else, the documentation should be cleaned up and a deprecation warning should be added to help people migrate to the supported way.

[smirea]

any update on this?
it's pretty painful right now to update the store and the refetchQueries with string options was a pretty clean solution that is not working.

[jpikora]

I think the race condition in question is the result of the fact that the mutation and refetchQueries promises are not chained, discussion: #1618

Following the reproduction steps from @ctavan, the user is redirected to the List of People right after the create mutation gets resolved, not waiting for the People query to be refetched, which leaves the cache out of sync as the fresh data from refetch are discarded due to lower request ID.

In this scenario, the solution could be to refetch queries manually in resolve callback of the mutation promise e.g.

import { apolloClient } from '@/providers/apollo'

function refetchQueries (queries) {
  return Promise.all(queries.map(q => apolloClient.query(
    Object.assign({}, q, {
      fetchPolicy: 'network-only'
    })
  )))
}

apolloClient.mutate({
  ...
})
  .then(() => {
    return refetchQueries([{
      query: PEOPLE_QUERY,
      variables: {
        ...
      }
    }])
  })
  .then(() => {
    // redirect
  })

[ctavan]

BTW I have updated my reproduction code at https://github.com/ctavan/react-apollo-error-template/tree/try-reproduction-with-fix to use the latest master of react-apollo-error-template which comes with these dependencies:

{
  "dependencies": {
    "apollo-cache-inmemory": "^1.2.2",
    "apollo-client": "^2.3.2",
    "apollo-link": "^1.2.2",
    "graphql": "^0.13.2",
    "graphql-tag": "^2.9.2",
    "react": "^16.4.0",
    "react-apollo": "2.1.4",
    "react-dom": "^16.4.0"
  }
}

The particular problem I have reported here already seems to be fixed in those released versions, so not sure if the referenced change was even required for solving this particular bug. Maybe it was just fixed at some point along the way with the Apollo Client 2.0 refactoring.

[Emiliano-Bucci]

Any updates on this? Having the same weird issue: after a mutation, when refetchQueries is called with some variables, the cache-first data is the one who returns