From c363592d2cd612a9fac73b79f57d15a93bbbde92 Mon Sep 17 00:00:00 2001 From: Mohamed EL AYADI Date: Thu, 12 Sep 2024 12:36:30 +0100 Subject: [PATCH 1/3] Add reproduction test for infinite loop retries (issue #8046) --- .../__tests__/infiniteQueryBehavior.test.tsx | 78 ++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx index 29367cad4f..4e24c627a1 100644 --- a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx +++ b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' import { waitFor } from '@testing-library/react' import { CancelledError, InfiniteQueryObserver } from '..' import { createQueryClient, queryKey, sleep } from './utils' -import type { InfiniteQueryObserverResult, QueryCache, QueryClient } from '..' +import type { InfiniteData, InfiniteQueryObserverResult, QueryCache , QueryClient} from '..' describe('InfiniteQueryBehavior', () => { let queryClient: QueryClient @@ -323,4 +323,80 @@ describe('InfiniteQueryBehavior', () => { unsubscribe() }) + + // Ensures https://github.com/TanStack/query/issues/8046 is resolved + test('InfiniteQueryBehavior should not enter an infinite loop when a page errors while retry is on', async () => { + let errorCount = 0 + const key = queryKey() + + interface TestResponse { + data: Array<{ id: string }> + nextToken?: number + } + + const fakeData = [ + { data: [{ id: 'item-1' }], nextToken: 1 }, + { data: [{ id: 'item-2' }], nextToken: 2 }, + { data: [{ id: 'item-3' }], nextToken: 3 }, + { data: [{ id: 'item-4' }] }, + ] + + const fetchData = async ({ nextToken = 0 }: { nextToken?: number }) => + new Promise((resolve, reject) => { + console.log(`getting page ${nextToken}...`) + setTimeout(() => { + if (nextToken == 2 && errorCount < 3) { + errorCount += 1 + console.error('rate limited!') + reject({ statusCode: 429 }) + return + } + resolve(fakeData[nextToken] as TestResponse) + console.log(`got page ${nextToken}`) + }, 500) + }) + + const observer = new InfiniteQueryObserver< + TestResponse, + Error, + InfiniteData, + TestResponse, + typeof key, + number + >(queryClient, { + retry: 5, + staleTime: 0, + retryDelay: 10, + + queryKey: key, + initialPageParam: 1, + getNextPageParam: (lastPage) => lastPage.nextToken, + queryFn: ({ pageParam }) => fetchData({ nextToken: pageParam }), + }) + + // Fetch Page 1 + const page1Data = await observer.fetchNextPage() + expect(page1Data.data?.pageParams).toEqual([1]) + + // Fetch Page 2, as per the queryFn, this will reject 2 times then resolves + const page2Data = await observer.fetchNextPage() + expect(page2Data.data?.pageParams).toEqual([1, 2]) + + // Fetch Page 3 + const page3Data = await observer.fetchNextPage() + expect(page3Data.data?.pageParams).toEqual([1, 2, 3]) + + // Now the real deal; re-fetching this query **should not** stamp into an + // infinite loop where the retryer every time restarts from page 1 + // once it reaches the page where it errors. + // For this to work, we'd need to reset the error count so we actually retry + console.log('###################################################') + errorCount = 0 + const reFetchedData = await observer.refetch() + + // Before resolution (while the bug persists), this will never be reached + // and the test will timeout. + expect(reFetchedData.data?.pageParams).toEqual([1, 2, 3]) + console.log(reFetchedData.data?.pageParams) + }) }) From 4f568ad0047bdffb5179295c2b9169afd6faef7e Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 12 Sep 2024 11:38:14 +0000 Subject: [PATCH 2/3] ci: apply automated fixes --- .../src/__tests__/infiniteQueryBehavior.test.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx index 4e24c627a1..dd1ae5ca31 100644 --- a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx +++ b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx @@ -2,7 +2,12 @@ import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' import { waitFor } from '@testing-library/react' import { CancelledError, InfiniteQueryObserver } from '..' import { createQueryClient, queryKey, sleep } from './utils' -import type { InfiniteData, InfiniteQueryObserverResult, QueryCache , QueryClient} from '..' +import type { + InfiniteData, + InfiniteQueryObserverResult, + QueryCache, + QueryClient, +} from '..' describe('InfiniteQueryBehavior', () => { let queryClient: QueryClient From 03fa7e1ed612de0cd0e8eba138409aa57e199198 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Thu, 12 Sep 2024 14:59:43 +0200 Subject: [PATCH 3/3] fix: retry for infinite queries The retryer lives above the fetchFn, and it re-runs the fetchFn whenever a retry happens. Usually, the fetchFn is a thin wrapper around the actual queryFn passed by the user. However, for infinite queries, it fetches all pages in a loop. The retryer breaks out of this loop if an error occurs on e.g. the second page, and then retries by running the fetchFn - which will re-set the loop This fix hoists the currentPage counter out of the fetchFn - into the closure created by onFetch. The outer closure is created from running `query.fetch` once, so it won't be re-set between retries. The fix also re-writes the fetch loop to always take the `currentPage` into account, where it was previously treating the first page differently --- .../__tests__/infiniteQueryBehavior.test.tsx | 12 ++----- .../query-core/src/infiniteQueryBehavior.ts | 33 +++++++++---------- 2 files changed, 17 insertions(+), 28 deletions(-) diff --git a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx index dd1ae5ca31..1ca35fdfcf 100644 --- a/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx +++ b/packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx @@ -329,8 +329,7 @@ describe('InfiniteQueryBehavior', () => { unsubscribe() }) - // Ensures https://github.com/TanStack/query/issues/8046 is resolved - test('InfiniteQueryBehavior should not enter an infinite loop when a page errors while retry is on', async () => { + test('InfiniteQueryBehavior should not enter an infinite loop when a page errors while retry is on #8046', async () => { let errorCount = 0 const key = queryKey() @@ -348,17 +347,14 @@ describe('InfiniteQueryBehavior', () => { const fetchData = async ({ nextToken = 0 }: { nextToken?: number }) => new Promise((resolve, reject) => { - console.log(`getting page ${nextToken}...`) setTimeout(() => { if (nextToken == 2 && errorCount < 3) { errorCount += 1 - console.error('rate limited!') reject({ statusCode: 429 }) return } resolve(fakeData[nextToken] as TestResponse) - console.log(`got page ${nextToken}`) - }, 500) + }, 10) }) const observer = new InfiniteQueryObserver< @@ -395,13 +391,9 @@ describe('InfiniteQueryBehavior', () => { // infinite loop where the retryer every time restarts from page 1 // once it reaches the page where it errors. // For this to work, we'd need to reset the error count so we actually retry - console.log('###################################################') errorCount = 0 const reFetchedData = await observer.refetch() - // Before resolution (while the bug persists), this will never be reached - // and the test will timeout. expect(reFetchedData.data?.pageParams).toEqual([1, 2, 3]) - console.log(reFetchedData.data?.pageParams) }) }) diff --git a/packages/query-core/src/infiniteQueryBehavior.ts b/packages/query-core/src/infiniteQueryBehavior.ts index 5db6e34ba1..04de4f7a30 100644 --- a/packages/query-core/src/infiniteQueryBehavior.ts +++ b/packages/query-core/src/infiniteQueryBehavior.ts @@ -13,14 +13,15 @@ export function infiniteQueryBehavior( ): QueryBehavior> { return { onFetch: (context, query) => { + const options = context.options as InfiniteQueryPageParamsOptions + const direction = context.fetchOptions?.meta?.fetchMore?.direction + const oldPages = context.state.data?.pages || [] + const oldPageParams = context.state.data?.pageParams || [] + let result: InfiniteData = { pages: [], pageParams: [] } + let currentPage = 0 + const fetchFn = async () => { - const options = context.options as InfiniteQueryPageParamsOptions - const direction = context.fetchOptions?.meta?.fetchMore?.direction - const oldPages = context.state.data?.pages || [] - const oldPageParams = context.state.data?.pageParams || [] - const empty = { pages: [], pageParams: [] } let cancelled = false - const addSignalProperty = (object: unknown) => { Object.defineProperty(object, 'signal', { enumerable: true, @@ -78,8 +79,6 @@ export function infiniteQueryBehavior( } } - let result: InfiniteData - // fetch next / previous page? if (direction && oldPages.length) { const previous = direction === 'backward' @@ -92,22 +91,20 @@ export function infiniteQueryBehavior( result = await fetchPage(oldData, param, previous) } else { - // Fetch first page - result = await fetchPage( - empty, - oldPageParams[0] ?? options.initialPageParam, - ) - const remainingPages = pages ?? oldPages.length - // Fetch remaining pages - for (let i = 1; i < remainingPages; i++) { - const param = getNextPageParam(options, result) + // Fetch all pages + do { + const param = + currentPage === 0 + ? (oldPageParams[0] ?? options.initialPageParam) + : getNextPageParam(options, result) if (param == null) { break } result = await fetchPage(result, param) - } + currentPage++ + } while (currentPage < remainingPages) } return result