Merge pull request #7317 from Shopify/jf-on-error

add watcher error reporting in theme fs

Author: EvilGenius13

.changeset/fix-theme-fs-watcher-error.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+Add error handler to chokidar file watcher to prevent unhandled exceptions

packages/theme/src/cli/utilities/theme-fs.test.ts

@@ -18,6 +18,7 @@ import {bulkUploadThemeAssets, deleteThemeAssets, fetchThemeAssets} from '@shopi
 import {renderError} from '@shopify/cli-kit/node/ui'
 import {Operation, type Checksum, type ThemeAsset} from '@shopify/cli-kit/node/themes/types'
 import {dirname, joinPath} from '@shopify/cli-kit/node/path'
+import {recordError} from '@shopify/cli-kit/node/analytics'
 import {AdminSession} from '@shopify/cli-kit/node/session'
 
 import EventEmitter from 'events'
@@ -33,6 +34,13 @@ vi.mock('./asset-ignore.js')
 vi.mock('@shopify/cli-kit/node/themes/api')
 vi.mock('@shopify/cli-kit/node/ui')
 vi.mock('@shopify/cli-kit/node/output')
+vi.mock('@shopify/cli-kit/node/analytics', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('@shopify/cli-kit/node/analytics')>()
+  return {
+    ...actual,
+    recordError: vi.fn(),
+  }
+})
 vi.mock('./theme-environment/hot-reload/server.js')
 
 beforeEach(async () => {
@@ -851,6 +859,35 @@ describe('theme-fs', () => {
     })
   })
 
+  describe('watcher error handling', () => {
+    const themeId = '123'
+    const adminSession = {token: 'token'} as AdminSession
+    const root = joinPath(locationOfThisFile, 'fixtures/theme')
+
+    beforeEach(() => {
+      const mockWatcher = new EventEmitter()
+      vi.spyOn(chokidar, 'watch').mockImplementation((_) => {
+        return mockWatcher as any
+      })
+    })
+
+    test('outputs a warning when the watcher emits an error', async () => {
+      // Given
+      const {outputWarn} = await import('@shopify/cli-kit/node/output')
+      const themeFileSystem = mountThemeFileSystem(root)
+      await themeFileSystem.ready()
+      await themeFileSystem.startWatcher(themeId, adminSession)
+
+      // When
+      const watcher = chokidar.watch('') as EventEmitter
+      watcher.emit('error', new Error('EMFILE: too many open files'))
+
+      // Then
+      expect(outputWarn).toHaveBeenCalledWith('File watcher error: Error: EMFILE: too many open files')
+      expect(recordError).toHaveBeenCalledWith('theme-service:file-watcher:error')
+    })
+  })
+
   describe('handleSyncUpdate', () => {
     const fileKey = 'assets/test.css'
     const themeId = '123'

packages/theme/src/cli/utilities/theme-fs.ts

@@ -10,6 +10,7 @@ import {joinPath, basename, relativePath} from '@shopify/cli-kit/node/path'
 import {lookupMimeType, setMimeTypes} from '@shopify/cli-kit/node/mimes'
 import {outputContent, outputDebug, outputInfo, outputToken, outputWarn} from '@shopify/cli-kit/node/output'
 import {buildThemeAsset} from '@shopify/cli-kit/node/themes/factories'
+import {recordError} from '@shopify/cli-kit/node/analytics'
 import {AdminSession} from '@shopify/cli-kit/node/session'
 import {bulkUploadThemeAssets, deleteThemeAssets} from '@shopify/cli-kit/node/themes/api'
 
@@ -347,6 +348,10 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti
         .on('add', queueFsEvent.bind(null, 'add'))
         .on('change', queueFsEvent.bind(null, 'change'))
         .on('unlink', queueFsEvent.bind(null, 'unlink'))
+        .on('error', (error) => {
+          outputWarn(`File watcher error: ${error}`)
+          recordError('theme-service:file-watcher:error')
+        })
     },
   }
 }