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

resumePausedMutations are not running after switching from offline to online after version 4.24.3 up to 4.32.6(latest) #5847

Closed
timabilov opened this issue Aug 6, 2023 · 31 comments · Fixed by #7019
Labels

Comments

@timabilov
Copy link

timabilov commented Aug 6, 2023

Describe the bug

8991d7c this is the commit where it breaks (4.24.3)

Not sure if this is correct behaviour. But based on the commit above i assume it is a bug indeed.
Paused mutations get stuck when rehydrating(loading) mutations in offline mode and switching back online.

Basically, when you reenter the exited app in offline mode it restores the mutations successfully, i see paused mutations in logs as a result of resumePausedMutations.

But when you get back online, paused mutations do not run. This happens in latest 4.32.6. When i reload(React Native) my javascript it runs again.
But, i got to the version where it works and triaged this.
basically this is the commit where it starts breaking
8991d7c

Shortly, 4.24.3 and upper version causes this bug

Your minimal, reproducible example

Run https://github.com/TanStack/query/tree/main/examples/react/offline

Steps to reproduce

  1. Go to any movie
  2. Go offline/ turn off your wife
  3. Add comment (to verify, add it three times)
  4. Refresh the page
  5. ENABLE your WIFI and then wait
  6. Nothing happens, but expected this mutations to get unpaused
  7. Refresh the page and you see updates are applied now sequentially.

This happens in my React Native setup too.

  1. Go offline
  2. Add todo (fire any mutation which will be paused)
  3. Exit from app then enter again (both RN debug/release mode)
  4. enable internet
  5. At the end i see 1 paused mutation pending in my logs.
    So it stucks/waits here:
queryClient.resumePausedMutations()

Expected behavior

As a user, i am expecting all mutations to start running after resume, but no mutations are running in that case.
In logs they are all paused

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

React Native 0.70.6

Tanstack Query adapter

react-query

TanStack Query version

4.32.6 (but started in 4.24.3!!)

TypeScript version

No response

Additional context

If i will reload my javascript after 5th step, then it resumes mutations again and runs them correctly

Also, if you have internet enabled before entering the app, it works as expected.

@timabilov timabilov changed the title resumePausedMutations are not running after switching from offline to online in version "4.32.6" resumePausedMutations are not running after switching from offline to online after version 4.24.3 up to 4.32.6(latest) Aug 6, 2023
@timabilov
Copy link
Author

timabilov commented Aug 6, 2023

resumePausedMutations() {
  if (!this.resuming) {
    const pausedMutations = this.mutations.filter(x => x.state.isPaused);
    this.resuming = notifyManager.notifyManager.batch(() => pausedMutations.reduce((promise, mutation) => promise.then(() => mutation.continue().catch(utils.noop)), Promise.resolve())).then(() => {
      this.resuming = undefined;
    });
  }

  return this.resuming;
}

So, i found that this is related to the fact that i have multiple setOnline(true) calls, and because of that 'resuming' patch - mutation.continue() only fired once where it basically still stays in pause.
But without this resuming patch, subsequent setOnline(true) -> resumePausedMutations calls fires my requests.

To be sure, it is definitely not request hanging or some timeout issues because my mutationFn is not called at all with given conditions.

Not sure if it is related to something with my connection anyway, because after setOnline(True) it still should trigger mutations, which is weird. but i see only resumePausedMutations triggers

@timabilov
Copy link
Author

To reproduce:

  1. Run https://github.com/TanStack/query/tree/main/examples/react/offline
  2. Go to any movie
  3. Go offline/ turn off your wife
  4. Add comment (to verify, add it three times)
  5. Refresh the page
  6. ENABLE your WIFI and then wait
  7. Nothing happens, but expected this mutations to get unpaused
  8. Refresh the page and you see updates are applied now sequentially.

@TkDodo i believe that this is incorrect behaviour

@apcrtc
Copy link

apcrtc commented Aug 16, 2023

We have the same issue with React Web + React Native.

@adampax
Copy link

adampax commented Aug 18, 2023

Also experiencing this or something similar on React Native with v4.24.6 of query, the query client, and async storage persister.

