Make shopify theme dev more resilient to HTTP errors with the Admin…

… API

.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

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)
  })
  })
 })

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<Pick<Theme, 'name' | 'role' | 'processing' | 'src'>>
 export type AssetParams = Pick<ThemeAsset, 'key'> & Partial<Pick<ThemeAsset, 'value' | 'attachment'>>
@@ -108,6 +110,7 @@ async function request<T>(
  session: AdminSession,
  params?: T,
  searchParams: {[name: string]: string} = {},
+ retries = 1,
 ): Promise<RestResponse> {
  const response = await throttler.throttle(() => restRequest(method, path, session, params, searchParams))
 
@@ -128,13 +131,33 @@ async function request<T>(
  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<RestResponse>
+ fail: () => never
+}
+
+async function handleRetriableError({path, retries, retry, fail}: RetriableErrorOptions): Promise<RestResponse> {
+ if (retries >= 3) {
+ fail()
+ }
+
+ outputDebug(`[${retries}] Retrying '${path}' request...`)
+
+ await sleep(0.2)
+ return retry()
+}