diff --git a/packages/react-relay/relay-hooks/__tests__/usePrefetchableForwardPaginationFragment-test.js b/packages/react-relay/relay-hooks/__tests__/usePrefetchableForwardPaginationFragment-test.js index d5f18755cdcb6..ad5b87d86ae8f 100644 --- a/packages/react-relay/relay-hooks/__tests__/usePrefetchableForwardPaginationFragment-test.js +++ b/packages/react-relay/relay-hooks/__tests__/usePrefetchableForwardPaginationFragment-test.js @@ -41,6 +41,7 @@ component Container( userRef: ?usePrefetchableForwardPaginationFragmentTest_user$key, minimalEdgesToFetch: number = 1, UNSTABLE_extraVariables?: unknown, + maxFetchSize?: ?number, ) { const { edges, @@ -76,6 +77,8 @@ component Container( UNSTABLE_extraVariables, }, minimalEdgesToFetch, + false, + maxFetchSize, ); loadMore = loadNext; refetch = _refetch; @@ -1108,3 +1111,354 @@ it('getServerEdges should return all unfiltered server edges', async () => { }, ]); }); + +it('should cap the prefetch request size at `maxFetchSize`', async () => { + const fragmentKey = environment.lookup(query.fragment).data?.node; + // render the initial page + let app; + await act(() => { + app = create( + + + {/* $FlowFixMe[incompatible-type] */} + + + , + ); + }); + if (app == null) { + throw new Error('should not be null'); + } + expect(app.toJSON()).toEqual('node1/0'); + + // The buffer size is 2, but `maxFetchSize` caps each request at 1 so we don't + // generate a query large enough to overwhelm the backend. + expect(environment.mock.getAllOperations().length).toBe(1); + expect(environment.mock.getAllOperations()[0].fragment.variables.first).toBe( + 1, + ); + + await act(() => { + environment.mock.resolveMostRecentOperation({ + data: { + node: { + __typename: 'User', + id: '1', + name: 'Alice', + friends: { + edges: [ + { + cursor: 'cursor:2', + node: { + __typename: 'User', + id: 'node:2', + name: 'node2', + username: 'username:node:2', + }, + }, + ], + pageInfo: { + startCursor: 'cursor:2', + endCursor: 'cursor:2', + hasNextPage: true, + hasPreviousPage: true, + }, + }, + }, + }, + }); + }); + + expect(app.toJSON()).toEqual('node1/1'); + + // The buffer still isn't full, so it prefetches again, again capped at 1. + expect(environment.mock.getAllOperations().length).toBe(1); + expect(environment.mock.getAllOperations()[0].fragment.variables.first).toBe( + 1, + ); +}); + +it('should cap an explicit `loadNext` request size at `maxFetchSize`', async () => { + const fragmentKey = environment.lookup(query.fragment).data?.node; + // render the initial page + let app; + await act(() => { + app = create( + + + {/* $FlowFixMe[incompatible-type] */} + + + , + ); + }); + if (app == null) { + throw new Error('should not be null'); + } + expect(app.toJSON()).toEqual('node1/0'); + + // Fulfill the initial prefetch so there is no pending operation. + await act(() => { + environment.mock.resolveMostRecentOperation({ + data: { + node: { + __typename: 'User', + id: '1', + name: 'Alice', + friends: { + edges: [ + { + cursor: 'cursor:2', + node: { + __typename: 'User', + id: 'node:2', + name: 'node2', + username: 'username:node:2', + }, + }, + { + cursor: 'cursor:3', + node: { + __typename: 'User', + id: 'node:3', + name: 'node3', + username: 'username:node:3', + }, + }, + ], + pageInfo: { + startCursor: 'cursor:3', + endCursor: 'cursor:3', + hasNextPage: true, + hasPreviousPage: true, + }, + }, + }, + }, + }); + }); + expect(app.toJSON()).toEqual('node1/2'); + + // Ask for many more items than exist in the buffer. Without a cap this would + // request `numToAdd + buffer` items; `maxFetchSize` limits it to 3. + await act(() => { + loadMore(100); + }); + expect(environment.mock.getAllOperations().length).toBe(1); + expect(environment.mock.getAllOperations()[0].fragment.variables.first).toBe( + 3, + ); +}); + +it('should stop prefetching after a pagination request errors to avoid thrashing the server', async () => { + const fragmentKey = environment.lookup(query.fragment).data?.node; + // render the initial page + let app; + await act(() => { + app = create( + + + {/* $FlowFixMe[incompatible-type] */} + + + , + ); + }); + if (app == null) { + throw new Error('should not be null'); + } + expect(app.toJSON()).toEqual('node1/0'); + + // It starts prefetching to fill the buffer. + expect(environment.mock.getAllOperations().length).toBe(1); + + // The server errors on the pagination request. + await act(() => { + environment.mock.rejectMostRecentOperation(new Error('uh oh')); + }); + + // Prefetching must NOT immediately retry, even though the buffer is not full + // and `hasNext` is still true. Otherwise it would thrash the server (DDOS). + expect(environment.mock.getAllOperations().length).toBe(0); +}); + +it('should resume prefetching after an error once `loadNext` is explicitly called', async () => { + const fragmentKey = environment.lookup(query.fragment).data?.node; + // render the initial page + let app; + await act(() => { + app = create( + + + {/* $FlowFixMe[incompatible-type] */} + + + , + ); + }); + if (app == null) { + throw new Error('should not be null'); + } + expect(app.toJSON()).toEqual('node1/0'); + expect(environment.mock.getAllOperations().length).toBe(1); + + await act(() => { + environment.mock.rejectMostRecentOperation(new Error('uh oh')); + }); + // Automatic prefetching is paused. + expect(environment.mock.getAllOperations().length).toBe(0); + + // An explicit `loadNext` is a deliberate user action, so it should retry. + await act(() => { + loadMore(1); + }); + expect(environment.mock.getAllOperations().length).toBe(1); + + await act(() => { + environment.mock.resolveMostRecentOperation({ + data: { + node: { + __typename: 'User', + id: '1', + name: 'Alice', + friends: { + edges: [ + { + cursor: 'cursor:2', + node: { + __typename: 'User', + id: 'node:2', + name: 'node2', + username: 'username:node:2', + }, + }, + ], + pageInfo: { + startCursor: 'cursor:2', + endCursor: 'cursor:2', + hasNextPage: true, + hasPreviousPage: true, + }, + }, + }, + }, + }); + }); + expect(app.toJSON()).toEqual('node1,node2/0'); +}); + +it('should resume prefetching after an error when a later `loadNext` is served from the buffer', async () => { + const fragmentKey = environment.lookup(query.fragment).data?.node; + // render the initial page + let app; + await act(() => { + app = create( + + + {/* $FlowFixMe[incompatible-type] */} + + + , + ); + }); + if (app == null) { + throw new Error('should not be null'); + } + expect(app.toJSON()).toEqual('node1/0'); + + // Initial prefetch fills the buffer (bufferSize = 2). + expect(environment.mock.getAllOperations().length).toBe(1); + await act(() => { + environment.mock.resolveMostRecentOperation({ + data: { + node: { + __typename: 'User', + id: '1', + name: 'Alice', + friends: { + edges: [ + { + cursor: 'cursor:2', + node: { + __typename: 'User', + id: 'node:2', + name: 'node2', + username: 'username:node:2', + }, + }, + { + cursor: 'cursor:3', + node: { + __typename: 'User', + id: 'node:3', + name: 'node3', + username: 'username:node:3', + }, + }, + ], + pageInfo: { + startCursor: 'cursor:3', + endCursor: 'cursor:3', + hasNextPage: true, + hasPreviousPage: true, + }, + }, + }, + }, + }); + }); + expect(app.toJSON()).toEqual('node1/2'); + // Buffer is full, so nothing is in flight. + expect(environment.mock.getAllOperations().length).toBe(0); + + // Consume one buffered item; this kicks off a top-up prefetch. + await act(() => { + loadMore(1); + }); + expect(app.toJSON()).toEqual('node1,node2/1'); + expect(environment.mock.getAllOperations().length).toBe(1); + + // The top-up prefetch fails, pausing automatic prefetching. One buffered edge + // (node3) still remains. + await act(() => { + environment.mock.rejectMostRecentOperation(new Error('uh oh')); + }); + expect(environment.mock.getAllOperations().length).toBe(0); + + // A later `loadNext` that is fulfilled entirely from the buffer (no network + // request) should still re-enable automatic prefetching. + await act(() => { + loadMore(1); + }); + expect(app.toJSON()).toEqual('node1,node2,node3/0'); + // Prefetching resumed: a fresh request is issued to refill the buffer. + expect(environment.mock.getAllOperations().length).toBe(1); +}); + +it('should treat a non-positive `maxFetchSize` as no cap (never requests `first: 0`)', async () => { + const fragmentKey = environment.lookup(query.fragment).data?.node; + // render the initial page + let app; + await act(() => { + app = create( + + + {/* $FlowFixMe[incompatible-type] */} + + + , + ); + }); + if (app == null) { + throw new Error('should not be null'); + } + expect(app.toJSON()).toEqual('node1/0'); + + // `maxFetchSize` of 0 is invalid and is ignored rather than clamping the + // request to `first: 0` (which would make no progress and loop forever). The + // hook still prefetches to fill the buffer (bufferSize = 2). + expect(environment.mock.getAllOperations().length).toBe(1); + expect(environment.mock.getAllOperations()[0].fragment.variables.first).toBe( + 2, + ); +}); diff --git a/packages/react-relay/relay-hooks/useLoadMoreFunction_EXPERIMENTAL.js b/packages/react-relay/relay-hooks/useLoadMoreFunction_EXPERIMENTAL.js index 43439265c8769..eb09ea794c08e 100644 --- a/packages/react-relay/relay-hooks/useLoadMoreFunction_EXPERIMENTAL.js +++ b/packages/react-relay/relay-hooks/useLoadMoreFunction_EXPERIMENTAL.js @@ -250,7 +250,7 @@ hook useLoadMoreFunction_EXPERIMENTAL( }, error: error => { fetchStatusRef.current = {kind: 'none'}; - observer.complete && observer.complete(); + observer.error && observer.error(error); onComplete && onComplete(error); }, }); diff --git a/packages/react-relay/relay-hooks/usePrefetchableForwardPaginationFragment.js b/packages/react-relay/relay-hooks/usePrefetchableForwardPaginationFragment.js index 011ad927ca4b5..c87533f18f598 100644 --- a/packages/react-relay/relay-hooks/usePrefetchableForwardPaginationFragment.js +++ b/packages/react-relay/relay-hooks/usePrefetchableForwardPaginationFragment.js @@ -106,6 +106,7 @@ hook usePrefetchableForwardPaginationFragment< }, minimalFetchSize: number = 1, disablePrefetching?: boolean = false, + maxFetchSize?: ?number, ): ReturnType { const fragmentNode = getFragment(fragmentInput); useStaticFragmentNodeWarning( @@ -175,11 +176,24 @@ hook usePrefetchableForwardPaginationFragment< // to synchronously get the loading state to decide whether to load more const isLoadingMoreRef = useRef(false); + // When a pagination request fails, automatic prefetching is paused so we don't + // keep retrying against a failing server in a tight loop (which can effectively + // DDOS the backend). Prefetching resumes when the product explicitly calls + // `loadNext`, or after a `refetch`/reset. + const hasPrefetchErroredRef = useRef(false); + const observer = useMemo(() => { function setIsLoadingFalse() { isLoadingMoreRef.current = false; setIsLoadingMore(false); } + function handleError() { + // Stop the automatic prefetch loop until the product explicitly asks for + // more items again, otherwise the prefetch `useEffect` would immediately + // re-issue the request and hammer the server. + hasPrefetchErroredRef.current = true; + setIsLoadingFalse(); + } return { start: () => { isLoadingMoreRef.current = true; @@ -188,13 +202,14 @@ hook usePrefetchableForwardPaginationFragment< reallySetIsLoadingMore(true); }, complete: setIsLoadingFalse, - error: setIsLoadingFalse, + error: handleError, unsubscribe: RelayFeatureFlags.ENABLE_USE_PAGINATION_IS_LOADING_FIX ? setIsLoadingFalse : undefined, }; }, [setIsLoadingMore]); const handleReset = useCallback(() => { + hasPrefetchErroredRef.current = false; if (!isRefetching) { // Do not reset items count during refetching const schedule = environment.getScheduler()?.schedule; @@ -235,11 +250,24 @@ hook usePrefetchableForwardPaginationFragment< prefetchingLoadMoreOptions?.UNSTABLE_extraVariables; const prefetchingOnComplete = prefetchingLoadMoreOptions?.onComplete; + // Normalize the cap to a usable upper bound. A non-positive `maxFetchSize` is + // treated as "no cap" rather than clamping requests to `first: 0`, which would + // make no progress and re-introduce the infinite prefetch loop this cap exists + // to prevent. + const effectiveMaxFetchSize = + maxFetchSize != null && maxFetchSize > 0 ? maxFetchSize : Infinity; + const showMore = useCallback( (numToAdd: number, options?: LoadMoreOptions) => { // Matches the behavior of `usePaginationFragment`. If there is a `loadMore` ongoing, // the hook handles making the `loadMore` a no-op. if (!isLoadingMoreRef.current || availableSizeRef.current >= 0) { + // An explicit `loadNext` is a deliberate user action, so clear any + // previous prefetch error and re-enable automatic prefetching — even + // when this call is served entirely from the buffer and issues no + // network request. + hasPrefetchErroredRef.current = false; + // Preemtively update `availableSizeRef`, so if two `loadMore` is called in the same tick, // a second `loadMore` can be no-op availableSizeRef.current -= numToAdd; @@ -252,9 +280,14 @@ hook usePrefetchableForwardPaginationFragment< // the requirement and cache, capped at the current amount defined by product if (!isLoadingMoreRef.current && availableSizeRef.current < 0) { loadMore( - Math.max( - minimalFetchSize, - Math.min(numToAdd, bufferSize - availableSizeRef.current), + // Cap the request at `maxFetchSize` so we never generate a query + // large enough to overwhelm the backend. + Math.min( + Math.max( + minimalFetchSize, + Math.min(numToAdd, bufferSize - availableSizeRef.current), + ), + effectiveMaxFetchSize, ), // Keep options For backward compatibility options ?? { @@ -296,6 +329,7 @@ hook usePrefetchableForwardPaginationFragment< bufferSize, loadMore, minimalFetchSize, + effectiveMaxFetchSize, edgeKeys, fragmentData, prefetchingUNSTABLE_extraVariables, @@ -322,15 +356,25 @@ hook usePrefetchableForwardPaginationFragment< !isLoadingMore && !isRefetching && !disablePrefetching && + // If a previous prefetch request errored, stop automatically retrying so + // we don't thrash the server. Prefetching resumes after an explicit + // `loadNext` or a `refetch`. + !hasPrefetchErroredRef.current && hasNext && (sourceSize - numInUse < bufferSize || numInUse > sourceSize) ) { const onComplete = prefetchingOnComplete; loadMore( - Math.max( - bufferSize - Math.max(sourceSize - numInUse, 0), - numInUse - sourceSize, - minimalFetchSize, + // Cap the request at `maxFetchSize` so we never generate a query large + // enough to overwhelm the backend (e.g. close to the connection's + // entire `totalCount`). + Math.min( + Math.max( + bufferSize - Math.max(sourceSize - numInUse, 0), + numInUse - sourceSize, + minimalFetchSize, + ), + effectiveMaxFetchSize, ), { onComplete, @@ -373,6 +417,8 @@ hook usePrefetchableForwardPaginationFragment< edgeKeys, isLoadingMore, minimalFetchSize, + effectiveMaxFetchSize, + disablePrefetching, environment, edgesFragment, ]); @@ -390,6 +436,8 @@ hook usePrefetchableForwardPaginationFragment< const refetchPagination = useCallback( (variables: TVariables, options?: Options) => { disposeFetchNext(); + // A refetch is a fresh start, so re-enable automatic prefetching. + hasPrefetchErroredRef.current = false; setIsRefetching(true); return refetch(variables, { ...options, diff --git a/packages/react-relay/relay-hooks/usePrefetchableForwardPaginationFragment_EXPERIMENTAL.js b/packages/react-relay/relay-hooks/usePrefetchableForwardPaginationFragment_EXPERIMENTAL.js index 3b4643960fab4..3e09f861276e6 100644 --- a/packages/react-relay/relay-hooks/usePrefetchableForwardPaginationFragment_EXPERIMENTAL.js +++ b/packages/react-relay/relay-hooks/usePrefetchableForwardPaginationFragment_EXPERIMENTAL.js @@ -105,6 +105,7 @@ hook usePrefetchableForwardPaginationFragment_EXPERIMENTAL< onComplete?: (Error | null) => void, }, minimalFetchSize: number = 1, + maxFetchSize?: ?number, ): ReturnType { const fragmentNode = getFragment(fragmentInput); useStaticFragmentNodeWarning( @@ -175,11 +176,24 @@ hook usePrefetchableForwardPaginationFragment_EXPERIMENTAL< // to synchronously get the loading state to decide whether to load more const isLoadingMoreRef = useRef(false); + // When a pagination request fails, automatic prefetching is paused so we don't + // keep retrying against a failing server in a tight loop (which can effectively + // DDOS the backend). Prefetching resumes when the product explicitly calls + // `loadNext`, or after a `refetch`/reset. + const hasPrefetchErroredRef = useRef(false); + const observer = useMemo(() => { function setIsLoadingFalse() { isLoadingMoreRef.current = false; setIsLoadingMore(false); } + function handleError() { + // Stop the automatic prefetch loop until the product explicitly asks for + // more items again, otherwise the prefetch `useEffect` would immediately + // re-issue the request and hammer the server. + hasPrefetchErroredRef.current = true; + setIsLoadingFalse(); + } return { start: () => { isLoadingMoreRef.current = true; @@ -188,13 +202,14 @@ hook usePrefetchableForwardPaginationFragment_EXPERIMENTAL< reallySetIsLoadingMore(true); }, complete: setIsLoadingFalse, - error: setIsLoadingFalse, + error: handleError, unsubscribe: RelayFeatureFlags.ENABLE_USE_PAGINATION_IS_LOADING_FIX ? setIsLoadingFalse : undefined, }; }, [setIsLoadingMore]); const handleReset = useCallback(() => { + hasPrefetchErroredRef.current = false; if (!isRefetching) { // Do not reset items count during refetching const schedule = environment.getScheduler()?.schedule; @@ -235,11 +250,24 @@ hook usePrefetchableForwardPaginationFragment_EXPERIMENTAL< prefetchingLoadMoreOptions?.UNSTABLE_extraVariables; const prefetchingOnComplete = prefetchingLoadMoreOptions?.onComplete; + // Normalize the cap to a usable upper bound. A non-positive `maxFetchSize` is + // treated as "no cap" rather than clamping requests to `first: 0`, which would + // make no progress and re-introduce the infinite prefetch loop this cap exists + // to prevent. + const effectiveMaxFetchSize = + maxFetchSize != null && maxFetchSize > 0 ? maxFetchSize : Infinity; + const showMore = useCallback( (numToAdd: number, options?: LoadMoreOptions) => { // Matches the behavior of `usePaginationFragment`. If there is a `loadMore` ongoing, // the hook handles making the `loadMore` a no-op. if (!isLoadingMoreRef.current || availableSizeRef.current >= 0) { + // An explicit `loadNext` is a deliberate user action, so clear any + // previous prefetch error and re-enable automatic prefetching — even + // when this call is served entirely from the buffer and issues no + // network request. + hasPrefetchErroredRef.current = false; + // Preemtively update `availableSizeRef`, so if two `loadMore` is called in the same tick, // a second `loadMore` can be no-op availableSizeRef.current -= numToAdd; @@ -252,9 +280,14 @@ hook usePrefetchableForwardPaginationFragment_EXPERIMENTAL< // the requirement and cache, capped at the current amount defined by product if (!isLoadingMoreRef.current && availableSizeRef.current < 0) { loadMore( - Math.max( - minimalFetchSize, - Math.min(numToAdd, bufferSize - availableSizeRef.current), + // Cap the request at `maxFetchSize` so we never generate a query + // large enough to overwhelm the backend. + Math.min( + Math.max( + minimalFetchSize, + Math.min(numToAdd, bufferSize - availableSizeRef.current), + ), + effectiveMaxFetchSize, ), // Keep options For backward compatibility options ?? { @@ -296,6 +329,7 @@ hook usePrefetchableForwardPaginationFragment_EXPERIMENTAL< bufferSize, loadMore, minimalFetchSize, + effectiveMaxFetchSize, edgeKeys, fragmentData, prefetchingUNSTABLE_extraVariables, @@ -321,15 +355,25 @@ hook usePrefetchableForwardPaginationFragment_EXPERIMENTAL< // `loadMore` hook has been updated with the latest cursor !isLoadingMore && !isRefetching && + // If a previous prefetch request errored, stop automatically retrying so + // we don't thrash the server. Prefetching resumes after an explicit + // `loadNext` or a `refetch`. + !hasPrefetchErroredRef.current && hasNext && (sourceSize - numInUse < bufferSize || numInUse > sourceSize) ) { const onComplete = prefetchingOnComplete; loadMore( - Math.max( - bufferSize - Math.max(sourceSize - numInUse, 0), - numInUse - sourceSize, - minimalFetchSize, + // Cap the request at `maxFetchSize` so we never generate a query large + // enough to overwhelm the backend (e.g. close to the connection's + // entire `totalCount`). + Math.min( + Math.max( + bufferSize - Math.max(sourceSize - numInUse, 0), + numInUse - sourceSize, + minimalFetchSize, + ), + effectiveMaxFetchSize, ), { onComplete, @@ -372,6 +416,7 @@ hook usePrefetchableForwardPaginationFragment_EXPERIMENTAL< edgeKeys, isLoadingMore, minimalFetchSize, + effectiveMaxFetchSize, environment, edgesFragment, ]); @@ -389,6 +434,8 @@ hook usePrefetchableForwardPaginationFragment_EXPERIMENTAL< const refetchPagination = useCallback( (variables: TVariables, options?: Options) => { disposeFetchNext(); + // A refetch is a fresh start, so re-enable automatic prefetching. + hasPrefetchErroredRef.current = false; setIsRefetching(true); return refetch(variables, { ...options, diff --git a/website/docs/api-reference/hooks/use-prefetchable-forward-pagination-fragment.mdx b/website/docs/api-reference/hooks/use-prefetchable-forward-pagination-fragment.mdx index f92c89d409053..373af97dba14d 100644 --- a/website/docs/api-reference/hooks/use-prefetchable-forward-pagination-fragment.mdx +++ b/website/docs/api-reference/hooks/use-prefetchable-forward-pagination-fragment.mdx @@ -86,7 +86,9 @@ module.exports = FriendsList; * `prefetchingLoadMoreOptions`: *_[Optional]_* fetching arguments to provide to the automatic prefetch. * `options`: *_[Optional]_* options object * `onComplete`: Function that will be called whenever the request has completed, including any incremental data payloads. If an error occurs during the request, `onComplete` will be called with an `Error` object as the first parameter. -* `minimumFetchSize`: Optional argument to define the minimum number of items to fetch in any requests. +* `minimalFetchSize`: Optional argument to define the minimum number of items to fetch in any requests. +* `disablePrefetching`: *_[Optional]_* boolean argument to disable the automatic prefetching that fills the buffer. When `true`, items are only fetched in response to explicit `loadNext` calls. Defaults to `false`. +* `maxFetchSize`: *_[Optional]_* argument to define the maximum number of items to fetch in any single request. Must be a positive integer. The number of items the hook computes for a request (either to fill the buffer or to fulfill a `loadNext`) is clamped to this value. This prevents the hook from generating large queries (for example, requesting close to the entire `totalCount` of a connection at once) that could overwhelm the backend. If unset (or not a positive number), no upper bound is applied. ### Return Value @@ -126,6 +128,7 @@ Object containing the following properties: ### Behavior * The component automatically fetches for more items to fill the buffer in `useEffect`. +* If a prefetch request errors, automatic prefetching is paused so the hook does not keep retrying against a failing server in a tight loop. Automatic prefetching resumes after the next explicit `loadNext` call or after a `refetch`. * The component is automatically subscribed to updates to the fragment data: if the data for this particular `User` is updated anywhere in the app (e.g. via fetching new data, or mutating existing data), the component will automatically re-render with the latest updated data. * The component will suspend if any data for that specific fragment is missing, and the data is currently being fetched by a parent query. * For more details on Suspense, see our [Loading States with Suspense](../../guided-tour/rendering/loading-states/) guide.