To test, we do the following:

  1. Turn the device to airplane mode or turn off wifi + data
  2. Perform a mutation. Observe with useIsMutating() that mutation count is > 0
  3. Remove the app from background
  4. With airplane mode still enabled, relaunch app
  5. Turn off airplane mode, observe that app and queryClient go online, but mutation is never POSTed, mutation count remains > 0

If we turn off airplane mode prior to relaunching the app, the mutation is is sent as expected.

@TkDodo
Copy link
Collaborator

TkDodo commented Aug 19, 2023

@timabilov for some reason, I can't get our own example to work. All the endpoints return 404 for me. Could be something with mock service worker that we are using in there :/

I would appreciate it if you could come up with a failing test case from within our repository, that would be the easiest for me to get started with understanding that issue. Thanks

@timabilov
Copy link
Author

timabilov commented Aug 19, 2023

lool, was readin my serious description over there to refresh my mind:
2. turn off your wife

😂

Sorry, that might take some time for me to write testcase now, but i already found the commit which is causing this issue, hope someone will help here.
Will try to reproduce this with test case in case if i will have a time

@TkDodo
Copy link
Collaborator

TkDodo commented Aug 19, 2023

I was laughing hard at that one, too 🤣

@adampax
Copy link

adampax commented Aug 21, 2023

The Code Sandbox version isn't working for me either (https://tanstack.com/query/v4/docs/react/examples/react/offline) but when running it locally with @timabilov's steps, am able to reproduce

To reproduce:

  1. Run https://github.com/TanStack/query/tree/main/examples/react/offline
  2. Go to any movie
  3. Go offline/ turn off your wife
  4. Add comment (to verify, add it three times)
  5. Refresh the page
  6. ENABLE your WIFI and then wait
  7. Nothing happens, but expected this mutations to get unpaused
  8. Refresh the page and you see updates are applied now sequentially.

I think it important to note that the issue happens when turning off the computer's wifi (or wife!) in the steps above and refreshing the page while the wifi is still disabled.

Disabling the connection via the Devtools doesn't reproduce the issue for me, perhaps because the connection is automatically restored when refreshing?

@janisievins
Copy link

janisievins commented Aug 25, 2023

I'm having the same issue in a react-native/react-native-web + react-query project using react-query version 4.29.7.
Tried rolling back to 4.24.2 and I confirm that the issue is not present in that version.

@tmikoss
Copy link

tmikoss commented Aug 25, 2023

Eagerly awaiting a fix on this. What's worse, the bug is both easy to encounter ("why won't this work, better refresh the page a couple times to be sure, oh my internet is off"), and hard to write automated tests for. A downgrade will solve this for now, but sooner or later we'll have to upgrade.

@TkDodo
Copy link
Collaborator

TkDodo commented Aug 25, 2023

@tmikoss if this is important to you, please consider contributing a fix.

@TkDodo
Copy link
Collaborator

TkDodo commented Aug 29, 2023

maybe someone can try this with v5 beta? We've changed how online detection works because of issues in chrome. So in v5, you'll always be seen as "online" in the react-query onlineManager, and we'll only go offline if there is an explicit offline event. I think this means that in the following steps to reproduce:

  1. Go to any movie
  1. Go offline/ turn off your wife
  2. Add comment (to verify, add it three times)
  3. Refresh the page
  4. ENABLE your WIFI and then wait
  5. Nothing happens, but expected this mutations to get unpaused
  6. Refresh the page and you see updates are applied now sequentially.

You would get the mutations resumed after step 4, even if you are offline. They would then likely fail with NetworkError. If you have retry set to at least 1, they would then be paused. I would expect step 5 to continue them normally then.

We're aware that this change in how we handle online / offline states might be a step back for offline apps, but it's the better tradeoff / lesser evil in this case.

See:

@adampax
Copy link

adampax commented Sep 7, 2023

I tested by updating the offline example to the beta versions below:

    "@tanstack/query-sync-storage-persister": "^5.0.0-beta.20",
    "@tanstack/react-location": "^3.7.0",
    "@tanstack/react-query": "^5.0.0-beta.20",
    "@tanstack/react-query-devtools": "^5.0.0-beta.20",
    "@tanstack/react-query-persist-client": "^5.0.0-beta.20",

