query icon indicating copy to clipboard operation
query copied to clipboard

Errored queries caught by ErrorBoundary are not retried on mount

Open tobias-tengler opened this issue 4 years ago β€’ 40 comments

Describe the bug I'm using Suspense and ErrorBoundaries and I'm seeing unexpected behavior when queries are erroring. If a query errors I'm displaying an ErrorBoundary for that part of the UI. Now if I navigate to another page using react-router-dom and navigate back to the page, the ErrorBoundary is still there and the query is not retried. I know that normally you would have to reset the error boundary while you are on the same page to trigger a re-run of the failed queries, but in this case I would've assumed that the queries are retried when I visit the page again, i.e. the components are mounted again.

To Reproduce

  1. Visit the Code Sandbox
  2. Open the console
  3. Wait until the error occurs and the fallback of the ErrorBoundary is shown: "An error occured!" (You need to click off of react-error-overlay to see it)
  4. Press the "Hide" button to unmount the ErrorBoundary
  5. Press the "Show" button to render it again
  6. Query is not re-executed and the error is immediately shown again

Expected behavior The query is re-executed after the component mounts, i.e. honoring the retryOnMount setting.

Desktop (please complete the following information):

  • Version: 3.24.4

tobias-tengler avatar Sep 23 '21 20:09 tobias-tengler

The docs say:

Whether you are using suspense or useErrorBoundaries in your queries, you will need a way to let queries know that you want to try again when re-rendering after some error occurred.

So I think in your scenario, utilizing useQueryErrorResetBoundary when the ErrorBoundary unmounts should work. Here is a working sandbox: https://codesandbox.io/s/naughty-nash-7jixq?file=/src/App.tsx

TkDodo avatar Sep 24 '21 08:09 TkDodo

Thank you for the quick response @TkDodo ! I sort of got it working as you described, but in my actual application (not the example) the useQueryErrorResetBoundary doesn't work. I have to additionally reset the queries to actually trigger a proper refresh. Any thoughts/ideas what could be happening?

Code is looking like this:

export function useResetReactQuery() {
    const { reset } = useQueryErrorResetBoundary();
    const queryClient = useQueryClient();

    const resetReactQuery = useCallback(async () => {
        reset();
        // await queryClient.resetQueries();
    }, [queryClient, reset]);

    return resetReactQuery;
}

export function useResetErrorBoundaryOnUnmount() {
    const reset = useResetReactQuery();

    useEffect(() => {
        return () => {
            reset();
        };
    }, [reset]);
}

export function ErrorFallback() {
    useResetErrorBoundaryOnUnmount();

    return <div>Fallback</div>;
}

// ...
<ErrorBoundary fallback={<ErrorFallback />}>
  <Suspense fallback={<LoadingSpinner centered />}>
    <Information /> // this is the component using useQuery
  </Suspense>
</ErrorBoundary>

Closing the issue, since you've answered my initial question.

tobias-tengler avatar Sep 24 '21 09:09 tobias-tengler

Hi @TkDodo @tannerlinsley,

I think this issue was closed too soon.

The example above from @TkDodo only works because the "hide" state does not also have a query. This is probably why @tobias-tengler was still seeing the issue in their real application.

Here is a modified example that shows the issue: https://codesandbox.io/s/nice-banach-x79q4r Press "Toggle" a few times and you'll see that the failed query is never retried. If you comment out the useQuery hook in ComponentWithQuery2 then you'll see the first query is retried as expected.

What seems to be happening:

  1. On unmount, query1 calls reset() which sets the isReset flag.
  2. On mount, query2 immediately resets the flag. If there is no query2 the flag remains set and query1 will be retried.

This inconsistency feels unintuitive, was it intentional? What is the proper workaround?

Thanks.

codebutler avatar Oct 07 '22 14:10 codebutler

yeah, this is weird. I was thinking that with two ErrorBoundaries, you would also need two reset contexts, like I've done here:

https://codesandbox.io/s/sparkling-bird-4t4zoq?file=/src/App.tsx

But it still doesn't work, not sure ...

