fix(#146): do not swallow uncaught exception/unhandled rejection when…

… importing the EdgeRuntime module

.changeset/rich-deers-tie.md

@@ -0,0 +1,5 @@
+---
+'@edge-runtime/vm': patch
+---
+
+Do not swallow uncaught exception/unhandled rejection when importing the EdgeRuntime module.

packages/runtime/src/edge-runtime.ts

@@ -36,21 +36,26 @@ export class EdgeRuntime<
       },
     })
 
-    defineHandlerProps({
-      object: this,
-      setterName: '__onUnhandledRejectionHandler',
-      setter: (handlers: RejectionHandler[]) =>
-        (unhandledRejectionHandlers = handlers),
-      getterName: '__rejectionHandlers',
-      getter: () => unhandledRejectionHandlers,
+    Object.defineProperty(this.context, '__onUnhandledRejectionHandlers', {
+      set: registerUnhandledRejectionHandlers,
+      configurable: false,
+      enumerable: false,
+    })
+    Object.defineProperty(this, '__rejectionHandlers', {
+      get: () => unhandledRejectionHandlers,
+      configurable: false,
+      enumerable: false,
     })
-    defineHandlerProps({
-      object: this,
-      setterName: '__onErrorHandler',
-      setter: (handlers: ErrorHandler[]) =>
-        (uncaughtExceptionHandlers = handlers),
-      getterName: '__errorHandlers',
-      getter: () => uncaughtExceptionHandlers,
+
+    Object.defineProperty(this.context, '__onErrorHandlers', {
+      set: registerUncaughtExceptionHandlers,
+      configurable: false,
+      enumerable: false,
+    })
+    Object.defineProperty(this, '__errorHandlers', {
+      get: () => uncaughtExceptionHandlers,
+      configurable: false,
+      enumerable: false,
     })
 
     this.evaluate<void>(getDefineEventListenersCode())
@@ -62,20 +67,32 @@ export class EdgeRuntime<
 }
 
 /**
- * Define system-level handlers to make sure that we report to the user
+ * Registere system-level handlers to make sure that we report to the user
  * whenever there is an unhandled rejection or exception before the process crashes.
+ * Do it on demand so we don't swallow rejections/errors for no reason.
  */
-process.on(
-  'unhandledRejection',
-  function invokeRejectionHandlers(reason, promise) {
-    unhandledRejectionHandlers?.forEach((handler) =>
-      handler({ reason, promise })
+function registerUnhandledRejectionHandlers(handlers: RejectionHandler[]) {
+  if (!unhandledRejectionHandlers) {
+    process.on(
+      'unhandledRejection',
+      function invokeRejectionHandlers(reason, promise) {
+        unhandledRejectionHandlers.forEach((handler) =>
+          handler({ reason, promise })
+        )
+      }
     )
   }
-)
-process.on('uncaughtException', function invokeErrorHandlers(error) {
-  uncaughtExceptionHandlers?.forEach((handler) => handler(error))
-})
+  unhandledRejectionHandlers = handlers
+}
+
+function registerUncaughtExceptionHandlers(handlers: ErrorHandler[]) {
+  if (!uncaughtExceptionHandlers) {
+    process.on('uncaughtException', function invokeErrorHandlers(error) {
+      uncaughtExceptionHandlers.forEach((handler) => handler(error))
+    })
+  }
+  uncaughtExceptionHandlers = handlers
+}
 
 /**
  * Generates polyfills for addEventListener and removeEventListener. It keeps
@@ -95,9 +112,9 @@ function getDefineEventListenersCode() {
 
     function __conditionallyUpdatesHandlerList(eventType) {
       if (eventType === 'unhandledrejection') {
-        self.__onUnhandledRejectionHandler = self.__listeners[eventType];
+        self.__onUnhandledRejectionHandlers = self.__listeners[eventType];
       } else if (eventType === 'error') {
-        self.__onErrorHandler = self.__listeners[eventType];
+        self.__onErrorHandlers = self.__listeners[eventType];
       }
     }
 
@@ -172,33 +189,3 @@ function getDispatchFetchCode() {
       .catch(error => getResponse({ error }))
   })`
 }
-
-/**
- * Defines a readable property on the VM and the corresponding writable property
- * on the VM's context. These properties are not enumerable nor updatable.
- */
-function defineHandlerProps({
-  object: instance,
-  setterName,
-  setter: setter,
-  getterName,
-  getter,
-}: {
-  object: EdgeRuntime
-  setterName: string
-  setter: (_: any) => void
-  getterName: string
-  getter: () => any
-}) {
-  Object.defineProperty(instance.context, setterName, {
-    set: setter,
-    configurable: false,
-    enumerable: false,
-  })
-
-  Object.defineProperty(instance, getterName, {
-    get: getter,
-    configurable: false,
-    enumerable: false,
-  })
-}

packages/runtime/tests/fixtures/legit-uncaught-exception.ts

@@ -0,0 +1,4 @@
+import { EdgeRuntime } from '../../src'
+
+new EdgeRuntime()
+throw new Error('intentional break')

packages/runtime/tests/fixtures/legit-unhandled-rejection.ts

@@ -0,0 +1,4 @@
+import { EdgeRuntime } from '../../src'
+
+new EdgeRuntime()
+Promise.reject(new Error('intentional break'))

packages/runtime/tests/fixtures/uncaught-exception.ts

@@ -0,0 +1,20 @@
+import { EdgeRuntime } from '../../src'
+import assert from 'assert'
+
+function main() {
+  const runtime = new EdgeRuntime()
+  runtime.context.handleError = (error: Error) => {
+    assert.strictEqual(error?.message, 'expected error')
+    console.log('TEST PASSED!')
+  }
+
+  runtime.evaluate(`
+    addEventListener('error', (error) => {
+      globalThis.handleError(error)
+    })
+    
+    throw new Error('expected error')
+  `)
+}
+
+main()

packages/runtime/tests/unhandled-rejection.ts → packages/runtime/tests/fixtures/unhandled-rejection.ts

@@ -1,4 +1,4 @@
-import { EdgeRuntime, runServer } from '../src'
+import { EdgeRuntime, runServer } from '../../src'
 import assert from 'assert'
 import fetch from 'node-fetch'
 import util from 'util'
@@ -44,19 +44,15 @@ async function main() {
       event.reason.message,
       'This ReadableStream did not return bytes.'
     )
-  } catch (error) {
-    return util.inspect(error)
+    return 'TEST PASSED!'
   } finally {
     await server.close()
   }
-
-  return 'TEST PASSED!'
 }
 
 main()
   .then(console.log)
   .catch((error) => {
     console.log('TEST FAILED!')
     console.log(error)
-    process.exit(1)
   })

packages/runtime/tests/rejections-and-errors.test.ts

@@ -0,0 +1,60 @@
+import { exec } from 'child_process'
+import { promisify } from 'util'
+import { resolve } from 'path'
+
+jest.setTimeout(2000)
+const execAsync = promisify(exec)
+
+it('handles correctly unhandled rejections', async () => {
+  const result = await execAsync(
+    `ts-node --transpile-only ${resolve(
+      __dirname,
+      './fixtures/unhandled-rejection.ts'
+    )}`,
+    { encoding: 'utf8' }
+  )
+  expect(result).toMatchObject({
+    stdout: expect.stringContaining('TEST PASSED!'),
+    stderr: '',
+  })
+})
+
+it('handles correctly uncaught exceptions', async () => {
+  const result = await execAsync(
+    `ts-node --transpile-only ${resolve(
+      __dirname,
+      './fixtures/uncaught-exception.ts'
+    )}`,
+    { encoding: 'utf8' }
+  )
+  expect(result).toMatchObject({
+    stdout: expect.stringContaining('TEST PASSED!'),
+    stderr: '',
+  })
+})
+
+it('does not swallow uncaught exceptions outside of evaluation', async () => {
+  const execAsync = promisify(exec)
+  await expect(
+    execAsync(
+      `ts-node --transpile-only ${resolve(
+        __dirname,
+        './fixtures/legit-uncaught-exception.ts'
+      )}`,
+      { encoding: 'utf8' }
+    )
+  ).rejects.toThrow(/intentional break/)
+})
+
+it('does not swallow unhandled rejections outside of evaluation', async () => {
+  const execAsync = promisify(exec)
+  await expect(
+    execAsync(
+      `ts-node --transpile-only ${resolve(
+        __dirname,
+        './fixtures/legit-unhandled-rejection.ts'
+      )}`,
+      { encoding: 'utf8' }
+    )
+  ).rejects.toThrow(/intentional break/)
+})

packages/vm/src/vm.ts

@@ -14,7 +14,7 @@ export interface VMOptions<T> {
    * Allows to extend the VMContext. Note that it must return a contextified
    * object so ideally it should return the same reference it receives.
    */
-  extend?: (context: VMContext) =>VMContext & T
+  extend?: (context: VMContext) => VMContext & T
   /**
    * Provides an initial map to the require cache.
    * If none is given, it will be initialized to an empty map.
@@ -45,7 +45,7 @@ export class VM<T extends Dictionary> {
     ) as VMContext
 
     this.requireCache = options.requireCache ?? new Map()
-    this.context = options.extend?.(context) ?? context as VMContext & T
+    this.context = options.extend?.(context) ?? (context as VMContext & T)
     this.requireFn = createRequire(this.context, this.requireCache)
   }