From 5117a7f6d67bbc7ee605b0d5b5e81b57fc0c1cec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 20 Oct 2026 11:12:38 +0200 Subject: [PATCH 1/3] Back off throttled API requests with bounded jittered retries --- .changeset/api-throttle-backoff.md | 5 + packages/cli-kit/src/private/node/api.test.ts | 211 +++++++++++++++++- packages/cli-kit/src/private/node/api.ts | 61 +++-- .../src/public/node/api/graphql.test.ts | 51 +++++ 4 files changed, 302 insertions(+), 26 deletions(-) create mode 100644 .changeset/api-throttle-backoff.md diff --git a/.changeset/api-throttle-backoff.md b/.changeset/api-throttle-backoff.md new file mode 100644 index 00000000000..5276607319b --- /dev/null +++ b/.changeset/api-throttle-backoff.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +Retry throttled API requests with bounded exponential backoff and jitter, respecting Retry-After delays. diff --git a/packages/cli-kit/src/private/node/api.test.ts b/packages/cli-kit/src/private/node/api.test.ts index ad0922fbb60..0494763dba3 100644 --- a/packages/cli-kit/src/private/node/api.test.ts +++ b/packages/cli-kit/src/private/node/api.test.ts @@ -1,7 +1,9 @@ import {retryAwareRequest, isNetworkError, isTransientNetworkError} from './api.js' import {recordRetry} from '../../public/node/analytics.js' import {ClientError} from 'graphql-request' +import {GraphQLError} from 'graphql' import {describe, test, vi, expect, beforeEach, afterEach} from 'vitest' +import {performance} from 'perf_hooks' vi.mock('../../public/node/analytics.js', () => ({ recordRetry: vi.fn(), @@ -10,6 +12,8 @@ vi.mock('../../public/node/analytics.js', () => ({ describe('retryAwareRequest', () => { beforeEach(() => { vi.useFakeTimers() + vi.spyOn(performance, 'now').mockImplementation(() => Date.now()) + vi.spyOn(Math, 'random').mockReturnValue(0.5) }) afterEach(() => { @@ -28,7 +32,7 @@ describe('retryAwareRequest', () => { } as any, ], headers: new Headers({ - 'retry-after': '200', + 'retry-after': '0.2', }), } @@ -89,7 +93,7 @@ describe('retryAwareRequest', () => { expect(mockRequestFn).toHaveBeenCalledTimes(4) expect(mockScheduleDelayFn).toHaveBeenCalledTimes(2) expect(mockScheduleDelayFn).toHaveBeenNthCalledWith(1, expect.anything(), 200) - expect(mockScheduleDelayFn).toHaveBeenNthCalledWith(2, expect.anything(), 500) + expect(mockScheduleDelayFn).toHaveBeenNthCalledWith(2, expect.anything(), 750) }) test('retries THROTTLED GraphQL errors that carry no 429 status or code', async () => { @@ -143,7 +147,7 @@ describe('retryAwareRequest', () => { }) expect(mockRequestFn).toHaveBeenCalledTimes(2) - expect(mockScheduleDelayFn).toHaveBeenCalledWith(expect.anything(), 500) + expect(mockScheduleDelayFn).toHaveBeenCalledWith(expect.anything(), 375) }) test('does not retry errors whose message says Throttled without a rate-limit code', async () => { @@ -186,6 +190,194 @@ describe('retryAwareRequest', () => { expect(mockScheduleDelayFn).not.toHaveBeenCalled() }) + test('backs off with jitter and recovers when the API needs 20 seconds to refill', async () => { + const startedAt = Date.now() + const success = {status: 200, headers: new Headers(), data: {version: 'created'}} + const request = vi.fn(async () => { + if (Date.now() - startedAt < 20_000) throw throttledError() + return success + }) + const scheduleDelay = vi.fn((fn, delay) => setTimeout(fn, delay)) + const result = retryAwareRequest( + {request, url: 'https://example.com/graphql', useNetworkLevelRetry: false}, + undefined, + {scheduleDelay}, + ) + const assertion = expect(result).resolves.toBe(success) + await vi.runAllTimersAsync() + await assertion + + expect(scheduleDelay.mock.calls.map(([, delay]) => delay)).toEqual([750, 1500, 3000, 6000, 6000, 6000]) + expect(request).toHaveBeenCalledTimes(7) + expect(Date.now() - startedAt).toBe(23_250) + }) + + test('spreads concurrent callers across different retry times', async () => { + vi.mocked(Math.random).mockReturnValueOnce(0).mockReturnValueOnce(0.999) + const scheduleDelay = vi.fn((fn, delay) => setTimeout(fn, delay)) + const requests = [0, 1].map(() => + vi.fn().mockRejectedValueOnce(throttledError()).mockResolvedValue({status: 200, headers: new Headers()}), + ) + const result = Promise.all( + requests.map((request) => + retryAwareRequest({request, url: 'https://example.com/graphql', useNetworkLevelRetry: false}, undefined, { + scheduleDelay, + }), + ), + ) + await vi.runAllTimersAsync() + await result + + expect(scheduleDelay.mock.calls.map(([, delay]) => delay)).toEqual([500, 999]) + for (const request of requests) expect(request).toHaveBeenCalledTimes(2) + }) + + test.each([ + {header: '2', delay: 2000}, + {header: '0.25', delay: 250}, + {header: 'Tue, 20 Oct 2026 12:00:10 GMT', delay: 10_000}, + {header: 'Tue, 20 Oct 2026 11:59:59 GMT', delay: 0}, + {header: 'invalid', delay: 750}, + {header: '-1', delay: 750}, + {header: 'Infinity', delay: 750}, + {header: ' ', delay: 750}, + ])('uses $delay milliseconds for Retry-After "$header"', async ({header, delay}) => { + vi.setSystemTime(new Date('2026-10-20T12:00:00Z')) + const startedAt = Date.now() + const success = {status: 200, headers: new Headers()} + const request = vi + .fn() + .mockRejectedValueOnce(new ClientError({status: 429, headers: new Headers({'retry-after': header})}, {query: ''})) + .mockResolvedValue(success) + const result = retryAwareRequest({request, url: 'https://example.com/graphql', useNetworkLevelRetry: false}) + const assertion = expect(result).resolves.toBe(success) + await vi.runAllTimersAsync() + await assertion + + expect(request).toHaveBeenCalledTimes(2) + expect(Date.now() - startedAt).toBe(delay) + }) + + test('stops scheduling retries within 30 seconds and preserves the last error', async () => { + const startedAt = Date.now() + let lastError: ClientError + const request = vi.fn(async () => { + lastError = throttledError() + throw lastError + }) + const result = retryAwareRequest({request, url: 'https://example.com/graphql', useNetworkLevelRetry: false}) + const assertion = expect(result).rejects.toBeInstanceOf(ClientError) + await vi.runAllTimersAsync() + await assertion + + await expect(result).rejects.toBe(lastError!) + expect(request).toHaveBeenCalledTimes(8) + expect(Date.now() - startedAt).toBe(29_250) + }) + + test('does not shorten a server delay that exceeds the remaining retry budget', async () => { + const error = throttledError({'retry-after': '60', 'x-request-id': 'throttled-request'}) + const request = vi.fn().mockRejectedValue(error) + const scheduleDelay = vi.fn() + const finalError = new Error('Rate limit could not recover') + const errorHandler = vi.fn(() => finalError) + + await expect( + retryAwareRequest({request, url: 'https://example.com/graphql', useNetworkLevelRetry: false}, errorHandler, { + scheduleDelay, + }), + ).rejects.toBe(finalError) + + expect(request).toHaveBeenCalledTimes(1) + expect(scheduleDelay).not.toHaveBeenCalled() + expect(errorHandler).toHaveBeenCalledWith(error, 'throttled-request') + }) + + test('does not start a retry when its timer fires after the deadline', async () => { + const error = throttledError() + const request = vi.fn().mockRejectedValue(error) + const scheduleDelay = vi.fn((fn) => setTimeout(fn, 30_000)) + const result = retryAwareRequest( + {request, url: 'https://example.com/graphql', useNetworkLevelRetry: false}, + undefined, + {scheduleDelay}, + ) + const assertion = expect(result).rejects.toBe(error) + await vi.runAllTimersAsync() + await assertion + + expect(request).toHaveBeenCalledTimes(1) + expect(scheduleDelay).toHaveBeenCalledTimes(1) + }) + + test('includes time spent on retried requests in the retry budget', async () => { + const error = throttledError() + const request = vi + .fn() + .mockRejectedValueOnce(error) + .mockImplementation(async () => { + vi.advanceTimersByTime(30_000) + throw error + }) + const result = retryAwareRequest({request, url: 'https://example.com/graphql', useNetworkLevelRetry: false}) + const assertion = expect(result).rejects.toBe(error) + await vi.runAllTimersAsync() + await assertion + + expect(request).toHaveBeenCalledTimes(2) + }) + + test.each([ + { + name: 'partial mutation data', + status: 200, + data: {createdVersion: {id: '123'}}, + errors: [{message: 'Throttled', extensions: {code: 'THROTTLED'}}], + }, + { + name: 'mixed permission and throttle errors', + status: 200, + data: undefined, + errors: [ + {message: 'Throttled', extensions: {code: 'THROTTLED'}}, + {message: 'Access denied', extensions: {code: 'ACCESS_DENIED'}}, + ], + }, + { + name: 'an unrelated error without a code', + status: 200, + data: undefined, + errors: [{message: 'Throttled', extensions: {code: 'THROTTLED'}}, {message: 'Invalid version'}], + }, + { + name: 'an HTTP 429 with permission errors', + status: 429, + data: undefined, + errors: [{message: 'Access denied', extensions: {code: 'ACCESS_DENIED'}}], + }, + ])('does not replay a response with $name', async ({status, data, errors}) => { + const error = new ClientError( + { + status, + data, + errors: errors.map(({message, extensions}) => new GraphQLError(message, {extensions})), + headers: new Headers(), + }, + {query: ''}, + ) + const request = vi.fn().mockRejectedValue(error) + const scheduleDelay = vi.fn() + + await expect( + retryAwareRequest({request, url: 'https://example.com/graphql', useNetworkLevelRetry: false}, undefined, { + scheduleDelay, + }), + ).rejects.toBe(error) + + expect(request).toHaveBeenCalledTimes(1) + expect(scheduleDelay).not.toHaveBeenCalled() + }) + test('fails after too many retries', async () => { // This test gives a false warning from vitest if fake timers are used. It thinks the exception is uncaught. vi.useRealTimers() @@ -484,7 +676,7 @@ describe('retryAwareRequest', () => { }, } as any, ], - headers: new Headers({'retry-after': '100'}), + headers: new Headers({'retry-after': '0.1'}), } const successResponse = { @@ -698,6 +890,17 @@ describe('retryAwareRequest', () => { }) }) +function throttledError(headers: Record = {}): ClientError { + return new ClientError( + { + status: 200, + errors: [new GraphQLError('Throttled', {extensions: {code: 'THROTTLED'}})], + headers: new Headers(headers), + }, + {query: ''}, + ) +} + describe('isTransientNetworkError', () => { test('identifies transient network errors that should be retried', () => { const transientErrors = [ diff --git a/packages/cli-kit/src/private/node/api.ts b/packages/cli-kit/src/private/node/api.ts index 969bbae3611..3fb4d1e8d33 100644 --- a/packages/cli-kit/src/private/node/api.ts +++ b/packages/cli-kit/src/private/node/api.ts @@ -1,6 +1,6 @@ import {sanitizedHeadersOutput} from './api/headers.js' import {sanitizeURL} from './api/urls.js' -import {hasRateLimitCode} from './analytics/graphql-error-codes.js' +import {graphQLErrorCodes, isRateLimitCode} from './analytics/graphql-error-codes.js' import {sleepWithBackoffUntil} from './sleep-with-backoff.js' import {outputDebug} from '../../public/node/output.js' import {recordRetry} from '../../public/node/analytics.js' @@ -16,6 +16,8 @@ export const allAPIs: API[] = ['admin', 'storefront-renderer', 'partners', 'busi const DEFAULT_RETRY_DELAY_MS = 1000 const DEFAULT_RETRY_LIMIT = 10 +const MAX_THROTTLE_RETRY_DELAY_MS = 8000 +const MAX_THROTTLE_RETRY_TIME_MS = 30_000 export type NetworkRetryBehaviour = | { @@ -223,14 +225,6 @@ async function makeVerboseRequest( const sanitizedHeaders = sanitizedHeadersOutput(responseHeaders) if (isThrottled(err)) { - let delayMs: number | undefined - - try { - delayMs = responseHeaders['retry-after'] ? Number.parseInt(responseHeaders['retry-after'], 10) : undefined - // eslint-disable-next-line no-catch-all/no-catch-all - } catch { - // ignore errors in extracting retry-after header - } return { status: 'can-retry', clientError: err, @@ -238,7 +232,7 @@ async function makeVerboseRequest( sanitizedHeaders, sanitizedUrl, requestId: responseHeaders['x-request-id'], - delayMs, + delayMs: retryAfterDelayMs(responseHeaders['retry-after']), } } else if (err.response.status === 401) { return { @@ -282,15 +276,29 @@ async function makeVerboseRequest( } } -// Shopify GraphQL APIs signal rate limiting with `extensions.code` set to -// `THROTTLED` (often on a 200 response) or `429` — the same codes that -// crash-report suppression and analytics grouping already treat as rate -// limiting via this shared helper. +// Partial data can include a successful mutation. Only retry fully rejected requests. function isThrottled(error: ClientError): boolean { - if (error.response.status === 429) { - return true + if (error.response.data != null) return false + const httpRateLimited = error.response.status === 429 + const errors = error.response.errors + if (!Array.isArray(errors) || errors.length === 0) return httpRateLimited + return errors.every((entry) => { + const codes = graphQLErrorCodes([entry]) + return (codes.length > 0 || httpRateLimited) && codes.every(isRateLimitCode) + }) +} + +function retryAfterDelayMs(retryAfter: string | undefined): number | undefined { + const value = retryAfter?.trim() + if (!value) return undefined + + const seconds = Number(value) + if (!Number.isNaN(seconds)) { + return Number.isFinite(seconds) && seconds >= 0 ? seconds * 1000 : undefined } - return hasRateLimitCode(error.response.errors) + + const retryAt = Date.parse(value) + return Number.isNaN(retryAt) ? undefined : Math.max(0, retryAt - Date.now()) } export async function simpleRequestWithDebugLog( @@ -343,6 +351,8 @@ ${result.sanitizedHeaders} * Makes a HTTP request to some API, retrying if response headers indicate a retryable error. * * If a request fails with a 429, the retry-after header determines a delay before an automatic retry is performed. + * Without a valid header, rate-limit retries use exponential backoff with jitter, capped at 8 seconds per delay. + * No new rate-limit retry starts after 30 seconds. A server-requested delay beyond that budget is not shortened. * * If unauthorizedHandler is provided, then it will be called in the case of a 401 and a retry performed. This allows * for a token refresh for instance. @@ -371,6 +381,7 @@ export async function retryAwareRequest= retryDeadline - performance.now()) { + outputDebug( + `Rate-limit retry budget exhausted after ${retriesUsed} retries for request to ${result.sanitizedUrl}`, + ) if (errorHandler) { throw errorHandler(result.clientError, result.requestId) } else { @@ -414,14 +432,13 @@ ${result.sanitizedHeaders} recordRetry(requestOptions.url, `http-retry-${retriesUsed}:${result.status}:`) } - // prefer to wait based on a header if given; the caller's preference if not; and a default if neither. - const retryDelayMs = result.delayMs ?? retryOptions.defaultDelayMs ?? DEFAULT_RETRY_DELAY_MS outputDebug(`Scheduling retry request #${retriesUsed} to ${result.sanitizedUrl} in ${retryDelayMs} ms`) + const previousResult = result // eslint-disable-next-line no-await-in-loop result = await new Promise>((resolve) => { retryOptions.scheduleDelay(() => { - resolve(makeVerboseRequest(requestOptions)) + resolve(performance.now() < retryDeadline ? makeVerboseRequest(requestOptions) : previousResult) }, retryDelayMs) }) } diff --git a/packages/cli-kit/src/public/node/api/graphql.test.ts b/packages/cli-kit/src/public/node/api/graphql.test.ts index e5da87b4e7c..0694ab0378b 100644 --- a/packages/cli-kit/src/public/node/api/graphql.test.ts +++ b/packages/cli-kit/src/public/node/api/graphql.test.ts @@ -131,6 +131,57 @@ afterEach(() => { }) describe('graphqlRequest', () => { + test('retries only the throttled mutation request with its original variables', async () => { + const receivedVariables: unknown[] = [] + server.use( + mockApi.mutation('CreateAppVersion', ({variables}) => { + receivedVariables.push(variables) + if (receivedVariables.length === 1) { + return HttpResponse.json( + {errors: [{message: 'Throttled', extensions: {code: 'THROTTLED'}}]}, + {headers: {'Retry-After': '0'}}, + ) + } + return HttpResponse.json({data: {appVersionCreate: {version: {id: 'created-version'}}}}) + }), + ) + const variables = {appId: 'test-app'} + const result = await graphqlRequest({ + api: 'mockApi', + url: mockedAddress, + query: 'mutation CreateAppVersion($appId: ID!) { appVersionCreate(appId: $appId) { version { id } } }', + variables, + }) + + expect(result).toEqual({appVersionCreate: {version: {id: 'created-version'}}}) + expect(receivedVariables).toEqual([variables, variables]) + }) + + test('does not repeat a mutation when a throttled response contains partial data', async () => { + let requestCount = 0 + server.use( + mockApi.mutation('CreateAppVersion', () => { + requestCount++ + return HttpResponse.json( + { + data: {appVersionCreate: {version: {id: 'created-version'}}}, + errors: [{message: 'Throttled', extensions: {code: 'THROTTLED'}}], + }, + {headers: {'Retry-After': '0'}}, + ) + }), + ) + await expect( + graphqlRequest({ + api: 'mockApi', + url: mockedAddress, + query: 'mutation CreateAppVersion { appVersionCreate { version { id } } }', + }), + ).rejects.toThrow('Throttled') + + expect(requestCount).toBe(1) + }) + test('calls debugLogRequestInfo once', async () => { let headers: any server.events.on('request:start', ({request}) => { From e8e9a9f30c396ae0f4cb504273272d394b3b3b9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 20 Oct 2026 12:12:44 +0200 Subject: [PATCH 2/3] Preserve throttle retries for read-only GraphQL queries --- packages/cli-kit/src/private/node/api.test.ts | 30 ++++++++++++++++++ packages/cli-kit/src/private/node/api.ts | 15 +++++++-- .../src/public/node/api/graphql.test.ts | 31 +++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/packages/cli-kit/src/private/node/api.test.ts b/packages/cli-kit/src/private/node/api.test.ts index 0494763dba3..f5cb6ff6308 100644 --- a/packages/cli-kit/src/private/node/api.test.ts +++ b/packages/cli-kit/src/private/node/api.test.ts @@ -378,6 +378,36 @@ describe('retryAwareRequest', () => { expect(scheduleDelay).not.toHaveBeenCalled() }) + test.each([ + {name: 'an invalid document', query: 'not graphql'}, + { + name: 'an ambiguous document', + query: 'query Read { app { id } } mutation Create { appCreate { id } }', + }, + {name: 'a batch', query: ['query Read { app { id } }', 'mutation Create { appCreate { id } }']}, + ])('does not retry partial data for $name', async ({query}) => { + const error = new ClientError( + { + status: 200, + data: {createdVersion: {id: '123'}}, + errors: [new GraphQLError('Throttled', {extensions: {code: 'THROTTLED'}})], + headers: new Headers(), + }, + {query}, + ) + const request = vi.fn().mockRejectedValue(error) + const scheduleDelay = vi.fn() + + await expect( + retryAwareRequest({request, url: 'https://example.com/graphql', useNetworkLevelRetry: false}, undefined, { + scheduleDelay, + }), + ).rejects.toBe(error) + + expect(request).toHaveBeenCalledTimes(1) + expect(scheduleDelay).not.toHaveBeenCalled() + }) + test('fails after too many retries', async () => { // This test gives a false warning from vitest if fake timers are used. It thinks the exception is uncaught. vi.useRealTimers() diff --git a/packages/cli-kit/src/private/node/api.ts b/packages/cli-kit/src/private/node/api.ts index 3fb4d1e8d33..f2d272e59bb 100644 --- a/packages/cli-kit/src/private/node/api.ts +++ b/packages/cli-kit/src/private/node/api.ts @@ -7,6 +7,7 @@ import {recordRetry} from '../../public/node/analytics.js' import {Headers} from 'form-data' import {ClientError} from 'graphql-request' +import {getOperationAST, GraphQLError, parse} from 'graphql' import {performance} from 'perf_hooks' @@ -276,9 +277,9 @@ async function makeVerboseRequest( } } -// Partial data can include a successful mutation. Only retry fully rejected requests. +// Partial mutation data can include a successful write. Read queries are safe to repeat. function isThrottled(error: ClientError): boolean { - if (error.response.data != null) return false + if (error.response.data != null && !isReadOnlyQuery(error.request.query)) return false const httpRateLimited = error.response.status === 429 const errors = error.response.errors if (!Array.isArray(errors) || errors.length === 0) return httpRateLimited @@ -288,6 +289,16 @@ function isThrottled(error: ClientError): boolean { }) } +function isReadOnlyQuery(query: string | string[]): boolean { + if (typeof query !== 'string') return false + try { + return getOperationAST(parse(query))?.operation === 'query' + } catch (error) { + if (error instanceof GraphQLError) return false + throw error + } +} + function retryAfterDelayMs(retryAfter: string | undefined): number | undefined { const value = retryAfter?.trim() if (!value) return undefined diff --git a/packages/cli-kit/src/public/node/api/graphql.test.ts b/packages/cli-kit/src/public/node/api/graphql.test.ts index 0694ab0378b..965a16ec54b 100644 --- a/packages/cli-kit/src/public/node/api/graphql.test.ts +++ b/packages/cli-kit/src/public/node/api/graphql.test.ts @@ -13,6 +13,7 @@ import * as system from '../system.js' import {test, vi, describe, expect, beforeEach, beforeAll, afterAll, afterEach} from 'vitest' import {TypedDocumentNode} from '@graphql-typed-document-node/core' +import {parse} from 'graphql' import {setupServer} from 'msw/node' import {graphql, HttpResponse} from 'msw' @@ -131,6 +132,36 @@ afterEach(() => { }) describe('graphqlRequest', () => { + test.each([false, true])('retries a throttled nullable query (typed document: %s)', async (useTypedDocument) => { + const receivedVariables: unknown[] = [] + const data = {appsConnection: {edges: [{node: {id: 'app-123'}}]}} + server.use( + mockApi.query('ListApps', ({variables}) => { + receivedVariables.push(variables) + if (receivedVariables.length === 1) { + return HttpResponse.json( + { + data: {appsConnection: null}, + errors: [{message: 'Throttled', path: ['appsConnection'], extensions: {code: 'THROTTLED'}}], + }, + {headers: {'Retry-After': '0'}}, + ) + } + return HttpResponse.json({data}) + }), + ) + const query = + 'query ListApps($query: String) { appsConnection(query: $query, first: 50) { edges { node { id } } } }' + const variables = {query: 'title:example'} + const options = {api: 'mockApi', url: mockedAddress, variables} + const result = useTypedDocument + ? await graphqlRequestDoc({...options, query: parse(query)}) + : await graphqlRequest({...options, query}) + + expect(result).toEqual(data) + expect(receivedVariables).toEqual([variables, variables]) + }) + test('retries only the throttled mutation request with its original variables', async () => { const receivedVariables: unknown[] = [] server.use( From c9285ac277e60d83431a8ab512ee69169764eec4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 20 Oct 2026 12:16:33 +0200 Subject: [PATCH 3/3] Parse asctime Retry-After dates in UTC --- packages/cli-kit/src/private/node/api.test.ts | 11 +++++++---- packages/cli-kit/src/private/node/api.ts | 4 +++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/cli-kit/src/private/node/api.test.ts b/packages/cli-kit/src/private/node/api.test.ts index f5cb6ff6308..d9af32b23f0 100644 --- a/packages/cli-kit/src/private/node/api.test.ts +++ b/packages/cli-kit/src/private/node/api.test.ts @@ -237,12 +237,16 @@ describe('retryAwareRequest', () => { {header: '0.25', delay: 250}, {header: 'Tue, 20 Oct 2026 12:00:10 GMT', delay: 10_000}, {header: 'Tue, 20 Oct 2026 11:59:59 GMT', delay: 0}, + {header: 'Tuesday, 20-Oct-26 12:00:10 GMT', delay: 10_000}, + {header: 'Tue Oct 20 12:00:10 2026', delay: 10_000}, + {header: 'Tue Oct 20 11:59:59 2026', delay: 0}, + {header: 'Tue Oct 6 12:00:10 2026', delay: 10_000, now: '2026-10-06T12:00:00Z'}, {header: 'invalid', delay: 750}, {header: '-1', delay: 750}, {header: 'Infinity', delay: 750}, {header: ' ', delay: 750}, - ])('uses $delay milliseconds for Retry-After "$header"', async ({header, delay}) => { - vi.setSystemTime(new Date('2026-10-20T12:00:00Z')) + ])('uses $delay milliseconds for Retry-After "$header"', async ({header, delay, now}) => { + vi.setSystemTime(new Date(now ?? '2026-10-20T12:00:00Z')) const startedAt = Date.now() const success = {status: 200, headers: new Headers()} const request = vi @@ -251,8 +255,7 @@ describe('retryAwareRequest', () => { .mockResolvedValue(success) const result = retryAwareRequest({request, url: 'https://example.com/graphql', useNetworkLevelRetry: false}) const assertion = expect(result).resolves.toBe(success) - await vi.runAllTimersAsync() - await assertion + await Promise.all([vi.runAllTimersAsync(), assertion]) expect(request).toHaveBeenCalledTimes(2) expect(Date.now() - startedAt).toBe(delay) diff --git a/packages/cli-kit/src/private/node/api.ts b/packages/cli-kit/src/private/node/api.ts index f2d272e59bb..e124470c259 100644 --- a/packages/cli-kit/src/private/node/api.ts +++ b/packages/cli-kit/src/private/node/api.ts @@ -308,7 +308,9 @@ function retryAfterDelayMs(retryAfter: string | undefined): number | undefined { return Number.isFinite(seconds) && seconds >= 0 ? seconds * 1000 : undefined } - const retryAt = Date.parse(value) + // The asctime HTTP-date format omits a time zone but still uses UTC. + const isAsctimeDate = /^[A-Za-z]{3} [A-Za-z]{3} (?: \d|\d{2}) \d{2}:\d{2}:\d{2} \d{4}$/.test(value) + const retryAt = Date.parse(isAsctimeDate ? `${value} GMT` : value) return Number.isNaN(retryAt) ? undefined : Math.max(0, retryAt - Date.now()) }