diff --git a/.changeset/silent-ghosts-hide.md b/.changeset/silent-ghosts-hide.md new file mode 100644 index 0000000000..9a40be70c5 --- /dev/null +++ b/.changeset/silent-ghosts-hide.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Make `shopify theme dev` more resilient to HTTP errors with the Admin API diff --git a/packages/cli-kit/src/public/node/themes/api.test.ts b/packages/cli-kit/src/public/node/themes/api.test.ts index 1a7be0fa6a..be7b966cd1 100644 --- a/packages/cli-kit/src/public/node/themes/api.test.ts +++ b/packages/cli-kit/src/public/node/themes/api.test.ts @@ -17,6 +17,7 @@ import {restRequest} from '@shopify/cli-kit/node/api/admin' import {AbortError} from '@shopify/cli-kit/node/error' vi.mock('@shopify/cli-kit/node/api/admin') +vi.mock('@shopify/cli-kit/node/system') const session = {token: 'token', storeFqdn: 'my-shop.myshopify.com'} @@ -305,9 +306,14 @@ describe('deleteTheme', () => { }) describe('request errors', () => { - const httpErrors = [401, 403, 500, 999] - - httpErrors.forEach((httpError) => { + const httpResponses = [ + {httpError: 401, expectedRetries: 3}, + {httpError: 403, expectedRetries: 1}, + {httpError: 500, expectedRetries: 3}, + {httpError: 999, expectedRetries: 1}, + ] + + httpResponses.forEach(({httpError, expectedRetries}) => { test(`${httpError} errors`, async () => { // Given vi.mocked(restRequest).mockResolvedValue({ @@ -316,12 +322,12 @@ describe('request errors', () => { headers: {}, }) - await expect(async () => { - // When - return deleteTheme(1, session) + // When + const deletePromise = async () => deleteTheme(1, session) - // Then - }).rejects.toThrowError(AbortError) + // Then + await expect(deletePromise).rejects.toThrowError(AbortError) + expect(restRequest).toBeCalledTimes(expectedRetries) }) }) }) diff --git a/packages/cli-kit/src/public/node/themes/api.ts b/packages/cli-kit/src/public/node/themes/api.ts index 8cf3e4f846..f104516344 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/packages/cli-kit/src/public/node/themes/api.ts @@ -10,6 +10,8 @@ import { buildThemeAsset, } from '@shopify/cli-kit/node/themes/factories' import {Result, Checksum, Key, Theme, ThemeAsset} from '@shopify/cli-kit/node/themes/types' +import {outputDebug} from '@shopify/cli-kit/node/output' +import {sleep} from '@shopify/cli-kit/node/system' export type ThemeParams = Partial> export type AssetParams = Pick & Partial> @@ -108,6 +110,7 @@ async function request( session: AdminSession, params?: T, searchParams: {[name: string]: string} = {}, + retries = 1, ): Promise { const response = await throttler.throttle(() => restRequest(method, path, session, params, searchParams)) @@ -128,13 +131,33 @@ async function request( case status === 403: return handleForbiddenError(response, session) case status === 401: - throw new AbortError(`[${status}] API request unauthorized error`) + // Retry 401 errors to be resilient to authentication errors. + return handleRetriableError({ + path, + retries, + retry: () => { + return request(method, path, session, params, searchParams, retries + 1) + }, + fail: () => { + throw new AbortError(`[${status}] API request unauthorized error`) + }, + }) case status === 422: throw new AbortError(`[${status}] API request unprocessable content: ${errors(response)}`) case status >= 400 && status <= 499: throw new AbortError(`[${status}] API request client error`) case status >= 500 && status <= 599: - throw new AbortError(`[${status}] API request server error`) + // Retry 500-family of errors as that may solve the issue (especially in 503 errors) + return handleRetriableError({ + path, + retries, + retry: () => { + return request(method, path, session, params, searchParams, retries + 1) + }, + fail: () => { + throw new AbortError(`[${status}] API request server error`) + }, + }) default: throw new AbortError(`[${status}] API request unexpected error`) } @@ -175,3 +198,21 @@ function errorMessage(response: RestResponse): string { return '' } + +interface RetriableErrorOptions { + path: string + retries: number + retry: () => Promise + fail: () => never +} + +async function handleRetriableError({path, retries, retry, fail}: RetriableErrorOptions): Promise { + if (retries >= 3) { + fail() + } + + outputDebug(`[${retries}] Retrying '${path}' request...`) + + await sleep(0.2) + return retry() +}