[Themes] Store theme asset upload errors in the themeFileSystem

Shopify · Jan 16, 2025 · 6edd52f · 6edd52f
1 parent 7d889bd
commit 6edd52f
Show file tree

Hide file tree

Showing 7 changed files with 108 additions and 6 deletions.
diff --git a/packages/cli-kit/src/public/node/themes/types.ts b/packages/cli-kit/src/public/node/themes/types.ts
@@ -99,6 +99,11 @@ export interface ThemeFileSystem extends VirtualFileSystem {
    * Applies filters to ignore files from .shopifyignore file, --ignore and --only flags.
    */
   applyIgnoreFilters: <T extends {key: string}>(files: T[]) => T[]
+
+  /**
+   * Stores upload errors returned when uploading files via the Asset API
+   */
+  uploadErrors: Map<Key, string[]>
 }
 
 /**

diff --git a/packages/theme/src/cli/utilities/theme-fs-empty.ts b/packages/theme/src/cli/utilities/theme-fs-empty.ts
@@ -13,6 +13,7 @@ function emptyFileSystem<T>(): T {
     root: '',
     files: new Map(),
     unsyncedFileKeys: new Set(),
+    uploadErrors: new Map(),
     ready: () => Promise.resolve(),
     delete: async (_: string) => {},
     read: async (_: string) => '',

diff --git a/packages/theme/src/cli/utilities/theme-fs.test.ts b/packages/theme/src/cli/utilities/theme-fs.test.ts
@@ -59,6 +59,7 @@ describe('theme-fs', () => {
           fsEntry({checksum: '64caf742bd427adcf497bffab63df30c', key: 'templates/404.json'}),
         ]),
         unsyncedFileKeys: new Set(),
+        uploadErrors: new Map(),
         ready: expect.any(Function),
         delete: expect.any(Function),
         write: expect.any(Function),
@@ -82,6 +83,7 @@ describe('theme-fs', () => {
         root,
         files: new Map(),
         unsyncedFileKeys: new Set(),
+        uploadErrors: new Map(),
         ready: expect.any(Function),
         delete: expect.any(Function),
         write: expect.any(Function),

diff --git a/packages/theme/src/cli/utilities/theme-fs.ts b/packages/theme/src/cli/utilities/theme-fs.ts
@@ -45,6 +45,7 @@ const THEME_PARTITION_REGEX = {
 
 export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOptions): ThemeFileSystem {
   const files = new Map<string, ThemeAsset>()
+  const uploadErrors = new Map<string, string[]>()
   const unsyncedFileKeys = new Set<string>()
   const filterPatterns = {
     ignoreFromFile: [] as string[],
@@ -147,12 +148,12 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti
 
         const [result] = await bulkUploadThemeAssets(Number(themeId), [{key: fileKey, value: content}], adminSession)
 
-        if (!result?.success) {
-          throw new Error(
-            result?.errors?.asset
-              ? `\n\n${result.errors.asset.map((error) => `- ${error}`).join('\n')}`
-              : 'Response was not successful.',
-          )
+        if (result?.success) {
+          uploadErrors.delete(fileKey)
+        } else {
+          const errors = result?.errors?.asset ?? ['Response was not successful.']
+          uploadErrors.set(fileKey, errors)
+          throw new Error(errors.join('\n'))
         }
 
         unsyncedFileKeys.delete(fileKey)
@@ -223,6 +224,7 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti
     root,
     files,
     unsyncedFileKeys,
+    uploadErrors,
     ready: () => themeSetupPromise,
     delete: async (fileKey: string) => {
       files.delete(fileKey)

diff --git a/packages/theme/src/cli/utilities/theme-fs/theme-fs-mock-factory.ts b/packages/theme/src/cli/utilities/theme-fs/theme-fs-mock-factory.ts
@@ -10,6 +10,7 @@ export function fakeThemeFileSystem(
     root,
     files,
     unsyncedFileKeys: new Set(),
+    uploadErrors: new Map(),
     ready: () => Promise.resolve(),
     delete: async (fileKey: string) => {
       files.delete(fileKey)

diff --git a/packages/theme/src/cli/utilities/theme-uploader.test.ts b/packages/theme/src/cli/utilities/theme-uploader.test.ts
@@ -4,6 +4,7 @@ import {
   MAX_UPLOAD_RETRY_COUNT,
   MINIMUM_THEME_ASSETS,
   uploadTheme,
+  updateUploadErrors,
 } from './theme-uploader.js'
 import {fakeThemeFileSystem} from './theme-fs/theme-fs-mock-factory.js'
 import {bulkUploadThemeAssets, deleteThemeAsset} from '@shopify/cli-kit/node/themes/api'
@@ -602,3 +603,83 @@ describe('theme-uploader', () => {
     )
   })
 })
+
+describe('updateUploadErrors', () => {
+  test('should clear error for successful upload', () => {
+    // Given
+    const themeFileSystem = fakeThemeFileSystem('tmp', new Map())
+    const result: Result = {
+      key: 'file1.liquid',
+      success: true,
+      errors: {},
+      operation: Operation.Upload,
+      asset: {key: 'file1.liquid', checksum: 'abc'},
+    }
+
+    // Pre-existing error that should be cleared
+    themeFileSystem.uploadErrors.set('file1.liquid', ['Old error'])
+
+    // When
+    updateUploadErrors(result, themeFileSystem)
+
+    // Then
+    expect(themeFileSystem.uploadErrors.has('file1.liquid')).toBe(false)
+  })
+
+  test('should set error for failed upload', () => {
+    // Given
+    const themeFileSystem = fakeThemeFileSystem('tmp', new Map())
+    const result: Result = {
+      key: 'file1.liquid',
+      success: false,
+      errors: {asset: ['Upload failed']},
+      operation: Operation.Upload,
+      asset: {key: 'file1.liquid', checksum: 'abc'},
+    }
+
+    // When
+    updateUploadErrors(result, themeFileSystem)
+
+    // Then
+    expect(themeFileSystem.uploadErrors.get('file1.liquid')).toEqual(['Upload failed'])
+  })
+
+  test('should set default error when no specific error provided', () => {
+    // Given
+    const themeFileSystem = fakeThemeFileSystem('tmp', new Map())
+    const result: Result = {
+      key: 'file1.liquid',
+      success: false,
+      errors: {},
+      operation: Operation.Upload,
+      asset: {key: 'file1.liquid', checksum: 'abc'},
+    }
+
+    // When
+    updateUploadErrors(result, themeFileSystem)
+
+    // Then
+    expect(themeFileSystem.uploadErrors.get('file1.liquid')).toEqual(['Response was not successful.'])
+  })
+
+  test('should update existing error', () => {
+    // Given
+    const themeFileSystem = fakeThemeFileSystem('tmp', new Map())
+    const result: Result = {
+      key: 'file1.liquid',
+      success: false,
+      errors: {asset: ['New error']},
+      operation: Operation.Upload,
+      asset: {key: 'file1.liquid', checksum: 'abc'},
+    }
+
+    // Pre-existing error that should be updated
+    themeFileSystem.uploadErrors.set('file1.liquid', ['Old error'])
+
+    // When
+    updateUploadErrors(result, themeFileSystem)
+
+    // Then
+    expect(themeFileSystem.uploadErrors.get('file1.liquid')).toEqual(['New error'])
+  })
+})
diff --git a/packages/theme/src/cli/utilities/theme-uploader.ts b/packages/theme/src/cli/utilities/theme-uploader.ts
@@ -370,9 +370,19 @@ async function uploadBatch(
   // store the results in uploadResults, overwriting any existing results
   results.forEach((result) => {
     uploadResults.set(result.key, result)
+    updateUploadErrors(result, localThemeFileSystem)
   })
 }
 
+export function updateUploadErrors(result: Result, localThemeFileSystem: ThemeFileSystem) {
+  if (result.success) {
+    localThemeFileSystem.uploadErrors.delete(result.key)
+  } else {
+    const errors = result.errors?.asset ?? ['Response was not successful.']
+    localThemeFileSystem.uploadErrors.set(result.key, errors)
+  }
+}
+
 async function handleBulkUpload(
   uploadParams: AssetParams[],
   themeId: number,