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

useSuspenseQuery does not retry after fetchQuery returned an error #7606

Closed
visualjerk opened this issue Jun 21, 2024 · 4 comments
Closed

useSuspenseQuery does not retry after fetchQuery returned an error #7606

visualjerk opened this issue Jun 21, 2024 · 4 comments

Comments

@visualjerk
Copy link
Contributor

visualjerk commented Jun 21, 2024

Describe the bug

When using useSuspenseQuery in a component that is mounted after a fetchQuery with the same queryKey encountered an error, useSuspenseQuery returns the same error without trying to call the queryFn again.

Your minimal, reproducible example

https://stackblitz.com/edit/tanstack-query-hgaemq?file=src%2Findex.jsx

Steps to reproduce

  1. Open browser dev tools
  2. Click on "Show component with query"
  3. The page goes blank and a console warning is logged: "An error occurred in the component"

Expected behavior

As a user, I would expect useSuspenseQuery to call the queryFn again, as the query cache does not hold any valid data.

How often does this bug happen?

None

Screenshots or Videos

No response

Platform

  • OS: macOS
  • Browser: Chrome
  • Version: 126.0.6478.62

Tanstack Query adapter

react-query

TanStack Query version

5.45.1

TypeScript version

No response

Additional context

We noticed this while using fetchQuery in a router loader in combination with useSuspenseQuery in the route component.

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 22, 2024

I see what's going on here. The problem is that when a suspense query errors and has no data yet, it throws an error to the error boundary. When that happens, you need to explicitly tell the library when it's fine to re-fetch the query, because react will re-render components even though they already threw the error once more before showing the error boundary. It's an implementation detail of react, but for us, it would mean another refetch attempt.

So there is no refetch happening because the error boundary wasn't reset. In your example, there is no error boundary at all, that's why you see the white page. If there was an error boundary with the recommended integration, you would see the boundary and be able to reset it.

We noticed this while using fetchQuery in a router loader in combination with useSuspenseQuery in the route component.

I'm not quite following here. When a route loader throws an error, it should show the route error boundary. The route component that uses useSuspenseQuery wouldn't render. But again, even if, throwing "the same error" to the error boundary seems okay.

@visualjerk
Copy link
Contributor Author

Thanks for investigating! If I understand correctly, the case you described seems to be slightly different. The first fetch is not triggered by a suspense query, but rather by a fetchQuery (or prefetchQuery). Therefore the recommended error boundary does not catch the initial error.

The second time the queryFn runs, it does not throw an error. That is why I would expect the suspense query to fetch successfully and the component to render. However the suspense query does not call the queryFn at all and instead instantly returns the error previously encountered in the fetchQuery.

Here is an updated example with an error boundary..

I would expect the component with the suspense query to render in this case:

  • Click "Fetch query" (Fetches query and runs into error)
  • Click "Show component with query"

Instead the error boundary renders its fallback.

It seems like the initial fetchQuery primes the query error boundary, even so it is not triggered from inside a suspense query. Is that to be expected? If so, is there a sensible way to handle this case?

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 24, 2024

The first fetch is not triggered by a suspense query, but rather by a fetchQuery (or prefetchQuery). Therefore the recommended error boundary does not catch the initial error.

why not? you mentioned route loaders? Not sure which router you are using, but both react-router and tanstack-router integrate with error boundaries, so if you throw an error in a route loader (which fetchQuery does), it should show the error boundary. Not that prefetchQuery does not throw errors.

Instead the error boundary renders its fallback.

Like I said, I think that's expected. useSuspenseQuery cannot trigger a new fetch because the error hasn't been reset yet with useQueryErrorResetBoundary. This happens here:

if (options.suspense || options.throwOnError) {
// Prevent retrying failed query if the error boundary has not been reset yet
if (!errorResetBoundary.isReset()) {
options.retryOnMount = false
}
}
}

So when fetchQuery primes a query in error state, useSuspenseQuery picks up that error, sees that we haven't reset yet, so it re-throws to show the error. Then, when the user resets, it re-fetches when it mounts again.

is there a sensible way to handle this case?

it seems like you are expecting fetches triggered by fetchQuery to behave differently than fetches triggered from useSuspenseQuery. That is not the case. The Query itself doesn't care what triggers it.

One way to go about is to catch errors from fetchQuery and then immediately remove the error'd query with queryClient.removeQueries. It seems like you are not interested in the error that happens in the route loader. I think the actual flow should be:

  • route loader fetches and it errors
  • the error boundary is shown
  • user retries the error
  • route loader runs again (or component renders, doesn't matter)
  • fetch succeeds and component is shown

@visualjerk
Copy link
Contributor Author

visualjerk commented Jun 25, 2024

Not sure which router you are using, but both react-router and tanstack-router integrate with error boundaries, so if you throw an error in a route loader (which fetchQuery does), it should show the error boundary. Not that prefetchQuery does not throw errors.

We are using tanstack-router and use the loader in a similar way to what is described in the tanstack-query docs. Simplified our setup looks like this:

// src/routes/route.tsx
export const Route = createFileRoute('/route')({
  component: () => <RouteComponent />,
  loader() {
    fetchQueryOne();
    fetchQueryTwo();
  },
});

function RouteComponent() {
  const queryOne = useSuspenseQueryOne()

  return (
    ...
    <Suspense fallback="loading ...">
      <NestedComponent />
    </Suspense>
  )
}

function NestedComponent() {
  const queryTwo = useSuspenseQueryTwo()

  return (
    ...
  )
}
// src/routes/__root.tsx
export const Route = createRootRoute({
  component: () => {
    if (!isAuthenticated()) {
      return <Login />
    }

    return <Outlet />
  },
});

We utilize the route loader to avoid waterfall loading of suspense queries but still want to control where to show a suspense fallback deeper inside the component tree. Additionally we don't render the outlet at all, when a user is not authenticated.

I guess the issue with this setup is, that we do not await the queries in the loader and therefore the router error boundary does not get triggered. We also do not render the route component in case of missing authentication, so the suspense queries do not even get triggered.

This results in this behavior:

  1. auth token runs out (e.g. due to inactivity)
  2. user returns and loads the page
  3. route loader runs and triggers the queries
  4. login component is rendered, as the user does not have a valid auth token
  5. queries run into auth error, which primes the query error boundary
  6. user logs in again
  7. route component triggers the suspense queries
  8. suspense queries return the error
  9. router error boundary is shown
  10. user retries
  11. route component renders

From a user perspective it seems weird that the error boundary is shown with an auth error, even so she just logged in.

One way to go about is to catch errors from fetchQuery and then immediately remove the error'd query with queryClient.removeQueries.

Thanks for the idea! I was looking for something like queryClient.resetErroBoundary, but did not see the connection to removeQueries. If we stick with our current setup this might indeed be the best way to handle it.

Other than that it might be best to rethink authentication handling in our router setup and use the redirect approach instead to avoid the route loader runs for unauthenticated users.

Really appreciate your help and insights!

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

No branches or pull requests

2 participants