Feat: Add retries and fallback logic to Host theme creation flow

Shopify · Jul 22, 2024 · f8212bf · f8212bf
1 parent 4e90537
commit f8212bf
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 30 deletions.
diff --git a/packages/app/src/cli/utilities/host-theme-manager.test.ts b/packages/app/src/cli/utilities/host-theme-manager.test.ts
@@ -1,4 +1,4 @@
-import {DEFAULT_THEME_ZIP, HostThemeManager} from './host-theme-manager.js'
+import {DEFAULT_THEME_ZIP, FALLBACK_THEME_ZIP, HostThemeManager} from './host-theme-manager.js'
 import {waitForThemeToBeProcessed} from './host-theme-watcher.js'
 import {createTheme} from '@shopify/cli-kit/node/themes/api'
 import {beforeEach, describe, expect, test, vi} from 'vitest'
@@ -10,12 +10,15 @@ vi.mock('@shopify/cli-kit/node/themes/api')
 vi.mock('./host-theme-watcher.js')
 
 describe('HostThemeManager', () => {
+  let themeManager: HostThemeManager
+  const adminSession: AdminSession = {token: 'token', storeFqdn: 'storeFqdn'}
+
   beforeEach(() => {
+    themeManager = new HostThemeManager(adminSession, {devPreview: true})
     vi.spyOn(ThemeManager.prototype, 'generateThemeName').mockImplementation(() => 'App Ext. Host Name')
   })
+
   test('should call createTheme with the provided name and src param', async () => {
-    const adminSession: AdminSession = {token: 'token', storeFqdn: 'storeFqdn'}
-    const themeManager = new HostThemeManager(adminSession, {devPreview: true})
     vi.mocked(createTheme).mockResolvedValue({
       id: 12345,
       name: 'Theme',
@@ -39,15 +42,7 @@ describe('HostThemeManager', () => {
   })
 
   describe('dev preview', () => {
-    let themeManager: HostThemeManager
-
-    beforeEach(() => {
-      const adminSession: AdminSession = {token: 'token', storeFqdn: 'storeFqdn'}
-      themeManager = new HostThemeManager(adminSession, {devPreview: true})
-      vi.spyOn(ThemeManager.prototype, 'generateThemeName').mockImplementation(() => 'App Ext. Host Name')
-    })
-
-    test('should call createHostTheme if devPreview is true', async () => {
+    test('should call createTheme with the provided name and src param', async () => {
       // Given
       vi.mocked(createTheme).mockResolvedValue({
         id: 12345,
@@ -71,7 +66,7 @@ describe('HostThemeManager', () => {
       )
     })
 
-    test('should call createTheme with the provided name and src param', async () => {
+    test('should wait for the theme to be processed', async () => {
       // Given
       vi.mocked(createTheme).mockResolvedValue({
         id: 12345,
@@ -80,37 +75,89 @@ describe('HostThemeManager', () => {
         createdAtRuntime: true,
         processing: true,
       })
+      vi.mocked(waitForThemeToBeProcessed).mockResolvedValue()
 
       // When
       await themeManager.findOrCreate()
 
       // Then
-      expect(createTheme).toHaveBeenCalledWith(
-        {
-          name: 'App Ext. Host Name',
-          role: DEVELOPMENT_THEME_ROLE,
-          src: DEFAULT_THEME_ZIP,
-        },
-        {storeFqdn: 'storeFqdn', token: 'token'},
-      )
+      expect(waitForThemeToBeProcessed).toHaveBeenCalledTimes(1)
     })
 
-    test('should wait for the theme to be processed', async () => {
+    test('should retry creating the theme if the first attempt fails', async () => {
       // Given
-      vi.mocked(createTheme).mockResolvedValue({
+      vi.mocked(createTheme).mockResolvedValueOnce(undefined).mockResolvedValueOnce({
         id: 12345,
         name: 'Theme',
         role: 'development',
         createdAtRuntime: true,
         processing: true,
       })
-      vi.mocked(waitForThemeToBeProcessed).mockResolvedValue()
 
       // When
       await themeManager.findOrCreate()
 
       // Then
-      expect(waitForThemeToBeProcessed).toHaveBeenCalledTimes(1)
+      expect(createTheme).toHaveBeenCalledTimes(2)
+      expect(createTheme).toHaveBeenNthCalledWith(
+        1,
+        {
+          role: DEVELOPMENT_THEME_ROLE,
+          name: 'App Ext. Host Name',
+          src: DEFAULT_THEME_ZIP,
+        },
+        adminSession,
+      )
+      expect(createTheme).toHaveBeenNthCalledWith(
+        2,
+        {
+          role: DEVELOPMENT_THEME_ROLE,
+          name: 'App Ext. Host Name',
+          src: DEFAULT_THEME_ZIP,
+        },
+        adminSession,
+      )
+    })
+
+    test('should retry creating the theme with the Fallback theme zip after 3 failed retry attempts', async () => {
+      // Given
+      vi.mocked(createTheme)
+        .mockResolvedValueOnce(undefined)
+        .mockResolvedValueOnce(undefined)
+        .mockResolvedValueOnce(undefined)
+        .mockResolvedValue({
+          id: 12345,
+          name: 'Theme',
+          role: 'development',
+          createdAtRuntime: true,
+          processing: true,
+        })
+
+      // When
+      await themeManager.findOrCreate()
+
+      // Then
+      expect(createTheme).toHaveBeenCalledTimes(4)
+      expect(createTheme).toHaveBeenLastCalledWith(
+        {
+          role: DEVELOPMENT_THEME_ROLE,
+          name: 'App Ext. Host Name',
+          src: FALLBACK_THEME_ZIP,
+        },
+        adminSession,
+      )
+    })
+
+    test('should throw a BugError if the theme cannot be created', async () => {
+      // Given
+      vi.mocked(createTheme).mockResolvedValue(undefined)
+
+      // When
+      // Then
+      await expect(themeManager.findOrCreate()).rejects.toThrow(
+        'Could not create theme with name "App Ext. Host Name" and role "development"',
+      )
+      expect(createTheme).toHaveBeenCalledTimes(4)
     })
   })
 })
diff --git a/packages/app/src/cli/utilities/host-theme-manager.ts b/packages/app/src/cli/utilities/host-theme-manager.ts
@@ -3,11 +3,15 @@ import {getHostTheme, removeHostTheme, setHostTheme} from '@shopify/cli-kit/node
 import {ThemeManager} from '@shopify/cli-kit/node/themes/theme-manager'
 import {AdminSession} from '@shopify/cli-kit/node/session'
 import {Theme} from '@shopify/cli-kit/node/themes/types'
-import {BugError} from '@shopify/cli-kit/node/error'
 import {createTheme} from '@shopify/cli-kit/node/themes/api'
 import {DEVELOPMENT_THEME_ROLE} from '@shopify/cli-kit/node/themes/utils'
+import {BugError} from '@shopify/cli-kit/node/error'
+import {outputDebug} from '@shopify/cli-kit/node/output'
+
+export const DEFAULT_THEME_ZIP = 'https://codeload.github.com/Shopify/dawn/zip/refs/tags/v15.0.0'
+export const FALLBACK_THEME_ZIP = 'https://cdn.shopify.com/theme-store/uhrdefhlndzaoyrgylhto59sx2i7.jpg'
+const retryAttemps = 3
 
-export const DEFAULT_THEME_ZIP = 'https://cdn.shopify.com/theme-store/uhrdefhlndzaoyrgylhto59sx2i7.jpg'
 export class HostThemeManager extends ThemeManager {
   protected context = 'App Ext. Host'
   protected devPreview: boolean
@@ -41,12 +45,30 @@ export class HostThemeManager extends ThemeManager {
       src: DEFAULT_THEME_ZIP,
     }
 
-    const theme = await createTheme(options, this.adminSession)
+    for (let attempt = 0; attempt < retryAttemps; attempt++) {
+      outputDebug(
+        `Attempt ${attempt}/${retryAttemps}: Creating theme with name "${options.name}" and role "${options.role}"`,
+      )
+      // eslint-disable-next-line no-await-in-loop
+      const theme = await createTheme(options, this.adminSession)
+
+      if (theme) {
+        this.setTheme(theme.id.toString())
+        outputDebug(`Waiting for theme with id "${theme.id}" to be processed`)
+        // eslint-disable-next-line no-await-in-loop
+        await waitForThemeToBeProcessed(theme.id, this.adminSession)
+        return theme
+      } else {
+        outputDebug(`Failed to create theme with name "${options.name}" and role "${options.role}". Retrying...`)
+      }
+    }
+
+    outputDebug(`Theme creation failed after ${retryAttemps} retries. Creating theme using fallback theme zip`)
+    const theme = await createTheme({...options, src: FALLBACK_THEME_ZIP}, this.adminSession)
     if (!theme) {
+      outputDebug(`Theme creation failed. Exiting process.`)
       throw new BugError(`Could not create theme with name "${options.name}" and role "${options.role}"`)
     }
-    this.setTheme(theme.id.toString())
-    await waitForThemeToBeProcessed(theme.id, this.adminSession)
     return theme
   }
 }