Don't crash on unhandled promise rejections with the CLI, closes #115

...instead, log them. Adds a new `logUnhandledRejections` option.
If a user has an `unhandledrejection` event listener in their
worker, the rejection will only be logged if the user doesn't call
`preventDefault()` on the event.

Author: mrbbot

packages/core/src/index.ts

@@ -624,7 +624,8 @@ export class MiniflareCore<
       const setupWatch = this.#setupWatch!.get(name);
       if (setupWatch) addAll(newWatchPaths, setupWatch);
     }
-    const { modules, processedModuleRules } = this.#instances!.CorePlugin;
+    const { modules, processedModuleRules, logUnhandledRejections } =
+      this.#instances!.CorePlugin;
 
     // Clean up process-wide promise rejection event listeners
     this.#globalScope?.[kDispose]();
@@ -633,7 +634,8 @@ export class MiniflareCore<
       this.#ctx.log,
       globals,
       bindings,
-      modules
+      modules,
+      logUnhandledRejections
     );
     this.#globalScope = globalScope;
     this.#bindings = bindings;

packages/core/src/plugins/core.ts

@@ -96,6 +96,7 @@ export interface CoreOptions {
   // Replaced in MiniflareCoreOptions with something plugins-specific
   mounts?: Record<string, string | CoreOptions | BindingsOptions>;
   routes?: string[];
+  logUnhandledRejections?: boolean;
 }
 
 function mapMountEntries([name, pathEnv]: [string, string]): [
@@ -291,6 +292,9 @@ export class CorePlugin extends Plugin<CoreOptions> implements CoreOptions {
   })
   routes?: string[];
 
+  @Option({ type: OptionType.NONE })
+  logUnhandledRejections?: boolean;
+
   readonly processedModuleRules: ProcessedModuleRule[] = [];
 
   readonly upstreamURL?: URL;

packages/core/src/standards/event.ts

@@ -6,6 +6,7 @@ import {
   ThrowingEventTarget,
   TypedEventListener,
   ValueOf,
+  prefixError,
 } from "@miniflare/shared";
 import { Response as BaseResponse, fetch as baseFetch } from "undici";
 import { DOMException } from "./domexception";