Then doing these steps:

  1. Run https://github.com/TanStack/query/tree/main/examples/react/offline
  2. Go to any movie
  3. Go offline/ turn off your wife
  4. Add comment (to verify, add it three times)
  5. Refresh the page
  6. ENABLE your WIFI and then wait
  7. Nothing happens, but expected this mutations to get unpaused
  8. Refresh the page and you see updates are applied now sequentially.

My observations:

You would get the mutations resumed after step 4, even if you are offline.

The mutation stayed isPaused here.

They would then likely fail with NetworkError.

I didn't see any network errors in Chrome dev console. (Maybe I'm looking in the wrong place?)

If you have retry set to at least 1, they would then be paused.

I tried both with and without retry here and they appeared to behave the same.

I would expect step 5 to continue them normally then.

On Step 5 (refreshing the page while wifi remains turned off), the mutation is shown as successful, with the toast message 'Successfully updated movie 1'. Additionally, while the wifi remains off, if I make more mutations, they are shown as successful.

I'm not sure how the mutation could be successful after Step 5 with wifi still off. Maybe it is related to testing with a local endpoint? I didn't look to closely how the mock api is set up.

I went ahead and tested beta 5 with my React Native project and the behavior remained the same as above

@badsyntax
Copy link

I'm having the same issue in a react-native/react-native-web + react-query project using react-query version 4.29.7. Tried rolling back to 4.24.2 and I confirm that the issue is not present in that version.

Thanks, things work for me with 4.24.2. Tried the latest beta with no luck.

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 11, 2023

I'm not sure how the mutation could be successful after Step 5 with wifi still off. Maybe it is related to testing with a local endpoint? I didn't look to closely how the mock api is set up.

@adampax the mock api intercepts network requests and delivers results before the actual fetch is made. The difference between v4 and v5 is that in v5, we always start with being online - so our internal onlineManager thinks your online, independent of what you actually are. That's because of the chrome issues I outlined above.

With that in mind, I think what you are seeing is expected. If we'd try with an actual API call, it would fail with NetworkError.

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 11, 2023

Tried the latest beta with no luck.

@badsyntax can you show a reproduction with v5 beta please? Or a replay.io recording? That would really help ...

@badsyntax
Copy link

@TkDodo in the meantime i've create a repro here, see README for instructions: https://github.com/badsyntax/react-query-mutation-resume-bug

i'll see what i can do with replay.io

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 12, 2023

thanks for this, I think I figured it out a bit:

when we resume paused mutations after you've reloaded the page, there is no "retryer" that can pick up the mutations from where they left, so we just always execute them:

this.execute(this.state.variables!)

This will then create the retryer and return a pending promise, because we start in paused state. This is technically fine, however, since we started this process via resumePausedMutations, we have now stored that promise in the mutationCache in this.resuming:

this.#resuming = (this.#resuming ?? Promise.resolve())

This was the fix that made sure that resuming paused mutations will always run in serial.

Now when you go online again, we call resumePausedMutations again, but it doesn't actually resume anything, because it already thinks it is resuming. This works when we have a retyer, because retryer.continue() resolves a Promise immediately if we are not online (another part of the aforementioned fix):

continue: () => {
const didContinue = continueFn?.()
return didContinue ? promise : Promise.resolve()
},

but it doesn't work if you are offline when the page loads.


I need to think a bit how to tackle this holistically, but what you can do on your end if to simply wrap the call to resumePausedMutations into another online check:

 <PersistQueryClientProvider
   client={queryClient}
   persistOptions={{ persister }}
   onSuccess={() => {
     // resume mutations after initial restore from localStorage was successful
+      if (onlineManager.isOnline()) {
           queryClient.resumePausedMutations().then(() => {
               queryClient.invalidateQueries()
           })
+      }
   }}
 >

We should probably do this internally, but I have to think about where exactly without breaking things. Also, this should be significantly less relevant with v5 because the onlineManager will always start in online mode.

@badsyntax
Copy link

badsyntax commented Sep 13, 2023

@TkDodo thanks for the investigation & explanation, that all makes sense! i can confirm the workaround works with the latest v5 beta