TkDodo avatar Oct 07 '22 15:10 TkDodo

Thanks for taking another look at this!

I think you would need to put different keys on each QueryErrorResetBoundary in your updated example for react to create a new instance when toggling states. However, that still doesn't fix the problem: https://codesandbox.io/s/suspicious-grass-juez4e

Calling reset() only toggles that flag, it doesn't invalidate any cache or trigger a render. The query has already been unmounted at this point, so it never had an opportunity to notice that the flag changed.

codebutler avatar Oct 07 '22 15:10 codebutler

yeah that makes sense - but it's still not working, is it? I don't really see why that is, because the scopes should be completely isolated by react context ...

something is off here because even if I remove query2 and the second error boundary completely, it still doesn't work:

https://codesandbox.io/s/suspicious-wilson-reie3j?file=/src/App.tsx

I'm using error boundaries as well and I don't have such a problem, but I'm also using an explicit button to trigger the reset. I think the error boundary is just never reset πŸ€”

TkDodo avatar Oct 07 '22 15:10 TkDodo

https://codesandbox.io/s/suspicious-wilson-reie3j?file=/src/App.tsx

You have a typo in here:

- const reset = useQueryErrorResetBoundary();
+ const { reset } = useQueryErrorResetBoundary();

...but that also doesn't fix the problem unless you also remove the QueryErrorResetBoundary completely. πŸ€”

codebutler avatar Oct 07 '22 15:10 codebutler

yeah sorry, that was stupid. some more attempts happening here:

https://codesandbox.io/s/blue-haze-ijtqt1?file=/src/App.tsx

it seems that we cannot react to an errorBoundary unmounting? I have tried resetKeys and onResetKeysChange as per the docs, as setting the resetKeys to something static is not working.

I think this is also why it works with explicit reset buttons: because we then really call reset. You can see in this latest example that even though I'm using resetKeys, onResetKeysChange for number 1 is never called

TkDodo avatar Oct 07 '22 15:10 TkDodo

If I'm reading this correctly, onResetKeysChange is not triggered if you go from no error to having an error. If you change query2 to also fail, both onResetKeysChange are called as expected.

codebutler avatar Oct 07 '22 17:10 codebutler

In your example onResetKeysChanged is called during the transition from error to no error, which is when we want to invalidate the query.

If within onResetKeysChanged you call queryClient.clear() instead of reset() the query re-runs as expected. So the problem isn't reset not being called, but that it doesn't have the desired effect.

codebutler avatar Oct 07 '22 18:10 codebutler

so as discussed here, it seems that resetKeys / onResetKeysChanged is not good enough:

  • https://github.com/TanStack/query/issues/3485#issuecomment-1093131919

having an effect that that listens to whatever the reset keys are and reset the boundary should work, but I can't get it to work :/

https://codesandbox.io/s/sharp-surf-2uu0kd?file=/src/App.tsx

TkDodo avatar Oct 08 '22 07:10 TkDodo

so as discussed here, it seems that resetKeys / onResetKeysChanged is not good enough:

having an effect that that listens to whatever the reset keys are and reset the boundary should work, but I can't get it to work :/

https://codesandbox.io/s/sharp-surf-2uu0kd?file=/src/App.tsx

If you add a console.log statement inside ComponentWithQuery before useQuery, it prints when you toggle back to the error state. Doesn't this mean the ErrorBoundary is being reset correctly but the query is immediately re-throwing?

