Repository: reduxjs/redux-toolkit

Problem statement:
Unexpected behaviour of isSuccess on second refetch
I have a simple endpoint `getData`
https://stackblitz.com/edit/react-ts-7vwdac?file=App.tsx

```ts
const delay = () => {
  return new Promise((resolve) => {
    setTimeout(resolve, 500);
  });
};

let count = 0;

const api = createApi({
  reducerPath: 'api',
  endpoints: (builder) => ({
    getData: builder.query<number, void>({
      queryFn: async () => {
        count += 1;

        await delay();

        if (count > 1) {
          return { error: true };
        }

        return { data: 123 };
      },
    }),
  }),
  baseQuery: fakeBaseQuery(),
});

const { useGetDataQuery } = api;
```

which is used like that

```tsx
function Component() {
  const { refetch, data, isError, isSuccess } = useGetDataQuery();

  console.log({ isError, isSuccess });

  return (
    <div>
      <button onClick={refetch}>refetch</button>
      <span>{data}</span>
    </div>
  );
}

export default function App() {
  return (
    <Provider store={store}>
      <Component />
    </Provider>
  );
}
```

output on mount is:
```ts
{ isError: false, isSuccess: false }
{ isError: false, isSuccess: false }
{ isError: false, isSuccess: true }
```

after calling `refetch` for the first time:
```ts
{ isError: false, isSuccess: true }
{ isError: true, isSuccess: false }
```

but if I call `refetch` one more time, I'll get
```ts
{ isError: false, isSuccess: true }
{ isError: true, isSuccess: false }
```

RTK switches `isSuccess` back to true for some reason while the request is loading

Is it the correct behaviour? Shouldn't `isSuccess` stay false?


Hints:
This appears to be intentional behaviour - `useQuery` holds onto the last successful request's `data`, and `isSuccess` is true if it has data and is currently fetching.

I don't fully understand the motivation behind this, though - hopefully somebody else can explain.

https://github.com/reduxjs/redux-toolkit/blob/ed9fc19027a31afee2234dea54fb36c8beef46ad/packages/toolkit/src/query/react/buildHooks.ts#L684-L695
Until the in-flight request returns, we _do_ have valid data from the _last completed_ request, so the _current_ status is still "successful".
Thanks for your replies!

I just need to know how many errors have occurred to show the user different error messages
```ts
const Component = () => {
    const { isError, errorCount } = useQuery();

    if (isError) {
       return errorCount > N ? 'please contact support' : 'retry button';
    }
}
```

At first I tried to do it this way, but stumbled upon the issue with isSuccess

```ts
const useErrorCount = ({ isError, isSuccess }) => {
   const [errorNum, setErrorNum] = useState(0);

   useWhenValueChanges(isError, () => {
        setErrorNum(errorNum + 1);
   });

   useWhenValueChanges(isSuccess, () => {
        setErrorNum(0);
   });
}

```
so I rewrote it like that

```ts
const useErrorCount = (status: QueryStatus) => {
  const [errorsNum, setErrorsNum] = useState(0);

  useWhenValueChanges(status, () => {
    if (status === QueryStatus.rejected) {
      setErrorsNum(errorsNum + 1);
    }
    if (status === QueryStatus.fulfilled) {
      setErrorsNum(0);
    }
  });

  return errorsNum > N;
}
```

But now I can't just call `useData()` in different components and expect errorCount to be in sync
```ts
const useData = () => {
  const { status, isError } = useQuery();
  const errorCount = useErrorCount(status);

  return { isError, errorCount }
}

// Component1.tsx
const { errorCount } = useData();
// Component2.tsx
const { errorCount } = useData();

// render Component1, fail -> refetch -> fail, errorCount = 2
// then render Component2 (errorCount is brand new)

```

I can probably store errorCount in redux and use `fulfilled` and `rejected` matchers, but It looks like a lot of pain

```ts
const api = createApi({
  reducerPath: 'api',
  endpoints: (builder) => ({
    getData: builder.query<number, void>({
      queryFn: async () => { ... },
    }),
  }),
  baseQuery: fakeBaseQuery(),
});

export const { matchRejected, matchFulfilled } = api.endpoints.getData;

createSlice({
  extraReducers: (builder) => {
    builder.addMatcher(matchFulfilled, (state) => {});
    builder.addMatcher(matchRejected, (state) => {});
  },
});
```

Is there a better way to add `errorCount`?
Storing the value in a Redux slice is the right approach. Tracking it at the component level won't give you a correct or consistent number of all possible errors that occurred.

(I'm admittedly also not sure how watching for 2 specific action types in a reducer is "a lot of pain" - that's very normal Redux logic.)
> I'm admittedly also not sure how watching for 2 specific action types in a reducer is "a lot of pain"

I think I would have to add this for every single request that I have ;) And maybe even some kind of subscription logic to `reset the state` when the last component that uses `useData()` hook gets unmounted

At a minimum, you could genericize the matcher logic:

```ts
const isMyApiError = (action) => {
  return action.type.startsWith("myapi/") && isRejected(action);
}
```
> This appears to be intentional behaviour - useQuery holds onto the last successful request's data, and isSuccess is true if it has data and is currently fetching.
> 
> I don't fully understand the motivation behind this, though - hopefully somebody else can explain.

The data that's currently available in `data` is the data returned from a successful fetch. You probably don't want that to flicker away just because a new fetch starts.  
At the same time, another fetch is going on -> both of these states are hit at the same time.

It's an attempt at simulating "suspense-like" behaviour, where you'd have the component in the last successfully rendered state, but at the same time had something like `pending` available to gray parts of the UI out or show a spinner.

That said, this "flickering" `isSuccess` doesn't seem intended to me.
@markerikson 
the only thing that comes to mind is

```ts
const hasRTKQMeta = ( action: any ): action is { meta: { arg: { endpointName: string } } } => action?.meta?.arg?.endpointName;

export const referenceSlice = createSlice({
  extraReducers: (builder) => {
    builder.addMatcher(
      (action) => action.type.startsWith('apiCache') && isRejected(action),
      (state, action) => {
        if (hasRTKQMeta(action)) {
          state.errors[action.meta.arg.endpointName] += 1;
        }
      },
    );
  ...
  },
});
```


```ts
const useData = () => {
  const { endpointName, ...rest } = useQuery()
  const { errors } = useAppSelector((state) => state.slice);
  
  return { contactSupport: errors[endpointName] > N, ...rest };
}
```

\+ some reset state logic on top of that hack ;)

I see that `@tanstack/react-query` has [errorUpdateCount](https://github.com/TanStack/query/discussions/3525#discussioncomment-2593717) flag
```ts
  const { errorUpdateCount } = useQuery()
```
Is there any change something like this will end up in RTK as well?
@rusnick31 not immediately likely. This is the first time anyone's ever asked for something like an error count that I know of.  Individual feature requests are always a possibility, but we generally try to avoid adding things unless it feels like there's enough usefulness _and_ a wide enough audience to justify adding something.  (After all, any feature we ship adds bytes to bundle size for everyone.)

In the React-Query case, it looks like that error count was something they were already tracking internally for some reason and exposed it publicly.  We don't keep track of error counts ourselves that I know of.

So, I'm not saying "no" permanently, but given that this seems doable in userland and this is the first time the question of "counting errors" has ever come up, I don't see an immediate reason to add this.

Patch:
diff --git a/packages/toolkit/src/query/react/buildHooks.ts b/packages/toolkit/src/query/react/buildHooks.ts
--- a/packages/toolkit/src/query/react/buildHooks.ts
+++ b/packages/toolkit/src/query/react/buildHooks.ts
@@ -934,12 +934,13 @@ export function buildHooks<Definitions extends EndpointDefinitions>({
       !hasData &&
       isFetching
 
-    // isSuccess = true when data is present.
+    // isSuccess = true when data is present and we're not refetching after an error.
     // That includes cases where the _current_ item is either actively
     // fetching or about to fetch due to an uninitialized entry.
     const isSuccess =
       currentState.isSuccess ||
-      ((isFetching || currentState.isUninitialized) && hasData)
+      (hasData &&
+        ((isFetching && !lastResult?.isError) || currentState.isUninitialized))
 
     return {
       ...currentState,


Test patch:
diff --git a/packages/toolkit/src/query/tests/buildHooks.test.tsx b/packages/toolkit/src/query/tests/buildHooks.test.tsx
--- a/packages/toolkit/src/query/tests/buildHooks.test.tsx
+++ b/packages/toolkit/src/query/tests/buildHooks.test.tsx
@@ -39,6 +39,7 @@ import type { MockInstance } from 'vitest'
 // the refetching behavior of components.
 let amount = 0
 let nextItemId = 0
+let refetchCount = 0
 
 interface Item {
   id: number
@@ -87,6 +88,17 @@ const api = createApi({
         },
       }),
     }),
+    getUserWithRefetchError: build.query<{ name: string }, number>({
+      queryFn: async (id) => {
+        refetchCount += 1
+
+        if (refetchCount > 1) {
+          return { error: true } as any
+        }
+
+        return { data: { name: 'Timmy' } }
+      },
+    }),
     getIncrementedAmount: build.query<{ amount: number }, void>({
       query: () => ({
         url: '',
@@ -431,6 +443,85 @@ describe('hooks tests', () => {
       ])
     })
 
+    test('isSuccess stays consistent if there is an error while refetching', async () => {
+      type LoadingState = {
+        id: number
+        isFetching: boolean
+        isSuccess: boolean
+        isError: boolean
+      }
+      const loadingHistory: LoadingState[] = []
+
+      function Component({ id = 1 }) {
+        const queryRes = api.endpoints.getUserWithRefetchError.useQuery(id)
+        const { refetch, data, status } = queryRes
+
+        useEffect(() => {
+          const { isFetching, isSuccess, isError } = queryRes
+          loadingHistory.push({ id, isFetching, isSuccess, isError })
+        }, [id, queryRes])
+
+        return (
+          <div>
+            <button
+              onClick={() => {
+                console.log('Refetching')
+                refetch()
+              }}
+            >
+              refetch
+            </button>
+            <div data-testid="name">{data?.name}</div>
+            <div data-testid="status">{status}</div>
+          </div>
+        )
+      }
+
+      render(<Component />, { wrapper: storeRef.wrapper })
+
+      await waitFor(() =>
+        expect(screen.getByTestId('name').textContent).toBe('Timmy'),
+      )
+
+      fireEvent.click(screen.getByText('refetch'))
+
+      await waitFor(() =>
+        expect(screen.getByTestId('status').textContent).toBe('pending'),
+      )
+
+      await waitFor(() =>
+        expect(screen.getByTestId('status').textContent).toBe('rejected'),
+      )
+
+      fireEvent.click(screen.getByText('refetch'))
+
+      await waitFor(() =>
+        expect(screen.getByTestId('status').textContent).toBe('pending'),
+      )
+
+      await waitFor(() =>
+        expect(screen.getByTestId('status').textContent).toBe('rejected'),
+      )
+
+      expect(loadingHistory).toEqual([
+        // Initial renders
+        { id: 1, isFetching: true, isSuccess: false, isError: false },
+        { id: 1, isFetching: true, isSuccess: false, isError: false },
+        // Data is returned
+        { id: 1, isFetching: false, isSuccess: true, isError: false },
+        // Started first refetch
+        { id: 1, isFetching: true, isSuccess: true, isError: false },
+        // First refetch errored
+        { id: 1, isFetching: false, isSuccess: false, isError: true },
+        // Started second refetch
+        // IMPORTANT We expect `isSuccess` to still be false,
+        // despite having started the refetch again.
+        { id: 1, isFetching: true, isSuccess: false, isError: false },
+        // Second refetch errored
+        { id: 1, isFetching: false, isSuccess: false, isError: true },
+      ])
+    })
+
     test('useQuery hook respects refetchOnMountOrArgChange: true', async () => {
       let data, isLoading, isFetching
       function User() {