fix(persistQueryClientRestore): better error handling for persistQueryClientRestore (#8969)

davincho · TkDodo · web-flow · commit 2e1282ca038f · 2025-04-10T18:13:17.000+02:00
* fix(persistQueryClientRestore): Do not swallow exceptions during in persistQueryClientRestore within `restoreClient` and `removeClient`

* fix: rethrow errors in removeClient within all branches

* fix: test for PersistQueryClientProvider in all consumers

* chore: implement PR feedback

* chore: mock console calls, so they do not end up in the console. This way we can also assert on the invocations

* chore: consoleWarn mocks to be restored after each test

* chore: remove tmp file commited accidentally

* chore: adjust docs and include new optional callback `onError`

---------

Co-authored-by: Dominik Dorfmeister &lt;office@dorfmeister.cc&gt;
diff --git a/docs/framework/react/plugins/persistQueryClient.md b/docs/framework/react/plugins/persistQueryClient.md
@@ -219,6 +219,10 @@ ReactDOM.createRoot(rootElement).render(
   - will be called when the initial restore is finished
   - can be used to [resumePausedMutations](../../../reference/QueryClient.md#queryclientresumepausedmutations)
   - if a Promise is returned, it will be awaited; restoring is seen as ongoing until then
+- `onError?: () => Promise<unknown> | unknown`
+  - optional
+  - will be called when an error is thrown during restoration
+  - if a Promise is returned, it will be awaited
 
 ### useIsRestoring
 
diff --git a/packages/query-persist-client-core/src/__tests__/persist.test.ts b/packages/query-persist-client-core/src/__tests__/persist.test.ts
@@ -1,6 +1,9 @@
 import { describe, expect, test, vi } from 'vitest'
 import { QueriesObserver } from '@tanstack/query-core'
-import { persistQueryClientSubscribe } from '../persist'
+import {
+  persistQueryClientRestore,
+  persistQueryClientSubscribe,
+} from '../persist'
 import {
   createMockPersister,
   createQueryClient,
@@ -63,3 +66,97 @@ describe('persistQueryClientSave', () => {
     unsubscribe()
   })
 })
+
+describe('persistQueryClientRestore', () => {
+  test('should rethrow exceptions in `restoreClient`', async () => {
+    const consoleMock = vi
+      .spyOn(console, 'error')
+      .mockImplementation(() => undefined)
+
+    const consoleWarn = vi
+      .spyOn(console, 'warn')
+      .mockImplementation(() => undefined)
+
+    const queryClient = createQueryClient()
+
+    const restoreError = new Error('Error restoring client')
+
+    const persister = createSpyPersister()
+
+    persister.restoreClient = () => Promise.reject(restoreError)
+
+    await expect(
+      persistQueryClientRestore({
+        queryClient,
+        persister,
+      }),
+    ).rejects.toBe(restoreError)
+
+    expect(consoleMock).toHaveBeenCalledTimes(1)
+    expect(consoleWarn).toHaveBeenCalledTimes(1)
+    expect(consoleMock).toHaveBeenNthCalledWith(1, restoreError)
+
+    consoleMock.mockRestore()
+    consoleWarn.mockRestore()
+  })
+
+  test('should rethrow exceptions in `removeClient` before `restoreClient`', async () => {
+    const consoleMock = vi
+      .spyOn(console, 'error')
+      .mockImplementation(() => undefined)
+
+    const consoleWarn = vi
+      .spyOn(console, 'warn')
+      .mockImplementation(() => undefined)
+
+    const queryClient = createQueryClient()
+
+    const restoreError = new Error('Error restoring client')
+    const removeError = new Error('Error removing client')
+
+    const persister = createSpyPersister()
+
+    persister.restoreClient = () => Promise.reject(restoreError)
+    persister.removeClient = () => Promise.reject(removeError)
+
+    await expect(
+      persistQueryClientRestore({
+        queryClient,
+        persister,
+      }),
+    ).rejects.toBe(removeError)
+
+    expect(consoleMock).toHaveBeenCalledTimes(1)
+    expect(consoleWarn).toHaveBeenCalledTimes(1)
+    expect(consoleMock).toHaveBeenNthCalledWith(1, restoreError)
+
+    consoleMock.mockRestore()
+    consoleWarn.mockRestore()
+  })
+
+  test('should rethrow error in `removeClient`', async () => {
+    const queryClient = createQueryClient()
+
+    const persister = createSpyPersister()
+    const removeError = new Error('Error removing client')
+
+    persister.removeClient = () => Promise.reject(removeError)
+    persister.restoreClient = () => {
+      return Promise.resolve({
+        buster: 'random-buster',
+        clientState: {
+          mutations: [],
+          queries: [],
+        },
+        timestamp: new Date().getTime(),
+      })
+    }
+
+    await expect(
+      persistQueryClientRestore({
+        queryClient,
+        persister,
+      }),
+    ).rejects.toBe(removeError)
+  })
+})
diff --git a/packages/query-persist-client-core/src/persist.ts b/packages/query-persist-client-core/src/persist.ts
@@ -84,12 +84,12 @@ export async function persistQueryClientRestore({
         const expired = Date.now() - persistedClient.timestamp > maxAge
         const busted = persistedClient.buster !== buster
         if (expired || busted) {
-          persister.removeClient()
+          return persister.removeClient()
         } else {
           hydrate(queryClient, persistedClient.clientState, hydrateOptions)
         }
       } else {
-        persister.removeClient()
+        return persister.removeClient()
       }
     }
   } catch (err) {
@@ -99,7 +99,10 @@ export async function persistQueryClientRestore({
         'Encountered an error attempting to restore client cache from persisted location. As a precaution, the persisted cache will be discarded.',
       )
     }
-    persister.removeClient()
+
+    await persister.removeClient()
+
+    throw err
   }
 }
 
