From e9bfcc4fba324642dc4d424bfd8cbf8009554560 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Mon, 23 Sep 2024 16:22:18 -0700
Subject: [PATCH] Handle errors thrown by reportError in queueMicrotask

---
 src/workerd/api/BUILD.bazel                   |  6 ++++
 src/workerd/api/global-scope.c++              | 15 +++++++++-
 .../api/tests/error-in-error-event-test.js    | 30 +++++++++++++++++++
 .../tests/error-in-error-event-test.wd-test   | 18 +++++++++++
 4 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 src/workerd/api/tests/error-in-error-event-test.js
 create mode 100644 src/workerd/api/tests/error-in-error-event-test.wd-test

diff --git a/src/workerd/api/BUILD.bazel b/src/workerd/api/BUILD.bazel
index 7713e536263..b15f85fc4b5 100644
--- a/src/workerd/api/BUILD.bazel
+++ b/src/workerd/api/BUILD.bazel
@@ -565,3 +565,9 @@ wd_test(
     args = ["--experimental"],
     data = ["tests/cross-context-promise-test.js"],
 )
+
+wd_test(
+    src = "tests/error-in-error-event-test.wd-test",
+    args = ["--experimental"],
+    data = ["tests/error-in-error-event-test.js"],
+)
diff --git a/src/workerd/api/global-scope.c++ b/src/workerd/api/global-scope.c++
index 0afa3f81ef9..56f2947cb9e 100644
--- a/src/workerd/api/global-scope.c++
+++ b/src/workerd/api/global-scope.c++
@@ -706,7 +706,20 @@ void ServiceWorkerGlobalScope::queueMicrotask(jsg::Lock& js, v8::Local<v8::Funct
               return jsg::check(
                   function->Call(js.v8Context(), js.v8Null(), argv.size(), argv.data()));
             }, [&](jsg::Value exception) {
-              reportError(js, jsg::JsValue(exception.getHandle(js)));
+              // The reportError call itself can potentially throw errors. Let's catch
+              // and report them as well.
+              js.tryCatch([&] { reportError(js, jsg::JsValue(exception.getHandle(js))); },
+                  [&](jsg::Value exception) {
+                if (IoContext::hasCurrent()) {
+                // If we're running in a request context then we want to abort the
+                // request with the exception here.
+                IoContext::current().abort(js.exceptionToKj(kj::mv(exception)));
+                } else {
+                // If we're running outside of a request context. There's nothing to
+                // abort but we at least want to log the exception.
+                js.reportError(jsg::JsValue(exception.getHandle(js)));
+                }
+              });
               return js.v8Undefined();
             });
           }));
diff --git a/src/workerd/api/tests/error-in-error-event-test.js b/src/workerd/api/tests/error-in-error-event-test.js
new file mode 100644
index 00000000000..5522d1a61a9
--- /dev/null
+++ b/src/workerd/api/tests/error-in-error-event-test.js
@@ -0,0 +1,30 @@
+import { strictEqual } from 'assert';
+
+export const test = {
+  async test(_, env) {
+    try {
+      await env.subrequest.fetch('http://example.org');
+    } catch (e) {
+      strictEqual(e.message, 'boom (real)');
+    }
+  },
+};
+
+export default {
+  async fetch() {
+    // Throwing an error in the error event listener should not be catchable directly
+    // but should cause the IoContext to be aborted with the error.
+    addEventListener(
+      'error',
+      () => {
+        throw new Error('boom (real)');
+      },
+      { once: true }
+    );
+    queueMicrotask(() => {
+      throw new Error('boom (unused)');
+    });
+    await scheduler.wait(1000);
+    return new Response('should not see this');
+  },
+};
diff --git a/src/workerd/api/tests/error-in-error-event-test.wd-test b/src/workerd/api/tests/error-in-error-event-test.wd-test
new file mode 100644
index 00000000000..2dcb5217649
--- /dev/null
+++ b/src/workerd/api/tests/error-in-error-event-test.wd-test
@@ -0,0 +1,18 @@
+using Workerd = import "/workerd/workerd.capnp";
+
+const unitTests :Workerd.Config = (
+  services = [
+    ( name = "error-in-error-event-test",
+      worker = (
+        modules = [
+          (name = "worker", esModule = embed "error-in-error-event-test.js")
+        ],
+        compatibilityDate = "2024-09-15",
+        compatibilityFlags = ["nodejs_compat_v2"],
+        bindings = [
+          (name = "subrequest", service = "error-in-error-event-test")
+        ],
+      )
+    ),
+  ],
+);