[edit] The workaround works in the repro, but i'm still having problems when using the beta in React Native. I don't understand the difference but i'll update this thread if i figure it out.

@nandorojo
Copy link
Contributor

Hey guys, I'm arriving here from what I think is a similar issue on v5, where mutations aren't firing after the page refreshes (and reconnects).

Tell me if my issue applies, or if I should create a separate issue (or if I'm just doing something incorrectly).

My mutations are working properly when the internet connection goes from off → on. However, it does not work when refreshing the page and then turning the network from off → on.

  1. Fire a mutation with network disabled. I see the mutation get added to localStorage correctly.
  2. Reload the page, and re-enable network from the Chrome Devtools. I still see the mutation in the local storage, so I expect it to resume. I have called client.setMutationDefaults correctly at the root of the app too.
  3. I expect the mutation to send again. However, it does not. I'm using the onSuccess prop on the PersistQueryClientProvider to call resumePausedMutations as documented, but the network tab never shows the mutation firing off. It just gets deleted from the local storage mutations array and that's it.

This all logs properly too:

onSuccess() {
  console.log('[PersistQueryClientProvider] onSuccess')
  chatQueryClient.resumePausedMutations().then(() => {
    console.log('[PersistQueryClientProvider] resumePausedMutations')
    chatQueryClient.invalidateQueries()
  })
}

Any ideas for why my mutations aren't firing again when the page refreshes and it's pulling them from local storage? Thanks!

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 23, 2023

It just gets deleted from the local storage mutations array and that's it.

if it gets deleted, it means its no longer in paused state. Can you show a separate reproduction for this please?

@nandorojo
Copy link
Contributor

Yeah I’ll put one together today. I assume it is no longer paused because I call resumePausedMutations. This makes sense, however, the mutationFn that is globally-registered isn’t getting called.

@nandorojo
Copy link
Contributor

nandorojo commented Oct 7, 2023

As a follow up, I put together a reproduction here. However, it doesn't actually reproduce my error (because it does indeed work), so I assume something is up on my end.

I've been exploring this for days but I can't seem to figure it out. I recorded a quick video in case it might ring a bell for what could be happening. If you're able to look at it, it would be super appreciated.

Here's a video of what's happening. https://www.loom.com/share/74d344da67b942a183ec0461bf5aa7b4

TLDR: Disconnecting & refreshing the page appears to empty out my mutations array without ever firing the mutations. And it seems to do this before React is really loaded (in dev mode in particular).

The behavior feels very odd.

If you look at the timestamp field you will see it change in this tiny interval, which makes me think that something is causing the persistence client to change or get messed up somewhere.

I called setMutationDefaults properly and everything, so I'm quite stuck. Hopefully something rings a bell. Thank you!

I'm wondering if my issue is somewhat related to this: #5867

@badsyntax
Copy link

badsyntax commented Oct 24, 2023

@TkDodo After adding the workaround you mentioned to the web example, i was still seeing the same behaviour in my React Native app. Your explanation of the initial problem (...but it doesn't actually resume anything, because it already thinks it is resuming) led me down the right debugging path.

In my case i was setting the focused state with the focusManager but in doing so it would attempt to resume mutations when the app is offline thus getting everything out of sync. Once i removed the code to set the focus state the offline cache now behaves the same as in the example posted above (with the workaround you mentioned).

This was the code i was using to set the focused state:

import { focusManager } from '@tanstack/react-query';
import { useEffect } from 'react';
import type { AppStateStatus } from 'react-native';
import { AppState } from 'react-native';

export function useAppFocusState() {
  useEffect(() => {
    const subscription = AppState.addEventListener(
      'change',
      (state: AppStateStatus) => {
        focusManager.setFocused(state === 'active');
      },
    );
    return () => subscription.remove();
  }, []);
}

To figure this all out i edited the code in node_modules to see when resumePausedMutations was called, and that's when i noticed it was being called more than expected.

At the end of the day i'm happy to say now i have no problems with the offline cache in my react native app using v5.

@schriker
Copy link

Hi! 👋, First of all, thank you for this library such an amazing work! I think I'm facing a similar issue as described above. I'm using ReactQuery with GraphQL in my React Native App, and resuming offline mutations seems to be failing when I kill my app and open it again but still offline.

So far I have found out that when I start the app fresh, and when I'm offline it looks like mutation gets fired, and because of that it will go to Error state and will be no longer paused. I'm not sure if this is expected?

I have tried this workaround but this doesn't help as onlineManager.isOnline() starts as true

 if (onlineManager.isOnline()) {
           queryClient.resumePausedMutations().then(() => {
               queryClient.invalidateQueries()
           })
}

But even if I remove queryClient.resumePausedMutations() from my PersistQueryClientProvider it looks like it is still getting fired from somewhere(?). If I set explicitly onlineManager.setOnline(fals) then it seems to queryClient.resumePausedMutations() be paused, but this breaks other flows.

My code:

// App.ts
export default function App() {
    useQueryFocusManager()
    const setPendingMutations = useSetAtom(pendingMutationsAtom)
    const isAuthorized = useAtomValue(isAuthorizedAtom)
    const themeScheme = useThemeScheme()
    const theme = useTheme(themeScheme)

    if (isAndroid) {
        NavigationBar.setBackgroundColorAsync(!isAuthorized
            ? theme.colors.background
            : theme.colors.accent
        )
    }

    return (
        <PersistQueryClientProvider
            client={queryClient}
            persistOptions={{
                persister: syncStoragePersister
            }}
            onSuccess={() => {
                queryClient.resumePausedMutations().then(() => {
                    setPendingMutations(0)
                })
            }}
        >
            <GraphqlContext.Provider value={graphqlClient}>
                <ThemeProvider theme={theme}>
                    <SafeAreaProvider>
                        <Wrapper>
                            <AppRouter />
                            <StatusBar style={themeScheme === 'dark' ? 'light' : 'dark'} />
                            <Toast />
                        </Wrapper>
                    </SafeAreaProvider>
                </ThemeProvider>
            </GraphqlContext.Provider>
        </PersistQueryClientProvider>
    )
}
// reactQueryClient.ts
onlineManager.setEventListener(setOnline => NetInfo.addEventListener(state => setOnline(Boolean(state.isConnected))))

export const onAppStateChange = (status: AppStateStatus) => {
    if (Platform.OS !== 'web') {
        focusManager.setFocused(status === 'active')
    }
}

export const queryClient = new QueryClient({
    defaultOptions: {
        queries: {
            gcTime: 1000 * 60 * 60 * 24 * 7,
        },
        mutations: {
            gcTime: 1000 * 60 * 60 * 24 * 7,
        }
    },
})

queryClient.setMutationDefaults([MutationKeys.CreateCategory], {
    mutationFn: variables => graphqlClient.request(CreateCategoryMutation, variables),
})

export const syncStoragePersister = createSyncStoragePersister({
    storage: reactQueryStorage
})
// useQueryFocusManager.ts
export const useQueryFocusManager = () => {
    useEffect(() => {
        const subscription = AppState.addEventListener('change', onAppStateChange)

        return () => subscription.remove()
    }, [])
}
// useCreateCategory.ts
export const useCreateCategory = () => {
    const request = useRequest()
    const queryClient = useQueryClient()
    const { handleError } = useRequestHelpers()

    return useMutation<CreateCategoryMutationType, ClientError, CreateCategoryMutationVariables, { meQuerySnapshot: MeQuery | undefined }>({
        mutationKey: [MutationKeys.CreateCategory],
        mutationFn: createCategoryMutationInput => request({
            document: CreateCategoryMutation,
            variables: createCategoryMutationInput
        }),
        onMutate: async newCategoryInput => {
            await queryClient.cancelQueries({ queryKey: [QueryKeys.Me] })
            const meQuerySnapshot = queryClient.getQueryData<MeQuery>([QueryKeys.Me])

            queryClient.setQueryData<MeQuery>([QueryKeys.Me], oldData => {
                if (!oldData) {
                    return undefined
                }

                return {
                    ...oldData,
                    me: {
                        ...oldData.me,
                        categories: [
                            ...oldData.me.categories,
                            {
                                __typename: "Category",
                                id: `${PENDING_PREFIX}-${Crypto.randomUUID()}`,
                                createdAt: new Date().toISOString(),
                                updatedAt: new Date().toISOString(),
                                name: newCategoryInput.data.name
                            }
                        ]
                    }
                }
            })

            return { meQuerySnapshot }
        },
        onError: (error, _, context) => {
            handleError(error)
            queryClient.setQueryData([QueryKeys.Me], context?.meQuerySnapshot)
        },
        onSettled: () => {
            queryClient.invalidateQueries({ queryKey: [QueryKeys.Me] })
        },
    })
}

Example:

  1. ✅ Going online while the app is open - works great!
23-11-19-10-06-54.mp4
  1. ✅ Killing the app but going back online before opening app again - works great!
23-11-19-10-07-37.mp4
  1. ❌ Killking the app and open it while offline, when back online I lose the data.
23-11-19-10-08-28.mp4

I'm looking for some ideas how can I handle 3rd case? Is there any way I can maybe inside error in setMutationDefaults put this mutation back as paused? Or somehow handle this other way?

@badsyntax
Copy link

badsyntax commented Nov 19, 2023

@schriker for the 3rd case, although a little different to my case, to fix it i had to remove the focusManager related code. So try remove focusManager.setFocused() in your code and see if that affects the behaviour.

@schriker
Copy link

schriker commented Nov 19, 2023

@schriker for the 3rd case, although a little different to my case, to fix it i had to remove the focusManager related code. So try remove focusManager.setFocused() in your code and see if that affects the behaviour.

Yes, I have tried I think all workarounds from this issue but none seems to resolve my case. At the moment I'm thinking on catching all mutations on default on Error and handling this somehow. But not sure yet. BTW. i forgot to post that I'm using versions:

    "@tanstack/query-sync-storage-persister": "^5.0.5",
    "@tanstack/react-query": "^5.0.5",
    "@tanstack/react-query-persist-client": "^5.8.4",

@schriker
Copy link

schriker commented Nov 19, 2023

@badsyntax Ok, You were right 👍 This was good direction, basically I had to check if I'm online using NetInfo library but in 2 places:

  1. To try to resume paused mutations only if online otherwise it will also throw Network Error and mutation will no longer be paused.
        <PersistQueryClientProvider
            client={queryClient}
            persistOptions={{
                persister: syncStoragePersister
            }}
            onSuccess={() => {
                NetInfo.fetch().then(state => {
                    if (state.isConnected) {
                        queryClient.resumePausedMutations()
                    }
                })
            }}
        >
  1. Setting focus manager also only if device is only otherwise this will also try to trigger mutations and if offline they will fail with NetworkError so no longer paused:
    NetInfo.fetch().then(state => {
        if (state.isConnected && Platform.OS !== 'web') {
            focusManager.setFocused(status === 'active')
        }
    })

For the focus manager I think there might be better way to handle this I just posting it for now like this. With this it looks like all 3 cases from my previous post works 👍

@badsyntax
Copy link

It would be ideal if the library would handle these states for us based off the state of the online manager.

@fardeem
Copy link

fardeem commented Mar 3, 2024

Just ran into the same issue and figured it out (thanks @schriker) so leaving some notes.

In particular the issue is:

  • if you go offline
  • trigger a mutation
  • reload the app while offline
  • then go online

react query should resume mutations but it doesn't.


When you trigger a mutation, it correctly gets persisted. Assuming your storage works, when you reload the app, the mutation gets restored.

At this point, if you naively call resumePausedMutations, it will try your mutation, fail due to network error and then go into the failed state and clear out the mutation. When mutations fail, unless you specify retry: true, it doesn't retry the mutation again.

However, if you conditionally call resumePausedMutations based on online status, it all works. The app loads and doesn't do anything. online status updates when the app goes online and finally resume is called.

Hopefully the longer explanation is useful to understand what's going on under the hood.

@TkDodo
Copy link
Collaborator

TkDodo commented Mar 3, 2024

Would it work if we'd wrap resumePausedMutations internally in another check if we're really online ?

or should we just document that such a check might be necessary in user-land before calling it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants