fix(dev): do not restart when a client aborts a connection (#1412)

Author: kedrzu

packages/nuxt-cli/src/dev/index.ts

@@ -6,7 +6,7 @@ import type { NuxtDevContext, NuxtDevIPCMessage, NuxtParentIPCMessage } from './
 import process from 'node:process'
 import defu from 'defu'
 import { overrideEnv } from '../utils/env.ts'
-import { isBrokenPipe } from '../utils/errors'
+import { isRemotePeerError } from '../utils/errors'
 import { debug } from '../utils/logger'
 import { startCpuProfile, stopCpuProfile } from '../utils/profile.ts'
 import { openInspector } from './inspect'
@@ -18,6 +18,22 @@ function formatErrorMessage(error: unknown): string {
   return error instanceof Error ? error.toString() : 'Unhandled Rejection'
 }
 
+/**
+ * Hand an unhandled rejection to the parent process and stop this one, unless
+ * it is only a client that went away — that is traffic, not a crash, and the
+ * session has to survive it.
+ */
+export function createRejectionHandler(report: (message: string) => void, stop: () => void): (reason: unknown) => void {
+  return (reason: unknown) => {
+    if (isRemotePeerError(reason)) {
+      debug('Ignoring remote peer error:', reason)
+      return
+    }
+    report(formatErrorMessage(reason))
+    stop()
+  }
+}
+
 interface InitializeOptions {
   data?: {
     overrides?: NuxtConfig
@@ -37,10 +53,10 @@ class IPC {
       process.once('disconnect', () => {
         process.exit(0)
       })
-      process.once('unhandledRejection', (reason) => {
-        this.send({ type: 'nuxt:internal:dev:rejection', message: formatErrorMessage(reason) })
-        process.exit()
-      })
+      process.on('unhandledRejection', createRejectionHandler(
+        message => this.send({ type: 'nuxt:internal:dev:rejection', message }),
+        () => process.exit(),
+      ))
     }
     process.on('message', async (message: NuxtParentIPCMessage) => {
       if (message.type === 'nuxt:internal:dev:context') {
@@ -219,8 +235,8 @@ export function createRestartHook(source: RestartSource): (callback: (reason?: D
   }
 
   function restartOnError(error: unknown) {
-    if (isBrokenPipe(error)) {
-      debug('Ignoring broken pipe:', error)
+    if (isRemotePeerError(error)) {
+      debug('Ignoring remote peer error:', error)
       return
     }
     restart({ type: 'error', message: formatErrorMessage(error) })

packages/nuxt-cli/src/utils/console.ts

@@ -4,7 +4,7 @@ import process from 'node:process'
 
 import { consola } from 'consola'
 
-import { isBrokenPipe } from './errors'
+import { isRemotePeerError } from './errors'
 import { debug } from './logger'
 
 // Filter out unwanted logs
@@ -68,8 +68,8 @@ export function restoreRawMode(): void {
 }
 
 function report(label: string, error: unknown) {
-  if (isBrokenPipe(error)) {
-    debug(`${label} ignoring broken pipe:`, error)
+  if (isRemotePeerError(error)) {
+    debug(`${label} ignoring remote peer error:`, error)
     return
   }
   consola.error(label, error)

packages/nuxt-cli/src/utils/errors.ts

@@ -1,10 +1,18 @@
+const REMOTE_PEER_ERROR_CODES = new Set([
+  'EPIPE',
+  'ERR_STREAM_DESTROYED',
+  'ECONNRESET',
+  'ECONNABORTED',
+  'ERR_STREAM_PREMATURE_CLOSE',
+])
+
 /**
- * A pipe closing early is a property of the other end of the pipe, not a fault
- * in the dev server: a clipboard tool that exited before we wrote to it, or
- * `nuxt dev | head` closing stdout. Such errors should never be reported as
- * crashes or trigger a restart.
+ * Errors that say something about the other end of a connection rather than
+ * about this process: a broken pipe, a client hanging up mid-request, a tab
+ * closed mid-navigation. These should not be reported as crashes or trigger
+ * a restart.
  */
-export function isBrokenPipe(error: unknown): boolean {
+export function isRemotePeerError(error: unknown): boolean {
   const code = (error as NodeJS.ErrnoException | undefined)?.code
-  return code === 'EPIPE' || code === 'ERR_STREAM_DESTROYED'
+  return !!code && REMOTE_PEER_ERROR_CODES.has(code)
 }

packages/nuxt-cli/test/unit/errors.spec.ts

@@ -1,18 +1,21 @@
 import { describe, expect, it } from 'vitest'
 
 import { stripCwd } from '../../src/dev/error'
-import { isBrokenPipe } from '../../src/utils/errors'
+import { isRemotePeerError } from '../../src/utils/errors'
 
-describe('isBrokenPipe', () => {
-  it('should detect closed pipes', () => {
-    expect(isBrokenPipe(Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }))).toBe(true)
-    expect(isBrokenPipe(Object.assign(new Error('destroyed'), { code: 'ERR_STREAM_DESTROYED' }))).toBe(true)
+describe('isRemotePeerError', () => {
+  it('should detect errors from the other end of a connection', () => {
+    expect(isRemotePeerError(Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }))).toBe(true)
+    expect(isRemotePeerError(Object.assign(new Error('destroyed'), { code: 'ERR_STREAM_DESTROYED' }))).toBe(true)
+    expect(isRemotePeerError(Object.assign(new Error('read ECONNRESET'), { code: 'ECONNRESET' }))).toBe(true)
+    expect(isRemotePeerError(Object.assign(new Error('aborted'), { code: 'ECONNABORTED' }))).toBe(true)
+    expect(isRemotePeerError(Object.assign(new Error('premature close'), { code: 'ERR_STREAM_PREMATURE_CLOSE' }))).toBe(true)
   })
 
   it('should ignore other errors', () => {
-    expect(isBrokenPipe(new Error('boom'))).toBe(false)
-    expect(isBrokenPipe(Object.assign(new Error('nope'), { code: 'ENOENT' }))).toBe(false)
-    expect(isBrokenPipe(undefined)).toBe(false)
+    expect(isRemotePeerError(new Error('boom'))).toBe(false)
+    expect(isRemotePeerError(Object.assign(new Error('nope'), { code: 'ENOENT' }))).toBe(false)
+    expect(isRemotePeerError(undefined)).toBe(false)
   })
 })
 

packages/nuxt-cli/test/unit/restart-hook.spec.ts

@@ -5,7 +5,7 @@ import process from 'node:process'
 
 import { afterEach, describe, expect, it, vi } from 'vitest'
 
-import { createRestartHook } from '../../src/dev'
+import { createRejectionHandler, createRestartHook } from '../../src/dev'
 
 const ERROR_EVENTS = ['uncaughtException', 'unhandledRejection'] as const
 
@@ -48,7 +48,7 @@ describe('restart hook', () => {
     expect(callback).toHaveBeenCalledExactlyOnceWith({ type: 'shortcut' })
   })
 
-  it('should restart on an error that is not a broken pipe', () => {
+  it('should restart on an error that is not a remote peer error', () => {
     const source = new EventEmitter()
     const callback = vi.fn()
     arm(source, callback)
@@ -57,6 +57,9 @@ describe('restart hook', () => {
     onError!(Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }))
     expect(callback).not.toHaveBeenCalled()
 
+    onError!(Object.assign(new Error('read ECONNRESET'), { code: 'ECONNRESET' }))
+    expect(callback).not.toHaveBeenCalled()
+
     onError!(new Error('boom'))
     expect(callback).toHaveBeenCalledWith({ type: 'error', message: expect.stringContaining('boom') })
   })
@@ -106,3 +109,36 @@ describe('restart hook', () => {
     expect(second).toHaveBeenCalledTimes(1)
   })
 })
+
+describe('rejection handler', () => {
+  it('should report the rejection and stop', () => {
+    const report = vi.fn()
+    const stop = vi.fn()
+
+    createRejectionHandler(report, stop)(new Error('boom'))
+
+    expect(report).toHaveBeenCalledExactlyOnceWith(expect.stringContaining('boom'))
+    expect(stop).toHaveBeenCalledTimes(1)
+  })
+
+  it('should describe a rejection that is not an error', () => {
+    const report = vi.fn()
+
+    createRejectionHandler(report, vi.fn())('nope')
+
+    expect(report).toHaveBeenCalledExactlyOnceWith('Unhandled Rejection')
+  })
+
+  it('should keep the process alive on remote peer errors', () => {
+    const report = vi.fn()
+    const stop = vi.fn()
+    const handle = createRejectionHandler(report, stop)
+
+    handle(Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }))
+    handle(Object.assign(new Error('read ECONNRESET'), { code: 'ECONNRESET' }))
+    handle(Object.assign(new Error('premature close'), { code: 'ERR_STREAM_PREMATURE_CLOSE' }))
+
+    expect(report).not.toHaveBeenCalled()
+    expect(stop).not.toHaveBeenCalled()
+  })
+})