Skip to content

Inconsistent refetching state for errored queries #5988

Closed as not planned
Closed as not planned

Description

@lundal

Describe the bug

Refetching an errored query displays inconsistent statuses. If the query has never been successful it flips between (loading, fetching) and (error, idle). If it has been successful once it flips between (error, fetching) and (error, idle). The error object is unavailable while fetching it the first case, but not the latter.

Your minimal, reproducible example

https://codesandbox.io/s/react-query-inconsistent-error-status-3n4ny9

Steps to reproduce

  1. Go to https://codesandbox.io/s/react-query-inconsistent-error-status-3n4ny9
  2. Let the example run for a few seconds
  3. See the inconsistent status updates between the two queries

Expected behavior

I would expect the query status to flip between (error, fetching) and (error, idle) and the error to be available regardless of whether it has once succeeded or not.

How often does this bug happen?

None

Screenshots or Videos

State differences when refetching:
Screenshot from 2023-09-11 11-21-07 Screenshot from 2023-09-11 11-21-52

From the example:
Screencast from 2023-09-11 14-02-06.webm

Platform

  • OS: Linux
  • Browser: Firefox 102 / Chrome 116

Tanstack Query adapter

react-query

TanStack Query version

v4.33.0

TypeScript version

v5.1.6

Additional context

No response

Activity

  1. TkDodo commented on Sep 11, 2023

    @TkDodo
    Collaborator

    This is on purpose. "Hard" errors (errors where we've never successfully fetched before) are reset and the query goes back to loading state.

    ...(!state.dataUpdatedAt && {
    error: null,
    status: 'pending',
    }),

    The main reason is that we want to go back to loading state so that users can display a fallback spinner instead of the error again. However, to be in loading state, we can't have errors (that's how that state is defined: no data and no error).

    In the case of a background error (where we previously had successfully fetched data), we keep both the data and the error to let the user decide what they want to display.

  2. lundal commented on Sep 11, 2023

    @lundal
    Author

    It seems really unintuitive to have background refetches behave differently depending on if the query has succeeded once upon a time or not.

    Why not let the user decide what they want to display in both cases? If they want to show a spinner they can always check the fetching state or reset the query completely.

    Unless the query has succeded at least once, there does not seem to be a way to display the error while refetching. This causes the page to flip between a spinner and an error message in our case.

  3. TkDodo commented on Sep 11, 2023

    @TkDodo
    Collaborator

    Why not let the user decide what they want to display in both cases? If they want to show a spinner they can always check the fetching state or reset the query completely.

    because most cases are implemented like this:

    const { data, isLoading, isError } = useQuery(...)
    
    if (isLoading) return <Loading />
    if (isError) return <Error />
    
    return data.map(...)
    

    with that, you wouldn't get a loader when you get a refetch after an error has occurred - you would explicitly need to check for that.

    It's also important to not cache these errors when using error boundaries. If errors are thrown to error boundaries, the error boundary will be displayed when an error occurs. If you click retry in the boundary, it unmounts and the component will be shown again. In those cases, you also want a query to be in loading state (and not in error state).

    Unless the query has succeded at least once, there does not seem to be a way to display the error while refetching. This causes the page to flip between a spinner and an error message in our case.

    Yes, we don't support that. This hasn't come up until now, so I'm wondering why you would like to keep showing the error instead of a loader?

  4. lundal commented on Sep 11, 2023

    @lundal
    Author
    const { data, isLoading, isError } = useQuery(...)
    
    if (isLoading) return <Loading />
    if (isError) return <Error />
    
    return data.map(...)
    

    That example will display the same inconsistency. If the query succeeds once and then continually fails during refetches, it will show <Loading/> → <Data/> → <Error/>. If the query fails from the beginning, it will show <Loading/> → <Error/> → <Loading/> → <Error/> → <Loading/> → <Error/> → ....

    I'm wondering why you would like to keep showing the error instead of a loader?

    For the same reason that we want to show stale data instead of a loader. Keep the UI stable while refetching in the background. Prevent the page from jumping back and forth between a loader and an error message. Be consistent.

    Our app uses quite short refetch intervals (1s - 10s), which exacerbate the issue. We are looking into alternate solutions, but this seems like a rather big inconsistency.

  5. TkDodo commented on Sep 11, 2023

    @TkDodo
    Collaborator

    I think it would be worse to keep displaying the error, without any indication that a refetch is taking place, because if it errors again, the users will not know how to get rid of the errors.

    My personal preference is to use error boundaries for first-time errors, and there, I would definitely expect another loading state once the retry in the boundary is confirmed. This is also about: when I use error boundaries, I expect my component to never render with an error, but if we keep errors in place, it would. It's similar to suspense where you also don't expect your component to render in loading state.

  6. marreman commented on Sep 14, 2023

    @marreman

    I have the same issue as @lundal. I'd like to be able to keep displaying the error and possibly also a spinner, while retrying. We do not use error boundaries or suspense yet, I have not read up on them, so I don't know how/if they would change the situation and enable some new possibilities.

  7. TkDodo commented on Sep 14, 2023

    @TkDodo
    Collaborator

    We do not use error boundaries or suspense yet

    just fyi, you can use error boundaries without suspense. it's a standard feature of react that has nothing to do with suspense.

    I'd like to be able to keep displaying the error and possibly also a spinner, while retrying.

    we have to distinguish between retrying and the actual error state. While the 3 default retries are going on, we expose the in between failures as failureReason (and failureCount is also available). You can display them.

    but when everything fails, you go into error state and that error is available to display. If the Query then refetches, it goes back to error state. I'm not sure how you trigger failed queries to refetch?

    • If it's a user interaction (that's how you'd do it with error boundaries), it would be weird to keep displaying the error because the user has "acknowledged" it. If you want that, you need to capture the error in your own local state.
    • If it's because of an automatic refetch (like window focus), you can disable those if you want:
    refetchOnWindowFocus: (query) => query.state.status !== 'error'
    
  8. marreman commented on Sep 14, 2023

    @marreman

    just fyi, you can use error boundaries without suspense. it's a standard feature of react that has nothing to do with suspense.

    Ah, ok, thanks!

    Yes, we have automatic refetch enabled and would like to keep it also for error states so that the app can try recover automatically. It makes sense for the query to be in a loading state while retrying, this is not about those retries.

    Maybe the difference is that for us, only a small part of the UI might be in an error state, while the rest of the page keeps being usable. The error state may stem from a user misconfiguring stuff and might take days to get fixed. So automatic refresh will most likely result in another error. The request is quick though, so the loader just flickers by.

    So maybe, as you write, I'll have to capture errors and successes myself, and store them, to get the desired behavior.

  9. TkDodo commented on Sep 14, 2023

    @TkDodo
    Collaborator

    So maybe, as you write, I'll have to capture errors and successes myself, and store them, to get the desired behavior.

    Yes, that's something you can always do.

    Maybe the difference is that for us, only a small part of the UI might be in an error state, while the rest of the page keeps being usable. The error state may stem from a user misconfiguring stuff and might take days to get fixed. So automatic refresh will most likely result in another error.

    I would say that this would be a prime example for an error boundary: You'd wrap the <ErrorBoundary> around the part of the screen that is affected by the misconfiguration, and show an Error for the user how to fix it + a Retry button. The users can then change the configuration, come back, click retry, and they would then see the hard loading state because they've dismissed the error.

    The request is quick though, so the loader just flickers by.

    Not with retries and exponential backoff :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions