fix(query-core): prevent re-notifying observers when unstable selector throws same error

.changeset/query-core-selector-error-loop.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/query-core': patch
+---
+
+fix(query-core): prevent re-notifying observers when an unstable selector throws the same error

packages/query-core/src/__tests__/queryObserver.test.tsx

@@ -534,6 +534,65 @@ describe('queryObserver', () => {
     })
   })
 
+  it('should not notify listeners when setOptions is called with an unstable selector throwing the same error', async () => {
+    const key = createKey()
+    const queryFn = () => Promise.resolve({ count: 1 })
+    const observer = new QueryObserver(queryClient, {
+      queryKey: key,
+      queryFn,
+      select: () => {
+        throw new Error('selector error')
+      },
+    })
+    const results: Array<QueryObserverResult> = []
+    const unsubscribe = observer.subscribe((result) => {
+      results.push(result)
+    })
+    await vi.advanceTimersByTimeAsync(10)
+    const initialNotificationCount = results.length
+
+    // Update with a new selector function reference throwing the same error
+    observer.setOptions({
+      queryKey: key,
+      queryFn,
+      select: () => {
+        throw new Error('selector error')
+      },
+    })
+    await vi.advanceTimersByTimeAsync(10)
+
+    // It should not notify listeners again because the result has not changed
+    expect(results.length).toBe(initialNotificationCount)
+
+    // When the error changes, it should update and notify listeners
+    observer.setOptions({
+      queryKey: key,
+      queryFn,
+      select: () => {
+        throw new Error('different error')
+      },
+    })
+    await vi.advanceTimersByTimeAsync(10)
+    expect(results.length).toBe(initialNotificationCount + 1)
+    expect(results[results.length - 1]?.error).toEqual(new Error('different error'))
+
+    // When the error message is same but details or cause change, it should update and notify listeners
+    observer.setOptions({
+      queryKey: key,
+      queryFn,
+      select: () => {
+        throw new Error('different error', { cause: 'cause-detail' })
+      },
+    })
+    await vi.advanceTimersByTimeAsync(10)
+    expect(results.length).toBe(initialNotificationCount + 2)
+    expect((results[results.length - 1]?.error as Error).cause).toBe(
+      'cause-detail',
+    )
+
+    unsubscribe()
+  })
+
   it('should not have isPlaceholderData true when selector throws on placeholder data', () => {
     const key = createKey()
     const queryFn = () => sleep(10).then(() => ({ count: 1 }))

packages/query-core/src/queryObserver.ts

@@ -73,6 +73,7 @@ export class QueryObserver<
     TQueryKey
   >
   #selectError: TError | null
+  #selectErrorUpdatedAt?: number
   #selectFn?: (data: TQueryData) => TData
   #selectResult?: TData
   // This property keeps track of the last query with defined data.
@@ -97,6 +98,7 @@ export class QueryObserver<
 
     this.#client = client
     this.#selectError = null
+    this.#selectErrorUpdatedAt = 0
 
     this.bindMethods()
     this.setOptions(options)
@@ -697,20 +699,27 @@ export class QueryObserver<
           data = replaceData(prevResult?.data, data, options)
           this.#selectResult = data
           this.#selectError = null
+          this.#selectErrorUpdatedAt = 0
         } catch (selectError) {
-          this.#selectError = selectError as TError
+          if (hasErrorChanged(this.#selectError, selectError)) {
+            this.#selectError = selectError as TError
+            this.#selectErrorUpdatedAt = Date.now()
+          }
         }
       }
     } else if (data === undefined) {
       // a stored select error belongs to previously selected data; once that
       // data is gone (query switch or reset), it must not leak into this result
       this.#selectError = null
+      this.#selectErrorUpdatedAt = 0
     }
 
     if (this.#selectError) {
       error = this.#selectError
       data = this.#selectResult
-      errorUpdatedAt = Date.now()
+      errorUpdatedAt =
+        this.#selectErrorUpdatedAt ||
+        (this.#selectErrorUpdatedAt = Date.now())
       status = 'error'
       isPlaceholderData = false
     }
@@ -974,3 +983,26 @@ function isStale(
     query.isStaleByTime(resolveQueryValue(options.staleTime, query))
   )
 }
+
+function hasErrorChanged(prevError: unknown, nextError: unknown): boolean {
+  if (!prevError) {
+    return true
+  }
+  if (prevError === nextError) {
+    return false
+  }
+  if (prevError instanceof Error && nextError instanceof Error) {
+    if (
+      prevError.name !== nextError.name ||
+      prevError.message !== nextError.message ||
+      (prevError as Error).cause !== (nextError as Error).cause
+    ) {
+      return true
+    }
+    return !shallowEqualObjects(prevError as any, nextError as any)
+  }
+  if (prevError instanceof Error || nextError instanceof Error) {
+    return true
+  }
+  return !shallowEqualObjects(prevError as any, nextError as any)
+}

packages/react-query/src/__tests__/useQuery.test.tsx

@@ -771,6 +771,41 @@ describe('useQuery', () => {
     expect(runs).toBe(2)
   })
 
+  it('should not trigger redundant updates when an unstable selector throws an error and the component re-renders', async () => {
+    const key = createKey()
+    const error = new Error('Select Error')
+    const states: Array<UseQueryResult<string>> = []
+
+    function Page() {
+      const [, rerender] = React.useReducer(() => ({}), {})
+      const state = useQuery<string, Error>({
+        queryKey: key,
+        queryFn: () => 'test',
+        select: () => {
+          throw error
+        },
+      })
+      states.push(state)
+      return (
+        <div>
+          <div>error: {state.error?.message}</div>
+          <button onClick={rerender}>rerender</button>
+        </div>
+      )
+    }
+
+    const rendered = renderWithClient(queryClient, <Page />)
+
+    await vi.advanceTimersByTimeAsync(0)
+    rendered.getByText('error: Select Error')
+    const countAfterError = states.length
+
+    fireEvent.click(rendered.getByRole('button', { name: 'rerender' }))
+    await vi.advanceTimersByTimeAsync(0)
+
+    expect(states.length).toBe(countAfterError + 1)
+  })
+
   it('should track properties and only re-render when a tracked property changes', async () => {
     const key = createKey()
     const states: Array<UseQueryResult<string>> = []