diff --git a/packages/react-query-persist-client/src/PersistQueryClientProvider.tsx b/packages/react-query-persist-client/src/PersistQueryClientProvider.tsx
@@ -12,20 +12,22 @@ import type { OmitKeyof, QueryClientProviderProps } from '@tanstack/react-query'
 export type PersistQueryClientProviderProps = QueryClientProviderProps & {
   persistOptions: OmitKeyof<PersistQueryClientOptions, 'queryClient'>
   onSuccess?: () => Promise<unknown> | unknown
+  onError?: () => Promise<unknown> | unknown
 }
 
 export const PersistQueryClientProvider = ({
   children,
   persistOptions,
   onSuccess,
+  onError,
   ...props
 }: PersistQueryClientProviderProps): React.JSX.Element => {
   const [isRestoring, setIsRestoring] = React.useState(true)
-  const refs = React.useRef({ persistOptions, onSuccess })
+  const refs = React.useRef({ persistOptions, onSuccess, onError })
   const didRestore = React.useRef(false)
 
   React.useEffect(() => {
-    refs.current = { persistOptions, onSuccess }
+    refs.current = { persistOptions, onSuccess, onError }
   })
 
   React.useEffect(() => {
@@ -35,13 +37,12 @@ export const PersistQueryClientProvider = ({
     }
     if (!didRestore.current) {
       didRestore.current = true
-      persistQueryClientRestore(options).then(async () => {
-        try {
-          await refs.current.onSuccess?.()
-        } finally {
+      persistQueryClientRestore(options)
+        .then(() => refs.current.onSuccess?.())
+        .catch(() => refs.current.onError?.())
+        .finally(() => {
           setIsRestoring(false)
-        }
-      })
+        })
     }
     return isRestoring ? undefined : persistQueryClientSubscribe(options)
   }, [props.client, isRestoring])
diff --git a/packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx b/packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx
@@ -539,6 +539,8 @@ describe('PersistQueryClientProvider', () => {
 
     const queryClient = createQueryClient()
     const removeClient = vi.fn()
+    const onSuccess = vi.fn()
+    const onError = vi.fn()
 
     const [error, persister] = createMockErrorPersister(removeClient)
 
@@ -563,13 +565,18 @@ describe('PersistQueryClientProvider', () => {
       <PersistQueryClientProvider
         client={queryClient}
         persistOptions={{ persister }}
+        onSuccess={onSuccess}
+        onError={onError}
       >
         <Page />
       </PersistQueryClientProvider>,
     )
 
     await waitFor(() => rendered.getByText('fetched'))
     expect(removeClient).toHaveBeenCalledTimes(1)
+    expect(onSuccess).toHaveBeenCalledTimes(0)
+    expect(onError).toHaveBeenCalledTimes(1)
+
     expect(consoleMock).toHaveBeenCalledTimes(1)
     expect(consoleMock).toHaveBeenNthCalledWith(1, error)
     consoleMock.mockRestore()
diff --git a/packages/solid-query-persist-client/src/PersistQueryClientProvider.tsx b/packages/solid-query-persist-client/src/PersistQueryClientProvider.tsx
@@ -11,6 +11,7 @@ import type { JSX } from 'solid-js'
 export type PersistQueryClientProviderProps = QueryClientProviderProps & {
   persistOptions: OmitKeyof<PersistQueryClientOptions, 'queryClient'>
   onSuccess?: () => void
+  onError?: () => void
 }
 
 export const PersistQueryClientProvider = (
@@ -25,13 +26,12 @@ export const PersistQueryClientProvider = (
 
   createEffect(() => {
     setIsRestoring(true)
-    persistQueryClientRestore(options()).then(async () => {
-      try {
-        await props.onSuccess?.()
-      } finally {
+    persistQueryClientRestore(options())
+      .then(() => props.onSuccess?.())
+      .catch(() => props.onError?.())
+      .finally(() => {
         setIsRestoring(false)
-      }
-    })
+      })
   })
 
   createEffect(() => {
diff --git a/packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx b/packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx
@@ -428,6 +428,8 @@ describe('PersistQueryClientProvider', () => {
       .mockImplementation(() => undefined)
     const queryClient = createQueryClient()
     const removeClient = vi.fn()
+    const onSuccess = vi.fn()
+    const onError = vi.fn()
 
     const [error, persister] = createMockErrorPersister(removeClient)
 
@@ -452,13 +454,18 @@ describe('PersistQueryClientProvider', () => {
       <PersistQueryClientProvider
         client={queryClient}
         persistOptions={{ persister }}
+        onSuccess={onSuccess}
+        onError={onError}
       >
         <Page />
       </PersistQueryClientProvider>
     ))
 
     await waitFor(() => screen.getByText('fetched'))
     expect(removeClient).toHaveBeenCalledTimes(1)
+    expect(onSuccess).toHaveBeenCalledTimes(0)
+    expect(onError).toHaveBeenCalledTimes(1)
+
     expect(onErrorMock).toHaveBeenCalledTimes(1)
     expect(onErrorMock).toHaveBeenNthCalledWith(1, error)
     onErrorMock.mockRestore()
diff --git a/packages/svelte-query-persist-client/src/PersistQueryClientProvider.svelte b/packages/svelte-query-persist-client/src/PersistQueryClientProvider.svelte
@@ -11,6 +11,7 @@
 
   export let client: QueryClient
   export let onSuccess: () => Promise<unknown> | unknown = () => undefined
+  export let onError: () => Promise<unknown> | unknown = () => undefined
   export let persistOptions: OmitKeyof<PersistQueryClientOptions, 'queryClient'>
 
   const isRestoring = writable(true)
@@ -22,15 +23,22 @@
       ...persistOptions,
       queryClient: client,
     })
-    promise.then(async () => {
-      if (!isStale) {
-        try {
+    promise
+      .then(async () => {
+        if (!isStale) {
           await onSuccess()
-        } finally {
+        }
+      })
+      .catch(async () => {
+        if (!isStale) {
+          await onError()
+        }
+      })
+      .finally(() => {
+        if (!isStale) {
           isRestoring.set(false)
         }
-      }
-    })
+      })
     onDestroy(() => {
       isStale = true
       unsubscribe()
diff --git a/packages/svelte-query-persist-client/tests/PersistQueryClientProvider.test.ts b/packages/svelte-query-persist-client/tests/PersistQueryClientProvider.test.ts
@@ -366,15 +366,19 @@ describe('PersistQueryClientProvider', () => {
 
     const queryClient = createQueryClient()
     const removeClient = vi.fn()
+    const onSuccess = vi.fn()
+    const onError = vi.fn()
 
     const [error, persister] = createMockErrorPersister(removeClient)
 
     const rendered = render(RemoveCache, {
-      props: { queryClient, persistOptions: { persister } },
+      props: { queryClient, persistOptions: { persister }, onError, onSuccess },
     })
 
     await waitFor(() => rendered.getByText('fetched'))
     expect(removeClient).toHaveBeenCalledTimes(1)
+    expect(onSuccess).toHaveBeenCalledTimes(0)
+    expect(onError).toHaveBeenCalledTimes(1)
     expect(consoleMock).toHaveBeenCalledTimes(1)
     expect(consoleMock).toHaveBeenNthCalledWith(1, error)
     consoleMock.mockRestore()
diff --git a/packages/svelte-query-persist-client/tests/RemoveCache/Provider.svelte b/packages/svelte-query-persist-client/tests/RemoveCache/Provider.svelte
@@ -6,8 +6,15 @@
 
   export let queryClient: QueryClient
   export let persistOptions: OmitKeyof<PersistQueryClientOptions, 'queryClient'>
+  export let onSuccess: () => void
+  export let onError: () => void
 </script>
 
-<PersistQueryClientProvider client={queryClient} {persistOptions}>
+<PersistQueryClientProvider
+  client={queryClient}
+  {persistOptions}
+  {onSuccess}
+  {onError}
+>
   <RemoveCache />
 </PersistQueryClientProvider>

Original file line number	Diff line number	Diff line change
`@@ -84,12 +84,12 @@ export async function persistQueryClientRestore({`
`84`	`84`	`const expired = Date.now() - persistedClient.timestamp > maxAge`
`85`	`85`	`const busted = persistedClient.buster !== buster`
`86`	`86`	`if (expired \|\| busted) {`
`87`		`- persister.removeClient()`
	`87`	`+ return persister.removeClient()`
`88`	`88`	`} else {`
`89`	`89`	`hydrate(queryClient, persistedClient.clientState, hydrateOptions)`
`90`	`90`	`}`
`91`	`91`	`} else {`
`92`		`- persister.removeClient()`
	`92`	`+ return persister.removeClient()`
`93`	`93`	`}`
`94`	`94`	`}`
`95`	`95`	`} catch (err) {`
`@@ -99,7 +99,10 @@ export async function persistQueryClientRestore({`
`99`	`99`	`'Encountered an error attempting to restore client cache from persisted location. As a precaution, the persisted cache will be discarded.',`
`100`	`100`	`)`
`101`	`101`	`}`
`102`		`- persister.removeClient()`
	`102`	`+`
	`103`	`+ await persister.removeClient()`
	`104`	`+`
	`105`	`+ throw err`
`103`	`106`	`}`
`104`	`107`	`}`
`105`	`108`