Merge pull request #5283 from Shopify/01-27-all_validations_errors_should_be_handled_by_the_dev-session

All validations errors should be handled by the dev-session

Author: isaacroldan

packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts

@@ -382,8 +382,8 @@ describe('app-event-watcher', () => {
         }
 
         // Given
-        const esbuildError = {message: 'Build failed'}
-        flowExtension.buildForBundle = vi.fn().mockRejectedValueOnce(esbuildError)
+        const buildError = {message: 'Build failed'}
+        flowExtension.buildForBundle = vi.fn().mockRejectedValueOnce(buildError)
 
         const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle')
         const app = testAppLinked({
@@ -406,6 +406,44 @@ describe('app-event-watcher', () => {
         expect(stderr.write).toHaveBeenCalledWith(`Build failed`)
       })
     })
+
+    test('uncaught errors are emitted', async () => {
+      await inTemporaryDirectory(async (tmpDir) => {
+        const fileWatchEvent: WatcherEvent = {
+          type: 'file_updated',
+          path: '/extensions/ui_extension_1/src/file.js',
+          extensionPath: '/extensions/ui_extension_1',
+          startTime: [0, 0],
+        }
+
+        // Given
+        const uncaughtError = new Error('Unexpected error')
+
+        const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle')
+        const app = testAppLinked({
+          allExtensions: [extension1],
+          configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'},
+        })
+        const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
+
+        // When
+        const mockManager = new MockESBuildContextManager()
+        mockManager.updateContexts = vi.fn().mockRejectedValueOnce(uncaughtError)
+
+        const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
+        const stderr = {write: vi.fn()} as unknown as Writable
+        const stdout = {write: vi.fn()} as unknown as Writable
+        const errorHandler = vi.fn()
+        watcher.onError(errorHandler)
+
+        await watcher.start({stdout, stderr, signal: abortController.signal})
+
+        await flushPromises()
+
+        // Then
+        expect(errorHandler).toHaveBeenCalledWith(uncaughtError)
+      })
+    })
   })
 })
 // Mock class for ESBuildContextManager

packages/app/src/cli/services/dev/app-events/app-event-watcher.ts

@@ -165,7 +165,7 @@ export class AppEventWatcher extends EventEmitter {
           this.emit('all', appEvent)
         })
         .catch((error) => {
-          this.options.stderr.write(`Error handling event: ${error.message}`)
+          this.emit('error', error)
         })
     })
     await this.fileWatcher.start()
@@ -204,6 +204,12 @@ export class AppEventWatcher extends EventEmitter {
     return this
   }
 
+  onError(listener: (error: Error) => Promise<void> | void) {
+    // eslint-disable-next-line @typescript-eslint/no-misused-promises
+    this.addListener('error', listener)
+    return this
+  }
+
   /**
    * Deletes the build output for the given extensions.
    *

packages/app/src/cli/services/dev/processes/dev-session.test.ts

@@ -196,6 +196,7 @@ describe('pushUpdatesForDevSession', () => {
     expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Action required'))
     expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Scopes updated'))
     expect(contextSpy).toHaveBeenCalledWith({outputPrefix: 'dev-session', stripAnsi: false}, expect.anything())
+    contextSpy.mockRestore()
   })
 
   test('update is retried if there is an error', async () => {
@@ -269,4 +270,18 @@ describe('pushUpdatesForDevSession', () => {
     expect(devSessionStatusManager.status.isReady).toBe(false)
     expect(devSessionStatusManager.status.previewURL).toBeUndefined()
   })
+
+  test('handles error events from the app watcher', async () => {
+    // Given
+    const testError = new Error('Watcher error')
+
+    // When
+    await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options)
+    appWatcher.emit('error', testError)
+    await flushPromises()
+
+    // Then
+    expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Error'))
+    expect(stdout.write).toHaveBeenCalledWith(expect.stringContaining('Watcher error'))
+  })
 })

packages/app/src/cli/services/dev/processes/dev-session.ts

@@ -150,6 +150,9 @@ export const pushUpdatesForDevSession: DevProcessFunction<DevSessionOptions> = a
       const result = await bundleExtensionsAndUpload({...processOptions, app: event.app})
       await handleDevSessionResult(result, {...processOptions, app: event.app})
     })
+    .onError(async (error) => {
+      await handleDevSessionResult({status: 'unknown-error', error}, processOptions)
+    })
 }
 
 async function handleDevSessionResult(