@@ -178,18 +179,33 @@ export type WorkerGlobalScopeEventMap = {
 
 export class WorkerGlobalScope extends ThrowingEventTarget<WorkerGlobalScopeEventMap> {}
 
+// true will be added to this set if #logUnhandledRejections is true so we
+// don't remove the listener on removeEventListener, and know to dispose it.
+type PromiseListenerSetMember =
+  | TypedEventListener<PromiseRejectionEvent>
+  | boolean;
+
+type PromiseListener =
+  | {
+      name: "unhandledRejection";
+      set: Set<PromiseListenerSetMember>;
+      listener: (reason: any, promise: Promise<any>) => void;
+    }
+  | {
+      name: "rejectionHandled";
+      set: Set<PromiseListenerSetMember>;
+      listener: (promise: Promise<any>) => void;
+    };
+
 export class ServiceWorkerGlobalScope extends WorkerGlobalScope {
   readonly #log: Log;
   readonly #bindings: Context;
   readonly #modules?: boolean;
+  readonly #logUnhandledRejections?: boolean;
   #calledAddFetchEventListener = false;
 
-  readonly #rejectionHandledListeners = new Set<
-    TypedEventListener<PromiseRejectionEvent>
-  >();
-  readonly #unhandledRejectionListeners = new Set<
-    TypedEventListener<PromiseRejectionEvent>
-  >();
+  readonly #unhandledRejection: PromiseListener;
+  readonly #rejectionHandled: PromiseListener;
 
   // Global self-references
   readonly global = this;
@@ -199,12 +215,29 @@ export class ServiceWorkerGlobalScope extends WorkerGlobalScope {
     log: Log,
     globals: Context,
     bindings: Context,
-    modules?: boolean
+    modules?: boolean,
+    logUnhandledRejections?: boolean
   ) {
     super();
     this.#log = log;
     this.#bindings = bindings;
     this.#modules = modules;
+    this.#logUnhandledRejections = logUnhandledRejections;
+
+    this.#unhandledRejection = {
+      name: "unhandledRejection",
+      set: new Set(),
+      listener: this.#unhandledRejectionListener,
+    };
+    this.#rejectionHandled = {
+      name: "rejectionHandled",
+      set: new Set(),
+      listener: this.#rejectionHandledListener,
+    };
+    // If we're logging unhandled rejections, register the process-wide listener
+    if (this.#logUnhandledRejections) {
+      this.#maybeAddPromiseListener(this.#unhandledRejection, true);
+    }
 
     // Only including bindings in global scope if not using modules
     Object.assign(this, globals);
@@ -248,24 +281,10 @@ export class ServiceWorkerGlobalScope extends WorkerGlobalScope {
     // Register process wide unhandledRejection/rejectionHandled listeners if
     // not already done so
     if (type === "unhandledrejection" && listener) {
-      if (this.#unhandledRejectionListeners.size === 0) {
-        this.#log.verbose("Adding process unhandledRejection listener...");
-        process.prependListener(
-          "unhandledRejection",
-          this.#unhandledRejectionListener
-        );
-      }
-      this.#unhandledRejectionListeners.add(listener as any);
+      this.#maybeAddPromiseListener(this.#unhandledRejection, listener);
     }
     if (type === "rejectionhandled" && listener) {
-      if (this.#rejectionHandledListeners.size === 0) {
-        this.#log.verbose("Adding process rejectionHandled listener...");
-        process.prependListener(
-          "rejectionHandled",
-          this.#rejectionHandledListener
-        );
-      }
-      this.#rejectionHandledListeners.add(listener as any);
+      this.#maybeAddPromiseListener(this.#rejectionHandled, listener);
     }
 
     super.addEventListener(type, listener, options);
@@ -286,26 +305,10 @@ export class ServiceWorkerGlobalScope extends WorkerGlobalScope {
     // Unregister process wide rejectionHandled/unhandledRejection listeners if
     // no longer needed and not already done so
     if (type === "unhandledrejection" && listener) {
-      const registered = this.#unhandledRejectionListeners.size > 0;
-      this.#unhandledRejectionListeners.delete(listener as any);
-      if (registered && this.#unhandledRejectionListeners.size === 0) {
-        this.#log.verbose("Removing process unhandledRejection listener...");
-        process.removeListener(
-          "unhandledRejection",
-          this.#unhandledRejectionListener
-        );
-      }
+      this.#maybeRemovePromiseListener(this.#unhandledRejection, listener);
     }
     if (type === "rejectionhandled" && listener) {
-      const registered = this.#rejectionHandledListeners.size > 0;
-      this.#rejectionHandledListeners.delete(listener as any);
-      if (registered && this.#rejectionHandledListeners.size === 0) {
-        this.#log.verbose("Removing process rejectionHandled listener...");
-        process.removeListener(
-          "rejectionHandled",
-          this.#rejectionHandledListener
-        );
-      }
+      this.#maybeRemovePromiseListener(this.#rejectionHandled, listener);
     }
 
     super.removeEventListener(type, listener, options);
@@ -423,22 +426,50 @@ export class ServiceWorkerGlobalScope extends WorkerGlobalScope {
     return (await Promise.all(event[kWaitUntil])) as WaitUntil;
   }
 
+  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+  #maybeAddPromiseListener(listener: PromiseListener, member: any): void {
+    if (listener.set.size === 0) {
+      this.#log.verbose(`Adding process ${listener.name} listener...`);
+      process.prependListener(listener.name as any, listener.listener as any);
+    }
+    listener.set.add(member);
+  }
+  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+  #maybeRemovePromiseListener(listener: PromiseListener, member: any): void {
+    const registered = listener.set.size > 0;
+    listener.set.delete(member);
+    if (registered && listener.set.size === 0) {
+      this.#log.verbose(`Removing process ${listener.name} listener...`);
+      process.removeListener(listener.name, listener.listener);
+    }
+  }
+  #resetPromiseListener(listener: PromiseListener): void {
+    if (listener.set.size > 0) {
+      this.#log.verbose(`Removing process ${listener.name} listener...`);
+      process.removeListener(listener.name, listener.listener);
+    }
+    listener.set.clear();
+  }
+
   // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
   #unhandledRejectionListener = (reason: any, promise: Promise<any>): void => {
     const event = new PromiseRejectionEvent("unhandledrejection", {
       reason,
       promise,
     });
     const notCancelled = super.dispatchEvent(event);
-    // If the event wasn't preventDefault()ed, remove the listener and cause
-    // an unhandled promise rejection again. This should terminate the program.
+    // If the event wasn't preventDefault()ed,
     if (notCancelled) {
-      process.removeListener(
-        "unhandledRejection",
-        this.#unhandledRejectionListener
-      );
-      // noinspection JSIgnoredPromiseFromCall
-      Promise.reject(reason);
+      if (this.#logUnhandledRejections) {
+        // log if we're logging unhandled rejections
+        this.#log.error(prefixError("Unhandled Promise Rejection", reason));
+      } else {
+        // ...otherwise, remove the listener and cause an unhandled promise
+        // rejection again. This should terminate the program.
+        this.#resetPromiseListener(this.#unhandledRejection);
+        // noinspection JSIgnoredPromiseFromCall
+        Promise.reject(reason);
+      }
     }
   };
 
@@ -449,22 +480,7 @@ export class ServiceWorkerGlobalScope extends WorkerGlobalScope {
   };
 
   [kDispose](): void {
-    if (this.#unhandledRejectionListeners.size > 0) {
-      this.#log.verbose("Removing process unhandledRejection listener...");
-      process.removeListener(
-        "unhandledRejection",
-        this.#unhandledRejectionListener
-      );
-    }
-    this.#unhandledRejectionListeners.clear();
-
-    if (this.#rejectionHandledListeners.size > 0) {
-      this.#log.verbose("Removing process rejectionHandled listener...");
-      process.removeListener(
-        "rejectionHandled",
-        this.#rejectionHandledListener
-      );
-    }
-    this.#rejectionHandledListeners.clear();
+    this.#resetPromiseListener(this.#unhandledRejection);
+    this.#resetPromiseListener(this.#rejectionHandled);
   }
 }

packages/core/test/plugins/core.spec.ts

@@ -187,6 +187,7 @@ test("CorePlugin: parses options from wrangler config", (t) => {
       "http://localhost:8787/*",
       "miniflare.mf:8787/*",
     ],
+    logUnhandledRejections: undefined,
   });
   // Check build upload dir defaults to dist
   options = parsePluginWranglerConfig(

packages/core/test/standards/event.promise.spec.ts

@@ -1,3 +1,4 @@
+import { setTimeout } from "timers/promises";
 import {
   PromiseRejectionEvent,
   ServiceWorkerGlobalScope,
@@ -176,3 +177,48 @@ test.serial(
     t.is(event.promise, promise);
   }
 );
+
+// Test logUnhandledRejections option
+test.serial(
+  "ServiceWorkerGlobalScope: logs unhandled rejections",
+  async (t) => {
+    const log = new TestLog();
+    const [logTrigger, logPromise] = triggerPromise<Error>();
+    log.error = logTrigger;
+    new ServiceWorkerGlobalScope(log, {}, {}, false, true);
+
+    const error = new Error("Oops, did I do that?");
+    // noinspection ES6MissingAwait
+    Promise.reject(error);
+
+    const event = await logPromise;
+    t.regex(
+      event.stack!,
+      /^Unhandled Promise Rejection: Error: Oops, did I do that\?/
+    );
+  }
+);
+test.serial(
+  "ServiceWorkerGlobalScope: doesn't log unhandled rejections if preventDefault() called",
+  async (t) => {
+    const log = new TestLog();
+    const globalScope = new ServiceWorkerGlobalScope(log, {}, {}, false, true);
+
+    const [eventTrigger, eventPromise] =
+      triggerPromise<PromiseRejectionEvent>();
+    globalScope.addEventListener("unhandledrejection", (e) => {
+      e.preventDefault();
+      eventTrigger(e);
+    });
+
+    const error = new Error("Oops, did I do that?");
+    // noinspection ES6MissingAwait
+    const promise = Promise.reject(error);
+
+    const event = await eventPromise;
+    t.is(event.promise, promise);
+
+    await setTimeout();
+    t.is(log.logsAtLevel(LogLevel.ERROR).length, 0);
+  }
+);

packages/http-server/src/index.ts

@@ -14,7 +14,7 @@ import {
   Response,
   logResponse,
 } from "@miniflare/core";
-import { randomHex } from "@miniflare/shared";
+import { prefixError, randomHex } from "@miniflare/shared";
 import { coupleWebSocket } from "@miniflare/web-sockets";
 import { BodyInit, Headers } from "undici";
 import { getAccessibleHosts } from "./helpers";
@@ -307,15 +307,7 @@ export function createRequestListener<Plugins extends HTTPPluginSignatures>(
         }
 
         // Add method and URL to stack trace
-        const proxiedError = new Proxy(e, {
-          get(target, propertyKey, receiver) {
-            const value = Reflect.get(target, propertyKey, receiver);
-            return propertyKey === "stack"
-              ? `${req.method} ${req.url}: ${value}`
-              : value;
-          },
-        });
-        mf.log.error(proxiedError);
+        mf.log.error(prefixError(`${req.method} ${req.url}`, e));
       }
     }
 

packages/miniflare/src/cli.ts

@@ -82,8 +82,12 @@ async function main() {
     ? LogLevel.DEBUG
     : LogLevel.INFO;
   const mfOptions = options as MiniflareOptions;
+
   mfOptions.log = new Log(logLevel);
   mfOptions.sourceMap = true;
+  // Catch and log unhandled rejections as opposed to crashing
+  mfOptions.logUnhandledRejections = true;
+
   const mf = new Miniflare(mfOptions);
   try {
     // Start Miniflare development server

packages/shared/src/error.ts

@@ -9,3 +9,16 @@ export abstract class MiniflareError<
     this.name = `${new.target.name} [${code}]`;
   }
 }
+
+// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
+export function prefixError(prefix: string, e: any): Error {
+  if (e.stack) {
+    return new Proxy(e, {
+      get(target, propertyKey, receiver) {
+        const value = Reflect.get(target, propertyKey, receiver);
+        return propertyKey === "stack" ? `${prefix}: ${value}` : value;
+      },
+    });
+  }
+  return e;
+}