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..b04b9c9b142 100644 --- a/src/workerd/api/global-scope.c++ +++ b/src/workerd/api/global-scope.c++ @@ -706,7 +706,28 @@ void ServiceWorkerGlobalScope::queueMicrotask(jsg::Lock& js, v8::LocalCall(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) { + // An error was thrown by the 'error' event handler. That's unfortunate. + // Let's log the error and just continue. It won't be possible to actually + // catch or handle this error so logging is really the only way to notify + // folks about it. + auto val = jsg::JsValue(exception.getHandle(js)); + // If the value is an object that has a stack propery, log that so we get + // the stack trace if it is an exception. + KJ_IF_SOME(obj, val.tryCast()) { + auto stack = obj.get(js, "stack"_kj); + if (!stack.isUndefined()) { + js.reportError(stack); + return; + } + } else { + } // Here to avoid a compile warning + // Otherwise just log the stringified value generically. + js.reportError(val); + }); return js.v8Undefined(); }); })); @@ -798,6 +819,16 @@ void ServiceWorkerGlobalScope::reportError(jsg::Lock& js, jsg::JsValue error) { .colno = jsg::check(message->GetStartColumn(js.v8Context())), .error = jsg::JsRef(js, error)}); if (dispatchEventImpl(js, kj::mv(event))) { + // If the value is an object that has a stack propery, log that so we get + // the stack trace if it is an exception. + KJ_IF_SOME(obj, error.tryCast()) { + auto stack = obj.get(js, "stack"_kj); + if (!stack.isUndefined()) { + js.reportError(stack); + return; + } + } + // Otherwise just log the stringified value generically. js.reportError(error); } } 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..95f6e278c35 --- /dev/null +++ b/src/workerd/api/tests/error-in-error-event-test.js @@ -0,0 +1,31 @@ +import { strictEqual } from 'assert'; + +export const test = { + async test(_, env) { + const resp = await env.subrequest.fetch('http://example.org'); + strictEqual(await resp.text(), 'ok'); + }, +}; + +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', + () => { + // This error is not going to be catachable. The best we can do is to log it. + // Unfortunately, workerd currently does not give us any mechanism to verify + // that it was logged in the test. Let's make sure the response is handled + // correctly at least. + throw new Error('boom (real)'); + }, + { once: true } + ); + queueMicrotask(() => { + throw new Error('boom (unused)'); + }); + await scheduler.wait(10); + return new Response('ok'); + }, +}; 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") + ], + ) + ), + ], +); diff --git a/src/workerd/jsg/jsg.h b/src/workerd/jsg/jsg.h index 9436f88ea34..5c9b2d5bb0a 100644 --- a/src/workerd/jsg/jsg.h +++ b/src/workerd/jsg/jsg.h @@ -2639,6 +2639,8 @@ class Lock { void runMicrotasks(); void terminateExecution(); + // Logs and reports the error to tail workers (if called within an request), + // the inspector (if attached), or to KJ_LOG(Info). virtual void reportError(const JsValue& value) = 0; private: