Surface local proxy errors during app dev

.changeset/proxy-error-visibility.md

@@ -0,0 +1,5 @@
+---
+'@shopify/app': patch
+---
+
+Surface local proxy errors during `app dev`: unmatched HTTP paths and websocket upgrades now log a warning instead of failing silently, the "Proxy server started" line only prints after the socket actually binds, and a fatal proxy error (failed bind, or a runtime error after bind) tears the dev session down instead of leaving the proxy dead with no further indication.

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

@@ -32,13 +32,17 @@ import {DeveloperPlatformClient} from '../../../utilities/developer-platform-cli
 import {AppEventWatcher} from '../app-events/app-event-watcher.js'
 import * as loader from '../../../models/app/loader.js'
 import {describe, test, expect, beforeEach, vi} from 'vitest'
+import {AbortController} from '@shopify/cli-kit/node/abort'
 import {ensureAuthenticatedAdmin, ensureAuthenticatedStorefront} from '@shopify/cli-kit/node/session'
 import {Config} from '@oclif/core'
 import {getEnvironmentVariables} from '@shopify/cli-kit/node/environment'
 import {isStorefrontPasswordProtected} from '@shopify/theme'
 import {fetchTheme} from '@shopify/cli-kit/node/themes/api'
 import {firstPartyDev} from '@shopify/cli-kit/node/context/local'
 import {adminFqdn} from '@shopify/cli-kit/node/context/fqdn'
+import {Writable} from 'stream'
+import net from 'net'
+import http from 'http'
 
 vi.mock('../../context/identifiers.js')
 vi.mock('@shopify/cli-kit/node/session.js')
@@ -737,3 +741,51 @@ describe('setup-dev-processes', () => {
     })
   })
 })
+
+describe('startProxyServer', () => {
+  const sinkStream = () =>
+    new Writable({
+      write(_chunk, _encoding, next) {
+        next()
+      },
+    })
+
+  test('rejects with a user-friendly EADDRINUSE message when the port is already in use', async () => {
+    // Hold a port so the proxy bind fails with EADDRINUSE.
+    const blocker = http.createServer()
+    await new Promise<void>((resolve) => blocker.listen(0, 'localhost', resolve))
+    const port = (blocker.address() as net.AddressInfo).port
+
+    const abortController = new AbortController()
+    try {
+      await expect(
+        startProxyServer(
+          {abortSignal: abortController.signal, stdout: sinkStream(), stderr: sinkStream()},
+          {port, rules: {default: 'http://localhost:1'}},
+        ),
+      ).rejects.toThrow(`Reverse HTTP proxy error - Port ${port} is already in use`)
+    } finally {
+      abortController.abort()
+      await new Promise<void>((resolve) => blocker.close(() => resolve()))
+    }
+  })
+
+  test('stays alive after bind and resolves cleanly when the abort signal fires', async () => {
+    const abortController = new AbortController()
+    let resolved = false
+    const proxyPromise = startProxyServer(
+      {abortSignal: abortController.signal, stdout: sinkStream(), stderr: sinkStream()},
+      {port: 0, rules: {default: 'http://localhost:1'}},
+    ).then(() => {
+      resolved = true
+    })
+
+    // Give the proxy enough time to bind and enter its long-running wait.
+    await new Promise<void>((resolve) => setTimeout(resolve, 50))
+    expect(resolved).toBe(false)
+
+    abortController.abort()
+    await expect(proxyPromise).resolves.toBeUndefined()
+    expect(resolved).toBe(true)
+  })
+})

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

@@ -25,7 +25,8 @@ import {getAvailableTCPPort} from '@shopify/cli-kit/node/tcp'
 import {isTruthy} from '@shopify/cli-kit/node/context/utilities'
 import {firstPartyDev} from '@shopify/cli-kit/node/context/local'
 import {getEnvironmentVariables} from '@shopify/cli-kit/node/environment'
