From 410f85866d9f13665382d00fc2b2d5c81997fc9c Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 28 Sep 2026 11:02:28 -0400 Subject: [PATCH 1/7] fix: retry read-only API requests the site rate-limits Some hosts, such as WP Cloud sites, throttle the burst of REST requests sent while the editor loads with a 429. core-data caches a failed taxonomy entity config for the session, so a throttled load leaves the Categories List and Terms List blocks unable to fetch terms until the editor reloads. The @wordpress bump in #691 exposed this: Gutenberg #78568 moved that load later in the burst, where it is throttled. Only readable 429 responses are retried, so the dev server, whose throttled CORS preflights surface as network errors, is not covered. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011A5docu5NJzK5jRH8X5AnS --- src/utils/api-fetch.js | 147 ++++++++++++++++++++++++- src/utils/api-fetch.test.js | 212 ++++++++++++++++++++++++++++++++++++ 2 files changed, 358 insertions(+), 1 deletion(-) diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index ce6470856..2ac654562 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -2,16 +2,26 @@ import apiFetch from '@wordpress/api-fetch'; import { getQueryArg } from '@wordpress/url'; import { __ } from '@wordpress/i18n'; import { getGBKit, POST_FALLBACKS } from './bridge'; -import { info, error as logError } from './logger'; +import { info, warn, error as logError } from './logger'; import { ensureTrailingSlash, stripTrailingSlash } from './url'; /** * @typedef {import('@wordpress/api-fetch').APIFetchMiddleware} APIFetchMiddleware + * @typedef {import('@wordpress/api-fetch').FetchHandler} FetchHandler */ /** Matches `/wp/v2/media` but not sub-paths like `/wp/v2/media/123`. */ const MEDIA_UPLOAD_PATH = /^\/wp\/v2\/media(\?|$)/; +/** Methods safe to repeat because they do not change server state. */ +const RETRYABLE_METHODS = [ 'GET', 'HEAD', 'OPTIONS' ]; + +/** Base delay before each retry; jitter of up to the same amount is added. */ +const RETRY_DELAYS_MS = [ 500, 2000 ]; + +/** Upper bound on a server-requested `Retry-After` delay. */ +const MAX_RETRY_AFTER_MS = 10_000; + /** * Initializes the API fetch configuration and middleware. * @@ -38,6 +48,9 @@ export function configureApiFetch() { apiFetch.use( apiFetch.createPreloadingMiddleware( preloadData ?? defaultPreloadData ) ); + apiFetch.setFetchHandler( + withRateLimitRetry( apiFetch.defaultFetchHandler ) + ); } /** @@ -545,6 +558,138 @@ function isRestIndexPath( path ) { return pathname === '' || pathname === '/'; } +/** + * Wraps a fetch handler to retry read-only requests the site rate-limits. + * + * Some hosts throttle the burst of requests sent while the editor loads with + * a 429. core-data caches some failed resolutions, such as the taxonomy + * entity config, for the rest of the session, so one throttled request can + * break a block, like Categories List, until the editor reloads. + * + * The handler requests the raw response to read its status, then parses it + * as api-fetch would. Wrapping the fetch handler rather than adding a + * middleware retries each network request once, including the pages + * `fetchAllMiddleware` requests. + * + * Exported for testing only. + * + * @param {FetchHandler} fetchHandler The handler performing the request. + * @return {FetchHandler} The handler with retries. + */ +export function withRateLimitRetry( fetchHandler ) { + return async ( options ) => { + const method = ( options.method ?? 'GET' ).toUpperCase(); + if ( ! RETRYABLE_METHODS.includes( method ) ) { + return fetchHandler( options ); + } + + for ( let attempt = 0; ; attempt++ ) { + let response; + let isOk = true; + try { + response = await fetchHandler( { ...options, parse: false } ); + } catch ( err ) { + // Network, offline, and abort errors have no response. + if ( typeof err?.status !== 'number' ) { + throw err; + } + response = err; + isOk = false; + } + + if ( + response.status === 429 && + attempt < RETRY_DELAYS_MS.length && + ! options.signal?.aborted + ) { + warn( + `Retrying ${ method } ${ + options.url ?? options.path + } after a 429 response` + ); + await wait( getRetryDelay( response, attempt ) ); + continue; + } + + if ( options.parse === false ) { + if ( ! isOk ) { + throw response; + } + return response; + } + return parseResponse( response, isOk ); + } + }; +} + +/** + * Returns how long to wait before retrying a rate-limited request. + * + * @param {Response} response The 429 response. + * @param {number} attempt Zero-based index of the retry about to happen. + * @return {number} Delay in milliseconds. + */ +function getRetryDelay( response, attempt ) { + const retryAfter = response.headers?.get?.( 'retry-after' ); + if ( retryAfter ) { + const seconds = Number( retryAfter ); + const delay = Number.isNaN( seconds ) + ? Date.parse( retryAfter ) - Date.now() + : seconds * 1000; + if ( ! Number.isNaN( delay ) ) { + return Math.min( Math.max( delay, 0 ), MAX_RETRY_AFTER_MS ); + } + } + + // Jitter spreads out requests that were throttled in the same burst. + const delay = RETRY_DELAYS_MS[ attempt ]; + return delay + Math.random() * delay; +} + +/** + * Resolves after the given delay. + * + * @param {number} ms Delay in milliseconds. + * @return {Promise} Resolves once the delay has elapsed. + */ +function wait( ms ) { + return new Promise( ( resolve ) => setTimeout( resolve, ms ) ); +} + +/** + * Parses a response the way api-fetch's default handler does, which does not + * expose its parsing. + * + * @param {Response} response The response. + * @param {boolean} isOk Whether the handler accepted the response. + * @return {Promise<*>} The parsed body; rejects with it for an error response. + */ +async function parseResponse( response, isOk ) { + if ( isOk && response.status === 204 ) { + return null; + } + + let body; + try { + if ( typeof response.text !== 'function' ) { + body = await response.json(); + } else { + const text = await response.text(); + body = isOk && text === '' ? null : JSON.parse( text ); + } + } catch { + throw { + code: 'invalid_json', + message: __( 'The response is not a valid JSON response.' ), + }; + } + + if ( ! isOk ) { + throw body; + } + return body; +} + const defaultPreloadData = { '/wp/v2/types?context=view': { body: { diff --git a/src/utils/api-fetch.test.js b/src/utils/api-fetch.test.js index 5de83e53c..773ffbb24 100644 --- a/src/utils/api-fetch.test.js +++ b/src/utils/api-fetch.test.js @@ -337,6 +337,218 @@ describe( 'api-fetch credentials handling', () => { ); } ); + describe( 'withRateLimitRetry', () => { + const rateLimited = ( headers = {} ) => + Promise.resolve( + new Response( 'Too Many Requests', { + status: 429, + headers: { 'Content-Type': 'text/html', ...headers }, + } ) + ); + const okResponse = () => + Promise.resolve( new Response( '{"ok":true}', { status: 200 } ) ); + + beforeEach( () => { + bridge.getGBKit.mockReturnValue( { + siteApiRoot: 'https://example.com/wp-json/', + siteApiNamespace: [], + namespaceExcludedPaths: [], + } ); + vi.useFakeTimers(); + vi.spyOn( Math, 'random' ).mockReturnValue( 0 ); + } ); + + afterEach( () => { + vi.useRealTimers(); + vi.restoreAllMocks(); + } ); + + it.each( [ 'GET', 'HEAD', 'OPTIONS' ] )( + 'retries a rate-limited %s request', + async ( method ) => { + global.fetch = vi + .fn() + .mockImplementationOnce( () => rateLimited() ) + .mockImplementationOnce( okResponse ); + + const request = apiFetch( { + path: '/wp/v2/taxonomies', + method, + parse: false, + } ); + await vi.runAllTimersAsync(); + + expect( ( await request ).status ).toBe( 200 ); + expect( global.fetch ).toHaveBeenCalledTimes( 2 ); + } + ); + + it( 'resolves with the parsed body of the retried request', async () => { + global.fetch = vi + .fn() + .mockImplementationOnce( () => rateLimited() ) + .mockImplementationOnce( okResponse ); + + const request = apiFetch( { path: '/wp/v2/taxonomies' } ); + await vi.runAllTimersAsync(); + + expect( await request ).toEqual( { ok: true } ); + } ); + + it( 'waits longer before each retry', async () => { + global.fetch = vi + .fn() + .mockImplementationOnce( () => rateLimited() ) + .mockImplementationOnce( () => rateLimited() ) + .mockImplementationOnce( okResponse ); + + const request = apiFetch( { path: '/wp/v2/taxonomies' } ); + + await vi.advanceTimersByTimeAsync( 499 ); + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + await vi.advanceTimersByTimeAsync( 1 ); + expect( global.fetch ).toHaveBeenCalledTimes( 2 ); + await vi.advanceTimersByTimeAsync( 1999 ); + expect( global.fetch ).toHaveBeenCalledTimes( 2 ); + await vi.advanceTimersByTimeAsync( 1 ); + + expect( await request ).toEqual( { ok: true } ); + expect( global.fetch ).toHaveBeenCalledTimes( 3 ); + } ); + + it( 'waits as long as the Retry-After header asks', async () => { + global.fetch = vi + .fn() + .mockImplementationOnce( () => + rateLimited( { 'Retry-After': '3' } ) + ) + .mockImplementationOnce( okResponse ); + + const request = apiFetch( { path: '/wp/v2/taxonomies' } ); + + await vi.advanceTimersByTimeAsync( 2999 ); + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + await vi.advanceTimersByTimeAsync( 1 ); + + expect( await request ).toEqual( { ok: true } ); + } ); + + it( 'caps a long Retry-After delay', async () => { + global.fetch = vi + .fn() + .mockImplementationOnce( () => + rateLimited( { 'Retry-After': '120' } ) + ) + .mockImplementationOnce( okResponse ); + + const request = apiFetch( { path: '/wp/v2/taxonomies' } ); + + await vi.advanceTimersByTimeAsync( 10_000 ); + + expect( await request ).toEqual( { ok: true } ); + } ); + + it( 'rejects as api-fetch would once retries run out', async () => { + global.fetch = vi.fn( () => rateLimited() ); + + const request = apiFetch( { path: '/wp/v2/taxonomies' } ); + const assertion = expect( request ).rejects.toMatchObject( { + code: 'invalid_json', + } ); + await vi.runAllTimersAsync(); + + await assertion; + expect( global.fetch ).toHaveBeenCalledTimes( 3 ); + } ); + + it( 'rejects with the response once retries run out without parsing', async () => { + global.fetch = vi.fn( () => rateLimited() ); + + const request = apiFetch( { + path: '/wp/v2/taxonomies', + parse: false, + } ); + const assertion = expect( request ).rejects.toMatchObject( { + status: 429, + } ); + await vi.runAllTimersAsync(); + + await assertion; + } ); + + it( 'does not retry a request that changes server state', async () => { + global.fetch = vi.fn( () => rateLimited() ); + + await expect( + apiFetch( { path: '/wp/v2/posts', method: 'POST', data: {} } ) + ).rejects.toMatchObject( { code: 'invalid_json' } ); + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'does not retry other error responses', async () => { + global.fetch = vi.fn( () => + Promise.resolve( + new Response( '{"code":"rest_forbidden"}', { + status: 403, + } ) + ) + ); + + await expect( + apiFetch( { path: '/wp/v2/taxonomies' } ) + ).rejects.toMatchObject( { code: 'rest_forbidden' } ); + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'does not retry network errors', async () => { + global.fetch = vi.fn( () => + Promise.reject( new TypeError( 'Failed to fetch' ) ) + ); + + await expect( + apiFetch( { path: '/wp/v2/taxonomies' } ) + ).rejects.toMatchObject( { code: 'fetch_error' } ); + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'does not retry an aborted request', async () => { + const controller = new AbortController(); + global.fetch = vi.fn( () => { + controller.abort(); + return rateLimited(); + } ); + + await expect( + apiFetch( { + path: '/wp/v2/taxonomies', + signal: controller.signal, + } ) + ).rejects.toMatchObject( { code: 'invalid_json' } ); + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + } ); + + it.each( [ + [ 'a 204 response', new Response( null, { status: 204 } ), null ], + [ 'an empty body', new Response( '', { status: 200 } ), null ], + ] )( 'resolves with null for %s', async ( _label, response, body ) => { + global.fetch = vi.fn( () => Promise.resolve( response ) ); + + expect( await apiFetch( { path: '/wp/v2/taxonomies' } ) ).toBe( + body + ); + } ); + + it( 'rejects invalid JSON in a successful response', async () => { + global.fetch = vi.fn( () => + Promise.resolve( new Response( '', { status: 200 } ) ) + ); + + await expect( + apiFetch( { path: '/wp/v2/taxonomies' } ) + ).rejects.toMatchObject( { code: 'invalid_json' } ); + } ); + } ); + it( 'should preserve other headers when adding Authorization', async () => { bridge.getGBKit.mockReturnValue( { siteApiRoot: 'https://example.com/wp-json/', From d76a2ae4e5cbd653bf69b089987b1cd2995d8ec3 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 28 Sep 2026 14:35:23 -0400 Subject: [PATCH 2/7] fix: fail at once when Retry-After exceeds the retry cap Clamping a long Retry-After to the cap sent both retries inside the window the server had closed, so they could only fail. The request now rejects immediately instead of after ~20s and two wasted requests. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011Y5SN26ekqCm2EmaR4CddU --- src/utils/api-fetch.js | 22 +++++++++++++--------- src/utils/api-fetch.test.js | 23 ++++++++++++++++++++--- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index 2ac654562..ff61d18ef 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -19,7 +19,7 @@ const RETRYABLE_METHODS = [ 'GET', 'HEAD', 'OPTIONS' ]; /** Base delay before each retry; jitter of up to the same amount is added. */ const RETRY_DELAYS_MS = [ 500, 2000 ]; -/** Upper bound on a server-requested `Retry-After` delay. */ +/** Longest `Retry-After` delay worth waiting for; longer ones fail at once. */ const MAX_RETRY_AFTER_MS = 10_000; /** @@ -602,13 +602,17 @@ export function withRateLimitRetry( fetchHandler ) { attempt < RETRY_DELAYS_MS.length && ! options.signal?.aborted ) { - warn( - `Retrying ${ method } ${ - options.url ?? options.path - } after a 429 response` - ); - await wait( getRetryDelay( response, attempt ) ); - continue; + const delay = getRetryDelay( response, attempt ); + // A retry sent before a longer `Retry-After` elapses would fail too. + if ( delay <= MAX_RETRY_AFTER_MS ) { + warn( + `Retrying ${ method } ${ + options.url ?? options.path + } after a 429 response` + ); + await wait( delay ); + continue; + } } if ( options.parse === false ) { @@ -637,7 +641,7 @@ function getRetryDelay( response, attempt ) { ? Date.parse( retryAfter ) - Date.now() : seconds * 1000; if ( ! Number.isNaN( delay ) ) { - return Math.min( Math.max( delay, 0 ), MAX_RETRY_AFTER_MS ); + return Math.max( delay, 0 ); } } diff --git a/src/utils/api-fetch.test.js b/src/utils/api-fetch.test.js index 773ffbb24..cc94ee338 100644 --- a/src/utils/api-fetch.test.js +++ b/src/utils/api-fetch.test.js @@ -433,21 +433,38 @@ describe( 'api-fetch credentials handling', () => { expect( await request ).toEqual( { ok: true } ); } ); - it( 'caps a long Retry-After delay', async () => { + it( 'waits for a Retry-After delay of up to 10 seconds', async () => { global.fetch = vi .fn() .mockImplementationOnce( () => - rateLimited( { 'Retry-After': '120' } ) + rateLimited( { 'Retry-After': '10' } ) ) .mockImplementationOnce( okResponse ); const request = apiFetch( { path: '/wp/v2/taxonomies' } ); - await vi.advanceTimersByTimeAsync( 10_000 ); + await vi.advanceTimersByTimeAsync( 9_999 ); + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + await vi.advanceTimersByTimeAsync( 1 ); expect( await request ).toEqual( { ok: true } ); } ); + it( 'does not retry when Retry-After asks for a longer delay', async () => { + global.fetch = vi.fn( () => + rateLimited( { 'Retry-After': '11' } ) + ); + + const request = apiFetch( { path: '/wp/v2/taxonomies' } ); + const assertion = expect( request ).rejects.toMatchObject( { + code: 'invalid_json', + } ); + await vi.runAllTimersAsync(); + + await assertion; + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + } ); + it( 'rejects as api-fetch would once retries run out', async () => { global.fetch = vi.fn( () => rateLimited() ); From 15f6966fd731ba4c8b776366447060d9b5e9aed2 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 28 Sep 2026 14:37:08 -0400 Subject: [PATCH 3/7] test: close gaps in the rate-limit retry tests Tests missed a jitter regression, the HTTP-date Retry-After form, and drift between the wrapper's copied parsing and api-fetch's own. A parity test now compares both handlers, and the no-retry tests fail on call counts rather than timing out. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011Y5SN26ekqCm2EmaR4CddU --- src/utils/api-fetch.test.js | 134 +++++++++++++++++++++++++++++++----- 1 file changed, 118 insertions(+), 16 deletions(-) diff --git a/src/utils/api-fetch.test.js b/src/utils/api-fetch.test.js index cc94ee338..7b6f33e4e 100644 --- a/src/utils/api-fetch.test.js +++ b/src/utils/api-fetch.test.js @@ -8,7 +8,7 @@ import { vi, } from 'vitest'; import apiFetch from '@wordpress/api-fetch'; -import { configureApiFetch } from './api-fetch'; +import { configureApiFetch, withRateLimitRetry } from './api-fetch'; import * as bridge from './bridge'; vi.mock( './bridge', async ( importOriginal ) => { @@ -347,6 +347,14 @@ describe( 'api-fetch credentials handling', () => { ); const okResponse = () => Promise.resolve( new Response( '{"ok":true}', { status: 200 } ) ); + // Responses are compared by status, as their bodies are single-use. + const settle = ( promise ) => + promise.then( + ( value ) => ( { resolved: summarize( value ) } ), + ( err ) => ( { rejected: summarize( err ) } ) + ); + const summarize = ( value ) => + value instanceof Response ? { status: value.status } : value; beforeEach( () => { bridge.getGBKit.mockReturnValue( { @@ -416,6 +424,28 @@ describe( 'api-fetch credentials handling', () => { expect( global.fetch ).toHaveBeenCalledTimes( 3 ); } ); + it( 'adds jitter in proportion to each base delay', async () => { + vi.spyOn( Math, 'random' ).mockReturnValue( 0.5 ); + global.fetch = vi + .fn() + .mockImplementationOnce( () => rateLimited() ) + .mockImplementationOnce( () => rateLimited() ) + .mockImplementationOnce( okResponse ); + + const request = apiFetch( { path: '/wp/v2/taxonomies' } ); + + await vi.advanceTimersByTimeAsync( 749 ); + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + await vi.advanceTimersByTimeAsync( 1 ); + expect( global.fetch ).toHaveBeenCalledTimes( 2 ); + await vi.advanceTimersByTimeAsync( 2999 ); + expect( global.fetch ).toHaveBeenCalledTimes( 2 ); + await vi.advanceTimersByTimeAsync( 1 ); + + expect( await request ).toEqual( { ok: true } ); + expect( global.fetch ).toHaveBeenCalledTimes( 3 ); + } ); + it( 'waits as long as the Retry-After header asks', async () => { global.fetch = vi .fn() @@ -433,6 +463,26 @@ describe( 'api-fetch credentials handling', () => { expect( await request ).toEqual( { ok: true } ); } ); + it( 'waits until the date the Retry-After header names', async () => { + vi.setSystemTime( new Date( '2026-01-01T00:00:00Z' ) ); + global.fetch = vi + .fn() + .mockImplementationOnce( () => + rateLimited( { + 'Retry-After': 'Thu, 01 Jan 2026 00:00:03 GMT', + } ) + ) + .mockImplementationOnce( okResponse ); + + const request = apiFetch( { path: '/wp/v2/taxonomies' } ); + + await vi.advanceTimersByTimeAsync( 2999 ); + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + await vi.advanceTimersByTimeAsync( 1 ); + + expect( await request ).toEqual( { ok: true } ); + } ); + it( 'waits for a Retry-After delay of up to 10 seconds', async () => { global.fetch = vi .fn() @@ -496,9 +546,17 @@ describe( 'api-fetch credentials handling', () => { it( 'does not retry a request that changes server state', async () => { global.fetch = vi.fn( () => rateLimited() ); - await expect( - apiFetch( { path: '/wp/v2/posts', method: 'POST', data: {} } ) - ).rejects.toMatchObject( { code: 'invalid_json' } ); + const request = apiFetch( { + path: '/wp/v2/posts', + method: 'POST', + data: {}, + } ); + const assertion = expect( request ).rejects.toMatchObject( { + code: 'invalid_json', + } ); + await vi.runAllTimersAsync(); + + await assertion; expect( global.fetch ).toHaveBeenCalledTimes( 1 ); } ); @@ -511,9 +569,13 @@ describe( 'api-fetch credentials handling', () => { ) ); - await expect( - apiFetch( { path: '/wp/v2/taxonomies' } ) - ).rejects.toMatchObject( { code: 'rest_forbidden' } ); + const request = apiFetch( { path: '/wp/v2/taxonomies' } ); + const assertion = expect( request ).rejects.toMatchObject( { + code: 'rest_forbidden', + } ); + await vi.runAllTimersAsync(); + + await assertion; expect( global.fetch ).toHaveBeenCalledTimes( 1 ); } ); @@ -522,9 +584,13 @@ describe( 'api-fetch credentials handling', () => { Promise.reject( new TypeError( 'Failed to fetch' ) ) ); - await expect( - apiFetch( { path: '/wp/v2/taxonomies' } ) - ).rejects.toMatchObject( { code: 'fetch_error' } ); + const request = apiFetch( { path: '/wp/v2/taxonomies' } ); + const assertion = expect( request ).rejects.toMatchObject( { + code: 'fetch_error', + } ); + await vi.runAllTimersAsync(); + + await assertion; expect( global.fetch ).toHaveBeenCalledTimes( 1 ); } ); @@ -535,12 +601,16 @@ describe( 'api-fetch credentials handling', () => { return rateLimited(); } ); - await expect( - apiFetch( { - path: '/wp/v2/taxonomies', - signal: controller.signal, - } ) - ).rejects.toMatchObject( { code: 'invalid_json' } ); + const request = apiFetch( { + path: '/wp/v2/taxonomies', + signal: controller.signal, + } ); + const assertion = expect( request ).rejects.toMatchObject( { + code: 'invalid_json', + } ); + await vi.runAllTimersAsync(); + + await assertion; expect( global.fetch ).toHaveBeenCalledTimes( 1 ); } ); @@ -564,6 +634,38 @@ describe( 'api-fetch credentials handling', () => { apiFetch( { path: '/wp/v2/taxonomies' } ) ).rejects.toMatchObject( { code: 'invalid_json' } ); } ); + + // The wrapper parses responses itself, so guard against drifting from + // api-fetch's own parsing when the package updates. + describe.each( [ true, false ] )( 'with parse: %s', ( parse ) => { + it.each( [ + [ 'a JSON body', 200, '{"id":1}' ], + [ 'an empty body', 200, '' ], + [ 'invalid JSON', 200, '' ], + [ 'a 204 response', 204, null ], + [ 'a JSON error', 404, '{"code":"rest_no_route"}' ], + [ 'an empty error body', 404, '' ], + [ 'an HTML error', 500, 'Error' ], + ] )( + 'settles %s as api-fetch does', + async ( _label, status, body ) => { + global.fetch = vi.fn( () => + Promise.resolve( new Response( body, { status } ) ) + ); + const options = { + url: 'https://example.com/wp-json/wp/v2/taxonomies', + parse, + }; + const handler = withRateLimitRetry( + apiFetch.defaultFetchHandler + ); + + expect( await settle( handler( options ) ) ).toEqual( + await settle( apiFetch.defaultFetchHandler( options ) ) + ); + } + ); + } ); } ); it( 'should preserve other headers when adding Authorization', async () => { From 15ce3c957d63ddd49e16c10a33506ae3d3ddaed1 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 28 Sep 2026 14:37:24 -0400 Subject: [PATCH 4/7] docs: trim the rate-limit retry JSDoc Keep the why for the final design and drop implementation detail that parseResponse already documents. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011Y5SN26ekqCm2EmaR4CddU --- src/utils/api-fetch.js | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index ff61d18ef..7648f2fc7 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -561,15 +561,9 @@ function isRestIndexPath( path ) { /** * Wraps a fetch handler to retry read-only requests the site rate-limits. * - * Some hosts throttle the burst of requests sent while the editor loads with - * a 429. core-data caches some failed resolutions, such as the taxonomy - * entity config, for the rest of the session, so one throttled request can - * break a block, like Categories List, until the editor reloads. - * - * The handler requests the raw response to read its status, then parses it - * as api-fetch would. Wrapping the fetch handler rather than adding a - * middleware retries each network request once, including the pages - * `fetchAllMiddleware` requests. + * core-data caches some failed resolutions for the session, so one throttled + * request in the editor's load burst can break a block until reload. As a + * fetch handler, it also retries each page `fetchAllMiddleware` requests. * * Exported for testing only. * From 84baa8aebcb8bdd71eef53e4cfce29435ab02b17 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 2 Oct 2026 14:57:13 -0400 Subject: [PATCH 5/7] fix: resolve the new jsdoc any-type lint warning Trunk's @wordpress/eslint-plugin 27 bump (#738) rejects `*` types, which the rate-limit retry's response parser used. Co-Authored-By: Claude Opus 5.5 --- src/utils/api-fetch.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index 7648f2fc7..9f004a937 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -660,7 +660,7 @@ function wait( ms ) { * * @param {Response} response The response. * @param {boolean} isOk Whether the handler accepted the response. - * @return {Promise<*>} The parsed body; rejects with it for an error response. + * @return {Promise} The parsed body; rejects with it for an error response. */ async function parseResponse( response, isOk ) { if ( isOk && response.status === 204 ) { From 263a96b1523c1bbe3cbbb52cad8227d946df1403 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 2 Oct 2026 15:02:12 -0400 Subject: [PATCH 6/7] fix: log 429 retries as info and warn when giving up A retried 429 is expected and self-healing, so it needs no attention. A 429 that exhausts its retries, or whose Retry-After exceeds the cap, can leave a failure cached for the session, and was previously logged nowhere. Co-Authored-By: Claude Opus 5.5 --- src/utils/api-fetch.js | 29 ++++++++++++++--------------- src/utils/api-fetch.test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index 9f004a937..fdeece831 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -591,22 +591,21 @@ export function withRateLimitRetry( fetchHandler ) { isOk = false; } - if ( - response.status === 429 && - attempt < RETRY_DELAYS_MS.length && - ! options.signal?.aborted - ) { - const delay = getRetryDelay( response, attempt ); - // A retry sent before a longer `Retry-After` elapses would fail too. - if ( delay <= MAX_RETRY_AFTER_MS ) { - warn( - `Retrying ${ method } ${ - options.url ?? options.path - } after a 429 response` - ); - await wait( delay ); - continue; + if ( response.status === 429 && ! options.signal?.aborted ) { + const request = `${ method } ${ options.url ?? options.path }`; + if ( attempt < RETRY_DELAYS_MS.length ) { + const delay = getRetryDelay( response, attempt ); + // A retry sent before a longer `Retry-After` elapses would fail too. + if ( delay <= MAX_RETRY_AFTER_MS ) { + info( `Retrying ${ request } after a 429 response` ); + await wait( delay ); + continue; + } } + // core-data may cache this failure for the session. + warn( `Giving up on ${ request } after a 429 response`, { + retries: attempt, + } ); } if ( options.parse === false ) { diff --git a/src/utils/api-fetch.test.js b/src/utils/api-fetch.test.js index 7b6f33e4e..f4f5df683 100644 --- a/src/utils/api-fetch.test.js +++ b/src/utils/api-fetch.test.js @@ -10,6 +10,7 @@ import { import apiFetch from '@wordpress/api-fetch'; import { configureApiFetch, withRateLimitRetry } from './api-fetch'; import * as bridge from './bridge'; +import * as logger from './logger'; vi.mock( './bridge', async ( importOriginal ) => { const actual = await importOriginal(); @@ -18,6 +19,7 @@ vi.mock( './bridge', async ( importOriginal ) => { getGBKit: vi.fn(), }; } ); +vi.mock( './logger' ); describe( 'api-fetch credentials handling', () => { let originalFetch; @@ -403,6 +405,22 @@ describe( 'api-fetch credentials handling', () => { expect( await request ).toEqual( { ok: true } ); } ); + it( 'logs each retry as info rather than a warning', async () => { + global.fetch = vi + .fn() + .mockImplementationOnce( () => rateLimited() ) + .mockImplementationOnce( okResponse ); + + const request = apiFetch( { path: '/wp/v2/taxonomies' } ); + await vi.runAllTimersAsync(); + await request; + + expect( logger.info ).toHaveBeenCalledWith( + expect.stringContaining( 'Retrying GET' ) + ); + expect( logger.warn ).not.toHaveBeenCalled(); + } ); + it( 'waits longer before each retry', async () => { global.fetch = vi .fn() @@ -513,6 +531,10 @@ describe( 'api-fetch credentials handling', () => { await assertion; expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + expect( logger.warn ).toHaveBeenCalledWith( + expect.stringContaining( 'Giving up on GET' ), + { retries: 0 } + ); } ); it( 'rejects as api-fetch would once retries run out', async () => { @@ -526,6 +548,10 @@ describe( 'api-fetch credentials handling', () => { await assertion; expect( global.fetch ).toHaveBeenCalledTimes( 3 ); + expect( logger.warn ).toHaveBeenCalledWith( + expect.stringContaining( 'Giving up on GET' ), + { retries: 2 } + ); } ); it( 'rejects with the response once retries run out without parsing', async () => { @@ -612,6 +638,7 @@ describe( 'api-fetch credentials handling', () => { await assertion; expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + expect( logger.warn ).not.toHaveBeenCalled(); } ); it.each( [ From 275ceddb8be7494e20343c4611f900108b4bc64b Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Sat, 3 Oct 2026 07:56:50 -0400 Subject: [PATCH 7/7] fix: stop waiting to retry once the request is aborted The retry delay ignored the request's signal, so an aborted request stayed pending until the delay elapsed. It now rejects at once with the signal's reason, as fetch does. Co-Authored-By: Claude Opus 5.5 --- src/utils/api-fetch.js | 21 ++++++++++++++++----- src/utils/api-fetch.test.js | 20 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index fdeece831..d136c3a38 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -598,7 +598,7 @@ export function withRateLimitRetry( fetchHandler ) { // A retry sent before a longer `Retry-After` elapses would fail too. if ( delay <= MAX_RETRY_AFTER_MS ) { info( `Retrying ${ request } after a 429 response` ); - await wait( delay ); + await wait( delay, options.signal ); continue; } } @@ -644,13 +644,24 @@ function getRetryDelay( response, attempt ) { } /** - * Resolves after the given delay. + * Resolves after the given delay, or rejects as `fetch` would once aborted. * - * @param {number} ms Delay in milliseconds. + * @param {number} ms Delay in milliseconds. + * @param {AbortSignal} [signal] Signal of the request being retried. * @return {Promise} Resolves once the delay has elapsed. */ -function wait( ms ) { - return new Promise( ( resolve ) => setTimeout( resolve, ms ) ); +function wait( ms, signal ) { + return new Promise( ( resolve, reject ) => { + const onAbort = () => { + clearTimeout( timer ); + reject( signal.reason ); + }; + const timer = setTimeout( () => { + signal?.removeEventListener( 'abort', onAbort ); + resolve(); + }, ms ); + signal?.addEventListener( 'abort', onAbort, { once: true } ); + } ); } /** diff --git a/src/utils/api-fetch.test.js b/src/utils/api-fetch.test.js index f4f5df683..a9b24fc9a 100644 --- a/src/utils/api-fetch.test.js +++ b/src/utils/api-fetch.test.js @@ -641,6 +641,26 @@ describe( 'api-fetch credentials handling', () => { expect( logger.warn ).not.toHaveBeenCalled(); } ); + it( 'rejects at once when aborted while waiting to retry', async () => { + const controller = new AbortController(); + global.fetch = vi.fn( () => rateLimited() ); + + const request = apiFetch( { + path: '/wp/v2/taxonomies', + signal: controller.signal, + } ); + const assertion = expect( request ).rejects.toMatchObject( { + name: 'AbortError', + } ); + await vi.advanceTimersByTimeAsync( 100 ); + controller.abort(); + await vi.advanceTimersByTimeAsync( 0 ); + + expect( vi.getTimerCount() ).toBe( 0 ); + await assertion; + expect( global.fetch ).toHaveBeenCalledTimes( 1 ); + } ); + it.each( [ [ 'a 204 response', new Response( null, { status: 204 } ), null ], [ 'an empty body', new Response( '', { status: 200 } ), null ],