function ComponentWithQuery() {
+  console.log("query!"); // would not see this if the ErrorBoundary did not reset 
   const { data } = useQuery(

The output is:

query! 
The above error occurred in the <ComponentWithQuery> component:
resetting nr 2

so the reset is happening after the query.

codebutler avatar Oct 10 '22 16:10 codebutler

looked into this again today because we're also facing it now, here is an altered example that seems to work:

  • https://codesandbox.io/s/quiet-feather-qi8x4e?file=/src/App.tsx

changes I made:

  • use FallbackComponent instead of fallback
  • in the unmount useEffect, we have the reset both the errorBoundary from react-error-boundary as well as the one from react-query.

One problem that remains is that each component needs to have its own "instance" of ErrorFallback, because otherwise, the component merely re-renders and never unmounts, so the effect doesn't run.

If we want to use the same FallbackComponent, we have to use resetKeys, as this example shows:

  • https://codesandbox.io/s/determined-raman-pnl513?file=/src/App.tsx

TkDodo avatar Jan 27 '23 12:01 TkDodo

@TkDodo I ran into this today and was quite confused by the behaviour. With retryOnMount set to true I was expecting that simply clearing the error state in a boundary thereby causing a remount of children would do the trick for retrying but I hit this issue and ended up here.

Seems like it wouldn't be bulletproof as a solution. For example if a query fails in one component but a different component then tries that same query shortly after I would assume it'll also fail without retrying in that second component. In the case of an SPA that second component could be an entirely different page if the user's recovery method was to simply navigate elsewhere.

Are you able to add some colour around the technical constraint that makes the use of this hook necessary?

cbovis avatar Feb 22 '23 17:02 cbovis

With retryOnMount set to true I was expecting that simply clearing the error state in a boundary thereby causing a remount of children would do the trick for retrying

I thought so too, but alas, it's not that simple. In my mental modal, what should happen is:

  • throw error
  • error boundary renders
  • error boundary unmounts, children render, fetch again

what actually happens is:

  • throw error
  • component renders again (!)
  • then, error boundary renders

https://twitter.com/TkDodo/status/1624720921842925574

the component re-rendering again in error state is pretty problematic for us, because we would immediately refetch. I tried to work around that, but then it kind of screwed suspense. It's pretty tricky.

Anyways, did my above solution with resetKeys not solve the issue?

TkDodo avatar Feb 23 '23 13:02 TkDodo

Hi, I met this bug earlier. I came up with a workaround, and the basic concept is:

  1. Clear the errored-query cache if error occurs. Thus, everytime when the component with the query (re)mounts, it works as if the query never happened.
  2. To acheive 1. I choose to config defaultOptions.queries.onError when creating the query client. Once the error is captured, the errored-query will get removed (via removeQueries) in a timeout (1000ms).
  3. To acheive 2, the error thrown in the query function should carry the query key, otherwise I wouldn't know which errored-query to remove. I created an error class QueryError to do so. (In other word, if onError function can get query key corresponding to the errored-query, things get simplier πŸ™‚)

Here is the PoC: https://codesandbox.io/s/error-boundary-w9s0rs

donrsh avatar Apr 10 '23 06:04 donrsh

In other word, if onError function can get query key corresponding to the errored-query, things get simplier

If you use the global onError handler of the QueryCache, you do get the whole query passed, which contains the QueryKey as well:

https://tanstack.com/query/v4/docs/react/reference/QueryCache#global-callbacks

TkDodo avatar Apr 10 '23 09:04 TkDodo

@TkDodo Thanks for advice, I adjust the PoC codes:

const queryClient = new QueryClient({
  queryCache: new QueryCache({
    onError(error, query) {
      setTimeout(() => {
        queryClient.removeQueries(query.queryKey);
      }, 1000);
    }
  }),
  defaultOptions: {
    queries: {
      retry: 1,
      suspense: true,
      useErrorBoundary: true
    }
  }
});

donrsh avatar Apr 10 '23 11:04 donrsh

I've faced up with this issue as well. @TkDodo unfortunately your last workaround doesn't work for me. Any ideas how to fix it? Thanks.

timsofteng avatar Jun 22 '23 21:06 timsofteng

@TkDodo what is the reason for setting retryOnMount to false when using suspense mode? https://github.com/TanStack/query/blob/main/packages/react-query/src/errorBoundaryUtils.ts#L32

I have 2 pages with same query, but different views. It could fail at one page, but as a user I'd prefer to have an automatic retry when I navigate to another page. This ensurePreventErrorBoundaryRetry sets retryOnMount to false and I see error message immediately without trying to refetch.

codesandbox with suspense vs non-suspense. https://codesandbox.io/s/react-query-retry-suspense-vs-no-suspense-nyqscr

mvp-v avatar Nov 08 '23 17:11 mvp-v

@mvp-v as far as I remember, react re-renders the component once more even though we already threw the error. Then, the optimisticii result would give us another loading state. What you can do is reset the boundary when you navigate away from the page. We do this as well and it works great.

TkDodo avatar Nov 14 '23 18:11 TkDodo

@mvp-v as far as I remember, react re-renders the component once more even though we already threw the error. Then, the optimisticii result would give us another loading state. What you can do is reset the boundary when you navigate away from the page. We do this as well and it works great.

I changed a bit @mvp-v https://codesandbox.io/s/react-query-retry-suspense-vs-no-suspense-nyqscr and replaced fallbackRender by FallbackComponent and used the above component. Unfortunately does not work.

function Fallback({ resetErrorBoundary }) {
  const { reset } = useQueryErrorResetBoundary();
  const navigate = useNavigate();
  useEffect(() => {
    navigate("/ns-resolve");

    return () => {
      reset();
      resetErrorBoundary();
     
    };
  }, [reset, resetErrorBoundary]);

  return <>test</>;
}

Does this What you can do is reset the boundary when you navigate away from the page. We do this as well and it works great. implemented as expected on Fallback??

karasavm avatar Nov 17 '23 17:11 karasavm

looked into this again today because we're also facing it now, here is an altered example that seems to work:

  • https://codesandbox.io/s/quiet-feather-qi8x4e?file=/src/App.tsx

changes I made:

  • use FallbackComponent instead of fallback
  • in the unmount useEffect, we have the reset both the errorBoundary from react-error-boundary as well as the one from react-query.

One problem that remains is that each component needs to have its own "instance" of ErrorFallback, because otherwise, the component merely re-renders and never unmounts, so the effect doesn't run.

If we want to use the same FallbackComponent, we have to use resetKeys, as this example shows:

  • https://codesandbox.io/s/determined-raman-pnl513?file=/src/App.tsx

@TkDodo There is a strange behaviour. If you wrap this

<QueryErrorResetBoundary>
          <ErrorBoundary FallbackComponent={Error1Fallback}>
            <Component1WithQuery />
          </ErrorBoundary>
        </QueryErrorResetBoundary>

in a component on your first example and used the component in place then it breaks the expected behaviour.

I prepared a more full example based on your first example with react-router where we can see that direct usage of ErrorBoundary works as expected but if wrapped on a component something strange is happening.

This is a react-query bug or Reacts issue because of twice rendering of error from error boundary? Is it possible to be fixed??

The question is if SCENARIO 2 on the provided example can work as expected. We want to, when we navigate on a page where an error occurred error boundary should redirect to another page and when we re visit the erroneous page react query to re fetch the data. (isLoading should appear).

https://codesandbox.io/p/sandbox/nostalgic-kalam-4qykvw?file=%2Fsrc%2FApp.tsx%3A45%2C35

karasavm avatar Nov 18 '23 12:11 karasavm

Tbh, I can see that there are a bunch of unexpected behaviours around QueryErrorResetBoundary. I would fully expect that if we call reset() in a useEffect cleanup that it would reset the boundary, but it somehow doesn't. If someone wants to take a look into this, please do.

On a broader note: I actually wanted to get rid of the whole QueryErrorResetBoundary component. Imo, it would be best if everything could work without it. The idea is that if a component is in error state, we throw to the error boundary, and the next time the component renders, we would refetch, because the query is in error state. Actually, we also don't need retryOnMount for this. It's a weird flag, because it also doesn't have anything to do with "retries". It's a refetch, but refetchOnMount is already a flag πŸ™ƒ .

One of the problems I ran into was that react renders the component one more time after we already threw the error. If we were to fetch here again, it would result in an wrong refetch / potentially infinite loop. There was also a problem with suspense afair. But I think if we could get this done, it would be the best solution because there wouldn't be a manual reset as far as Query is concerned at all.

If that sounds interesting for you to work on, please give it a go. I'm happy to give support but I won't actively go down this rabbit hole for now :)

TkDodo avatar Nov 29 '23 07:11 TkDodo

@TkDodo hi, I looked into it and majorly understood that it happens because we first return the cached result from the query and during second mount, since it already knows that it fails, the error boundary kick in. Because when I set cache time as 0, the query starts to refetch on the second mount.

What if we only cache responses if they're successful? Or does that harm us in a broader perspective?

lovishwadhwa avatar Jan 29 '24 18:01 lovishwadhwa

@TkDodo Hi, I tried to solve the bug in issue(error query again occurs when using ErrorBoundary and react-router-dom together) like this.

whenever error occurs, there are some problems with defaultOptions.queries.onError callback function removing errored queries.

First, errorUpdatedCount is always zero. Therefore, It is not possible to debug how many error queries have been tried. Second, resetQueries seems more appropriate than removeQueries. removeQuery is completely removing cached query in the background. but resetQueries initialized the value of the cached query. So it seems more semantically, in my opinion.

For this reason, I add resetErrorQuery function on QuerryErrorResetBoundary, and called it with reset method from queryResetErrorBoundary.

// react-query/src/QueryErrorResetBoundary.tsx
type QueryErrorResetBoundary = QueryErrorResetBoundaryValue & { resetErrorQuery: () => void }

const resetErrorQuery = () => {
  const queryClient = useQueryClient();
  const queryCache = queryClient.getQueryCache();
  const queryKeys = queryCache.getAll().filter((q: Query) => q.state.status === 'error');

  if (queryKeys) {
    queryKeys.forEach(({ queryKey }) => {
      queryClient.resetQueries({ queryKey });
    });
  }
};

function createValues(): QueryErrorResetBoundary {
  return { ...createValue(), resetErrorQuery }
}

const QueryErrorResetBoundaryContext = React.createContext(createValues())

export interface QueryErrorResetBoundaryProps {
  children:
    | ((value: QueryErrorResetBoundary) => React.ReactNode)
    | React.ReactNode
}

export const QueryErrorResetBoundary = ({
  children,
}: QueryErrorResetBoundaryProps) => {
  const [value] = React.useState(() => createValues())
  return (
    <QueryErrorResetBoundaryContext.Provider value={value}>
      {typeof children === 'function'
        ? (children as Function)(value)
        : children}
    </QueryErrorResetBoundaryContext.Provider>
  )
}
const ErrorUI = ({onReset}) => {
   return <button onClick={onReset}>Retry!</button>
}

const MyComponent = () => {
 (...)

 return (
    <QueryErrorResetBoundary>
      {({ reset, resetErrorQuery }) => {
      <ErrorBoundary 
          onReset = {() => {
             reset();
             resetErrorQuery();
          }} />
          fallbackRender = {({onReset}) => <ErrorUI onReset={onReset} />}
    </QueryErrorResetBoundary>
  );

So this function only initialize errored query when user clicks the button on the fallback UI of ErrorBoundary. Then, after the ErrorBoundary catches the error, the errored query is removed and the query retry works normally. Does it look okay?

mogooee avatar Mar 07 '24 06:03 mogooee

I was also bitten by this issue. I also decided I wanted more control than to just pass everything to the ErrorBoundary and have to reset it there, so have reverted to v4. I dont really feel like this behaviour makes any logical sense, and the inability to disable it just compounds the issue. There are cases when you want suspense, but don't want error boundaries, and v5 doesn't seem to support that without caching errors.

makeable avatar Jun 04 '24 16:06 makeable

There are cases when you want suspense, but don't want error boundaries, and v5 doesn't seem to support that

fwiw, react doesn't support that either ;)

TkDodo avatar Jun 04 '24 17:06 TkDodo

I have got this same issue, not sure if this is solved or not yet tho. When the error caught in my error boundary and i try to send the next subsequent query, the cached error shows up instead of the data being fetched

JonathanYon avatar Jul 16 '24 10:07 JonathanYon

and i try to send the next subsequent query

what does that mean exactly? Please show a reproduction

TkDodo avatar Jul 16 '24 11:07 TkDodo