-import {outputInfo} from '@shopify/cli-kit/node/output'
+import {outputInfo, outputWarn} from '@shopify/cli-kit/node/output'
+import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components'
 import {adminFqdn} from '@shopify/cli-kit/node/context/fqdn'
 
 interface ProxyServerProcess extends BaseProcess<{
@@ -295,9 +296,57 @@ export const startProxyServer: DevProcessFunction<{
   localhostCert?: LocalhostCert
 }> = async ({abortSignal, stdout}, {port, rules, localhostCert}) => {
   const {server} = await getProxyingWebServer(rules, abortSignal, localhostCert, stdout)
+
+  // `server.listen` is event-based and returns the Server synchronously, so awaiting it
+  // does not actually wait for the socket to bind. Wrap it in a promise that resolves on
+  // 'listening' and rejects on 'error' (e.g. EADDRINUSE) so a failed bind surfaces to the
+  // dev runner instead of crashing Node via an uncaught 'error' event.
+  await new Promise<void>((resolve, reject) => {
+    const onError = (err: NodeJS.ErrnoException) => {
+      server.off('listening', onListening)
+      reject(translateBindError(err, port))
+    }
+    const onListening = () => {
+      server.off('error', onError)
+      resolve()
+    }
+    server.once('error', onError)
+    server.once('listening', onListening)
+    server.listen(port, 'localhost')
+  })
+
   outputInfo(
     `Proxy server started on port ${port} ${localhostCert ? `with certificate ${localhostCert.certPath}` : ''}`,
     stdout,
   )
-  await server.listen(port, 'localhost')
+
+  // Stay alive for the lifetime of the dev session. Resolve cleanly when the abort signal
+  // fires; reject if the server emits a post-bind 'error' so a dead proxy tears down the
+  // dev session instead of leaving the user with one warning line and silently broken
+  // request forwarding for the rest of the session.
+  await new Promise<void>((resolve, reject) => {
+    const cleanup = () => {
+      abortSignal.removeEventListener('abort', onAbort)
+      server.off('error', onError)
+    }
+    const onAbort = () => {
+      cleanup()
+      resolve()
+    }
+    const onError = (err: Error) => {
+      cleanup()
+      useConcurrentOutputContext({outputPrefix: 'proxy', stripAnsi: false}, () => {
+        outputWarn(`Reverse HTTP proxy error - Server stopped: ${err.message}`, stdout)
+      })
+      reject(err)
+    }
+    server.on('error', onError)
+    abortSignal.addEventListener('abort', onAbort, {once: true})
+  })
+}
+
+function translateBindError(err: NodeJS.ErrnoException, port: number): Error {
+  if (err.code === 'EADDRINUSE') return new Error(`Reverse HTTP proxy error - Port ${port} is already in use`)
+  if (err.code === 'EACCES') return new Error(`Reverse HTTP proxy error - Permission denied binding port ${port}`)
+  return new Error(`Reverse HTTP proxy error - Couldn't start on port ${port}: ${err.message}`)
 }

packages/app/src/cli/utilities/app/http-reverse-proxy.test.ts

@@ -69,6 +69,56 @@ describe.sequential.each(each)('http-reverse-proxy for %s', (protocol) => {
   })
 })
 
+describe.sequential('http-reverse-proxy without a default rule', () => {
+  test('returns 500 for unmatched HTTP paths', {retry: 2}, async () => {
+    const abortController = new AbortController()
+    const targetServer = http.createServer((_req, res) => {
+      res.writeHead(200)
+      res.end('ok')
+    })
+    await new Promise<void>((resolve) => targetServer.listen(0, 'localhost', resolve))
+    const targetPort = (targetServer.address() as net.AddressInfo).port
+
+    const {server} = await getProxyingWebServer({'/known': `http://localhost:${targetPort}`}, abortController.signal)
+    await new Promise<void>((resolve) => server.listen(0, 'localhost', resolve))
+    const proxyPort = (server.address() as net.AddressInfo).port
+
+    try {
+      const response = await fetch(`http://localhost:${proxyPort}/unknown`, {
+        agent: new http.Agent({keepAlive: false}),
+      })
+      expect(response.status).toBe(500)
+      await expect(response.text()).resolves.toContain('Invalid path')
+    } finally {
+      server.closeAllConnections()
+      await new Promise<void>((resolve) => server.close(() => resolve()))
+      targetServer.closeAllConnections()
+      await new Promise<void>((resolve) => targetServer.close(() => resolve()))
+    }
+  })
+
+  test('destroys websocket connections that do not match any rule', {retry: 2}, async () => {
+    const abortController = new AbortController()
+    const {server} = await getProxyingWebServer({'/known': 'http://localhost:1'}, abortController.signal)
+    await new Promise<void>((resolve) => server.listen(0, 'localhost', resolve))
+    const proxyPort = (server.address() as net.AddressInfo).port
+
+    try {
+      await new Promise<void>((resolve, reject) => {
+        const ws = new WebSocket(`ws://localhost:${proxyPort}/unmatched`, {
+          agent: new http.Agent({keepAlive: false}),
+        })
+        ws.on('open', () => reject(new Error('connection should not have opened')))
+        ws.on('error', () => resolve())
+        ws.on('unexpected-response', () => resolve())
+      })
+    } finally {
+      server.closeAllConnections()
+      await new Promise<void>((resolve) => server.close(() => resolve()))
+    }
+  })
+})
+
 function getTestReverseProxy(protocol: 'http' | 'https') {
   return test.extend<{
     setup: {

packages/app/src/cli/utilities/app/http-reverse-proxy.ts

@@ -58,6 +58,9 @@ function getProxyServerWebsocketUpgradeListener(
         })
       })
     }
+    useConcurrentOutputContext({outputPrefix: 'proxy', stripAnsi: false}, () => {
+      outputWarn(`Reverse HTTP proxy error - Invalid websocket path: ${req.url ?? ''}`, stdout)
+    })
     socket.destroy()
   }
 }
@@ -80,6 +83,9 @@ function getProxyServerRequestListener(
       })
     }
 
+    useConcurrentOutputContext({outputPrefix: 'proxy', stripAnsi: false}, () => {
+      outputWarn(`Reverse HTTP proxy error - Invalid path: ${req.url ?? ''}`, stdout)
+    })
     outputDebug(outputContent`
 Reverse HTTP proxy error - Invalid path: ${req.url ?? ''}
 These are the allowed paths: