-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
resetStore: Store reset while query was in flight. #2919
Comments
The point of calling I think throwing an error on having an inflight query is actually the intended outcome here. However, the part where you're seeing |
Is it possible to display the query name causing this error? Error message is not helpful, it is difficult to fix this error. |
@Poincare If no one else has noticed the undefined promise, then it's probably something specific to my app which is using vue-apollo. I'll try to think up how I could repro this in isolation, but not much comes to mind. I'm still not clear why throwing an error is the intended outcome the way it seems to work now. I could see if there was a way to handle the error, but aren't the promises getting rejected just floating around internally in apollo-client and detached from userland-access? Or I could see if there was a way to first stop inflight queries purposely, and then safely call resetStore. But I'm just not wrapping my head around calling something that may or may not throw an exception with no way to catch it. If I'm missing something obvious, then my apologies. |
I believe the correct approach is the stop inflight queries at the application level and then call I'm not sure if I completely grok the problem. I imagine that you do see the error "Store reset while query was in flight" in your application - is this error not possible to catch? |
is this issue related to this one? |
In my case, I was running into this and needed a way to wait for in-flight queries to resolve. It worked pretty well in my case because I have wrappers around calling const inflightPromises = [];
function refetch(variables) {
const refetchPromise = myObservable.refetch(variables).then(results => {
// you could use native array methods here too
// just remove the promise from the array
_.remove(inflightPromises, p => p === refetchPromise);
return results;
});
inflightPromises.push(refetchPromise);
return refetchPromise;
}
function reset() {
// Promise.all does not resolve with empty arrays
const waitFor = inflightPromises.length
? Promise.all(inflightPromises)
: Promise.resolve();
// maybe also guard against calling this while waiting
return waitFor.then(() => apolloClient.resetStore());
} |
@Poincare When using react-apollo, the state of inflight queries is an implementation detail. Should this problem be reported there? I think the correct solution would be to wait for all pending queries to finalize, meanwhile holding any new incoming queries, resetting the store, and then running the pending queries. This is very hard to do at any level other than the client, no? My use case is that when the user changes the language, I want to refetch everything. The "better/more efficient" alternative is passing the current language to every query, which is a lot of work for a pretty rare event. There are a bunch more people with this issue at #1945… |
That said, it seems that in my case the issue is solved by resetting the store at the same time as setting the new language state instead of after the language state was set. |
I'm hitting the error again in another place, and I can't work around it. I'm having a hard time imagining a situation where you call |
Not sure this helps anyone in this thread but I didn't think to see if cookies.set('jwt', response_data.data.getCreds.token)
this.props.client.resetStore()
this.props.loggedIn(response_data) to this cookies.set('jwt', response_data.data.getCreds.token)
this.props.client.resetStore().then(() => {
this.props.loggedIn(response_data)
}) got rid of the error |
I agree with the sentiment that in-flight queries should be automatically dropped and refetched instead of throwing errors (or at least, that behavior should be available as an option). Here's my situation: after completing a mutation with lots of side effects, I would like to drop the entire cache (in lieu of support for targeted cache invalidation) and then navigate to the next page. If I call If I instead reverse the process to |
@dallonf If you want to drop entire cache without triggering reload of current page queries you can achieve that by calling |
@Vlasenko Ah, thanks, that's definitely an easier workaround than what I was going for (forcing an empty render while resetting the store), although this still definitely needs to be addressed. Relying on an undocumented internal function shouldn't be the official solution to any problem! |
@dallonf I'm not from official team, just a user. I understand that calling
If you have access to Apollo Cache somehow, you can call |
same issue. Here is my code snippet: onError(({ graphQLErrors, networkError }) => {
if (graphQLErrors)
graphQLErrors.map(error => {
// console.log(`[GraphQL error]: Message: ${message}, Location: ${locations}, Path: ${path}`)
if (error.code === 1001) {
auth.signout();
//client.resetStore()
client.cache.reset();
window.location.replace('#/login');
//client.resetStore()
// .then(result => {
// window.location.replace('#/login');
// })
// .catch(err => {
// console.log('reset store error: ', err);
// });
}
});
if (networkError) console.log(`[Network error]: ${networkError}`);
}), if I ignore handling the promise And, I have another issue. If |
resetStore: Store reset while query was in flight. apollographql/apollo-client#2919 (comment)
Not sure if this is relevant to all the use cases here but this flow is working ok for me: login = async ({ token, user, authenticated }) => {
const {
props: { client }
} = this;
if (authenticated) {
localStorage.setItem("token", token);
await client.resetStore();
this.setState({ user });
}
return null;
};
logout = async () => {
const {
props: { client }
} = this;
localStorage.setItem("token", "");
this.setState({ user: null, token: null });
await client.resetStore();
}; |
@joshdcuneo your solution works as you're storing "login" state in the React component state which is not a case in most examples presented here. The problem occurs when you try to store "login" state in the apollo cache which is the most obvious place to store it. I want to have access to it from any place in the app. It's more and more frustrating to use Apollo Client. With Redux I didn't have such kind of problems. I'm really considering getting back to Redux for local state management. |
I've created simplest possible reproduction repository here: https://github.com/lukejagodzinski/apollo-client-reset-store-bug Error is thrown on each sign out (
To my surprise it take really long time to resolve those promises. Even when result of the query was already displayed the promise related with this query is still being processed? Maybe it some other bug in the code. I've also recorded short screencast showing a bug https://youtu.be/hY_JiLApXzk |
I was just having the same issue. After some experimentation, including some fixes listed above (to no avail,) I ended up making my logout function synchronous and it got rid of the error. No Error: signoutUser: () => {
//Remove token in localStorage
localStorage.setItem("token", "");
//End Apollo Client Session
apolloClient.resetStore();
} Error: signoutUser: async () => {
//Remove token in localStorage
localStorage.setItem("token", "");
//End Apollo Client Session
await apolloClient.resetStore();
} I don't get why I was getting this error because I have the onError property for my ApolloClient instance set which seems to work in catching all other Apollo errors??? //Catches all errors
onError: ({
operation,
graphQLErrors,
networkError
}) => {
//operations object not currently in use, but stores query/mutation data the error occurred on
if (networkError) Vue.toasted.show(networkError.message);
if (graphQLErrors) {
for (let err of graphQLErrors) {
Vue.toasted.show(err.message);
}
}
}``` |
Hi, not sure if it is worth opening a new issue for this, but sometimes Any idea what could cause |
@eric-burel I think I ran into something similar, but was too lazy to open issue or create minimal reproduction. Here is what happened for me:
|
@doomsower very interesting, I will dig that later but thank you very much for the insight. This might be related indeed, as the view changes on logout even if the resetStore() is not done. It also happens on login (without logout), which also triggers some redirect. Edit: this has been solved in Vulcan by @ErikDakoda : VulcanJS/Vulcan#2313. Instead of relying on a Promise, the solution for us was to use the new |
Thanks for reporting this. There hasn't been any activity here in quite some time, so we'll close this issue for now. If this is still a problem (using a modern version of Apollo Client), please let us know. Thanks! |
Still happening on stable as of two weeks ago |
The way I see it,
And do the following in order:
Is there any case where this wouldn't be appropriate? In-progress Query components don't need to know that the store is resetting; from their perspective it can just look like the query took from before the reset to after the reset to finish. Queries that were finished before the reset, and subsequently need to be refetched, can just jump from Subscriptions don't need to see a status change due to the reset either. From their perspective it can look like they're still subscribed and there just haven't been any events. |
@Vlasenko resetting only the cache seems risky unless you're stopping all of your in-flight queries at application level. Queries that were in-flight before the reset (e.g. queries for the previous logged-in user) could write invalid results to the reset cache when finished. |
@jedwards1211 a very common use case is to reset the store on logout, if the pre-reset queries are restarted, they will either fail or resolve with invalidated parameters ( |
I don't know how you're putting in the auth header but in my case when the queries are restarted, the middleware to add headers gets called anew and injects the new user's auth header |
Running into this issue now. Try all the solutions above, but seems not work. Anyone have some news about this issue ? |
Same here, and I am using @apollo/client 3.0.0-beta.43 |
I can't reproduce this anymore with @apollo/client 3.0.0-beta.46. |
It's happening in my project for modern apollo ├── @apollo/react-hoc@3.1.3
├── @apollo/react-hooks@3.1.3
├── @apollo/react-ssr@3.1.3
├── apollo-cache@1.3.4
├── apollo-cache-inmemory@1.6.5
├── apollo-client@2.6.8
├── apollo-link@1.2.13
├── apollo-link-batch-http@1.2.13
├── apollo-link-error@1.1.12
├── apollo-upload-client@12.1.0
├── apollo-utilities@1.3.3 Reported error Error: Network error: Store reset while query was in flight (not completed in link chain)
at new ApolloError (bundle.esm.js:63)
at ObservableQuery.push.../../node_modules/apollo-client/bundle.esm.js.ObservableQuery.getCurrentResult (bundle.esm.js:159)
at QueryData.push.../../node_modules/@apollo/react-hooks/lib/react-hooks.esm.js.QueryData.getQueryResult (react-hooks.esm.js:265)
at QueryData._this.getExecuteResult (react-hooks.esm.js:73)
at QueryData.push.../../node_modules/@apollo/react-hooks/lib/react-hooks.esm.js.QueryData.execute (react-hooks.esm.js:106)
at react-hooks.esm.js:380
at useDeepMemo (react-hooks.esm.js:354)
at useBaseQuery (react-hooks.esm.js:380)
at useQuery (react-hooks.esm.js:397)
at Query (react-components.esm.js:8) "
at Meta (http://localhost:8080/common.chunk.js:104011:3) // <------------ This is my component
at Query (http://localhost:8080/app.chunk.js:167:26) So basically what I'm trying to achieve is to reset apollo in-memory cache on redux store change when user logged out, it may be because user clicked "Logout" link intentionally or token expired etc. After "logout" user is automatically redirected to another page that run some graphql queries about the page (head meta data like title, description etc.). I'm not sure how should I call Is something like below (#3766 (comment)) the way to fix it?
|
Just open react-development-tools, find ApolloProvider and exec for (let i = 0; i< 3; i++) {
console.log('i', i);
setTimeout($r.props.client.resetStore, 10)
} I still got error
Before, in pure js i just checking by if(!client.queryManager.fetchCancelFns.size) {
await client.resetStore()
} but i white in typescript now and client.queryManager is private https://github.com/apollographql/apollo-client/blob/main/src/core/ApolloClient.ts#L74 It's so sad! I can't check this any more and apollo does not provide any checking((( UPD. But i can check like this client["queryManager"].fetchCancelFns.size and no typescript errors... |
Calling client.stop() method before clearStore solved my problem. client.stop() Accroding the official doc: |
@khushahal-sharma it's bot optimal. For example, i have many subscriptions and got many events for updating at the moment. In this case starts many cicles "Send query, stop, send another...". Checking global sending status more useful. |
still encounter this error with "@apollo/client": "~3.4.17". |
Intended outcome:
resetStore
without errors being thrown.https://github.com/apollographql/apollo-client/blob/b354cf07e979def7aa0219e60dca63e8a33fa822/packages/apollo-client/src/core/QueryManager.ts#L787..L796
Actual outcome:
I'm running into an issue when calling
resetStore
after a logout scenario. I think I understand the reasoning, but in my case I can't seem to find a workable way around it, and something else odd is happening:fetchQueryPromises
in snippet above is aMap
, with values of{promise,reject,resolve}
, so I look inside to see what they are:const promises = Array.from(this.fetchQueryPromises.values()).map(x => x.promise)
My plan was to
await Promise.all(promises)
before callingresetStore
, however,promises
is[undefined]
. For some reason, an entry is added tofetchQueryPromises
without a promise, so I can't wait for it before resetting.The actual error
Store reset while query was in flight.
looks like it might not actually cause a problem, which was my main concern. But it seems like there should be a way to avoid an error being thrown.Waiting for existing promises to complete before reset, or exposing a function to allow the user to await them before resetting seems right to me.
Is there a case for calling
resetStore
where a thrown error is the right outcome?Version
The text was updated successfully, but these errors were encountered: