diff --git a/src/bun.js/bindings/webcore/JSEventListener.cpp b/src/bun.js/bindings/webcore/JSEventListener.cpp index fe36e1dfef..b748a5ca80 100644 --- a/src/bun.js/bindings/webcore/JSEventListener.cpp +++ b/src/bun.js/bindings/webcore/JSEventListener.cpp @@ -206,6 +206,9 @@ void JSEventListener::handleEvent(ScriptExecutionContext& scriptExecutionContext if (scope.exception()) [[unlikely]] { auto* exception = scope.exception(); scope.clearException(); + // Don't report termination exceptions + if (vm.isTerminationException(exception)) + return; event.target()->uncaughtExceptionInEventHandler(); reportException(lexicalGlobalObject, exception); return; @@ -233,14 +236,15 @@ void JSEventListener::handleEvent(ScriptExecutionContext& scriptExecutionContext // InspectorInstrumentation::didCallFunction(&scriptExecutionContext); auto handleExceptionIfNeeded = [&](JSC::Exception* exception) -> bool { - // if (is(scriptExecutionContext)) { - // auto* scriptController = downcast(scriptExecutionContext).script(); - // bool terminatorCausedException = (exception && vm.isTerminationException(exception)); - // if (terminatorCausedException || (scriptController && scriptController->isTerminatingExecution())) - // scriptController->forbidExecution(); - // } - if (exception) { + // Check if this is a termination exception (e.g., from process.exit() or worker shutdown) + // Termination exceptions should not be reported as they are expected + bool isTermination = vm.isTerminationException(exception); + if (isTermination) { + // For termination exceptions, just return without reporting + return true; + } + event.target()->uncaughtExceptionInEventHandler(); reportException(lexicalGlobalObject, exception); return true; @@ -253,11 +257,16 @@ void JSEventListener::handleEvent(ScriptExecutionContext& scriptExecutionContext // Node handles promises in the return value and throws an uncaught exception on nextTick if it rejects. // See event_target.js function addCatch in node - if (retval.isObject()) { + // Skip promise handling for workers to avoid VM entry scope issues when async errors occur (fixes #20911) + // Workers handle unhandled rejections through their own error event mechanism + if (retval.isObject() && !scriptExecutionContext.isWorkerGlobalScope()) { auto then = retval.get(lexicalGlobalObject, vm.propertyNames->then); if (scope.exception()) [[unlikely]] { auto* exception = scope.exception(); scope.clearException(); + // Don't report termination exceptions + if (vm.isTerminationException(exception)) + return; event.target()->uncaughtExceptionInEventHandler(); reportException(lexicalGlobalObject, exception); return; @@ -267,9 +276,18 @@ void JSEventListener::handleEvent(ScriptExecutionContext& scriptExecutionContext arglist.append(JSValue(JSC::jsUndefined())); arglist.append(JSValue(JSC::JSFunction::create(vm, lexicalGlobalObject, 1, String(), jsFunctionEmitUncaughtExceptionNextTick, ImplementationVisibility::Public, NoIntrinsic))); // err => process.nextTick(() => throw err) JSC::call(lexicalGlobalObject, then, retval, arglist, "Promise.then is not callable"_s); + + // Check if VM termination was requested during the call + if (vm.hasTerminationRequest()) { + return; + } + if (scope.exception()) [[unlikely]] { auto* exception = scope.exception(); scope.clearException(); + // Don't report termination exceptions + if (vm.isTerminationException(exception)) + return; event.target()->uncaughtExceptionInEventHandler(); reportException(lexicalGlobalObject, exception); return; diff --git a/test/regression/issue/20911.test.ts b/test/regression/issue/20911.test.ts new file mode 100644 index 0000000000..80098a298d --- /dev/null +++ b/test/regression/issue/20911.test.ts @@ -0,0 +1,66 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +// https://github.com/oven-sh/bun/issues/20911 +test("Worker async onmessage error should not crash process", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` +const blob = new Blob( + [ + \` + self.onmessage = async () => { + throw new Error('pong') + } + \`, + ], + { + type: 'application/typescript', + }, +) +const url = URL.createObjectURL(blob) +const worker = new Worker(url) +worker.onerror = (error) => console.error(error) +worker.postMessage('ping') + +// keep alive +setInterval(() => {}, 1000) +`, + ], + env: bunEnv, + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }); + + // Race: either the process exits (crashes) or we see the ErrorEvent output + const errorEventPromise = (async () => { + const stderr = await proc.stderr.text(); + return stderr.includes("ErrorEvent"); + })(); + + const result = await Promise.race([ + proc.exited.then(code => ({ type: "exited" as const, code })), + errorEventPromise.then(() => ({ type: "errorEvent" as const })), + Bun.sleep(2000).then(() => ({ type: "timeout" as const })), + ]); + + // If process exited before we saw ErrorEvent, it crashed + if (result.type === "exited") { + expect(result.code).not.toBe(134); // 134 = SIGABRT (abort/crash) + expect(result.code).not.toBe(139); // 139 = SIGSEGV (segfault) + } + + // Terminate the process if it's still running + proc.kill(); + const exitCode = await proc.exited; + + // Process should exit cleanly when killed (SIGTERM or SIGKILL), not crash with SIGABRT + expect(exitCode).not.toBe(134); // 134 = SIGABRT (abort/crash) + expect(exitCode).not.toBe(139); // 139 = SIGSEGV (segfault) +}); + +// TODO: Sync handler errors also crash workers, but that's a separate issue +// test("Worker sync onmessage error should work as before", async () => { ... });