Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 36 additions & 5 deletions src/jsc/bindings/webcore/WorkerMessagingProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "SerializedScriptValue.h"
#include "Worker.h"
#include "ZigGlobalObject.h"
#include <JavaScriptCore/Error.h>
#include <JavaScriptCore/JSPromise.h>
#include <wtf/TZoneMallocInlines.h>

Expand Down Expand Up @@ -445,15 +446,40 @@ void WorkerMessagingProxy::postMessageToWorkerObject(MessageWithMessagePorts&& m
}
}

void WorkerMessagingProxy::postMessageErrorToWorkerObject(String&& message)
void WorkerMessagingProxy::postMessageErrorToWorkerObject(String&& message, RefPtr<SerializedScriptValue>&& errorForDefaultReport)
{
ScriptExecutionContext::postTaskTo(m_loaderContextIdentifier, m_loaderLoopKind, [protectedThis = Ref { *this }, message = WTF::move(message).isolatedCopy()](ScriptExecutionContext&) {
ScriptExecutionContext::postTaskTo(m_loaderContextIdentifier, m_loaderLoopKind, [protectedThis = Ref { *this }, message = WTF::move(message).isolatedCopy(), errorForDefaultReport = WTF::move(errorForDefaultReport)](ScriptExecutionContext& context) {
RefPtr workerObject = protectedThis->m_workerObject;
if (!workerObject)
return;
bool hadListener = workerObject->hasEventListeners(eventNames().errorEvent);
ErrorEvent::Init init;
init.message = message;
init.cancelable = true;
workerObject->dispatchEvent(ErrorEvent::create(eventNames().errorEvent, init, EventIsTrusted::Yes));
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Default action for a Web Worker error nobody listened for: report it as an uncaught error
// in this (the parent) context, so it prints and sets the exit code, and a parent that is
// itself a worker repeats this one level up (https://html.spec.whatwg.org/multipage/workers.html#runtime-script-errors-2).
// node:worker_threads always has a listener (its JS wrapper re-emits on the EventEmitter).
Comment thread
robobun marked this conversation as resolved.
if (protectedThis->m_options.kind != WorkerOptions::Kind::Web || hadListener || workerObject->wasTerminated())
return;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
auto* globalObject = context.globalObject();
auto& vm = JSC::getVM(globalObject);
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
JSC::JSValue reported;
if (errorForDefaultReport) {
reported = errorForDefaultReport->deserialize(*globalObject, globalObject, SerializationErrorMode::NonThrowing);
if (scope.exception()) [[unlikely]] {
if (!scope.clearExceptionExceptTermination())
return;
reported = {};
}
}
if (!reported)
reported = JSC::createError(globalObject, message);
Bun__reportUnhandledError(globalObject, JSC::JSValue::encode(reported));
CLEAR_IF_EXCEPTION(scope);
});
}

Expand Down Expand Up @@ -501,12 +527,17 @@ bool WorkerMessagingProxy::postSerializedErrorToWorkerObject(Zig::GlobalObject&
void WorkerMessagingProxy::postErrorToWorkerObject(Zig::GlobalObject& workerGlobalObject, const String& message, JSC::JSValue error)
{
switch (m_options.kind) {
case WorkerOptions::Kind::Web:
postMessageErrorToWorkerObject(String { message });
case WorkerOptions::Kind::Web: {
auto& vm = JSC::getVM(&workerGlobalObject);
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
auto serialized = SerializedScriptValue::create(workerGlobalObject, error, SerializationForStorage::No, SerializationErrorMode::NonThrowing);
CLEAR_IF_EXCEPTION(scope);
postMessageErrorToWorkerObject(String { message }, WTF::move(serialized));
return;
}
case WorkerOptions::Kind::Node:
if (!postSerializedErrorToWorkerObject(workerGlobalObject, error))
postMessageErrorToWorkerObject(String { message });
postMessageErrorToWorkerObject(String { message }, nullptr);
return;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/jsc/bindings/webcore/WorkerMessagingProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ class WorkerMessagingProxy final : public ThreadSafeRefCounted<WorkerMessagingPr
void releaseWorkerThread();
void drainMessagesToWorkerObject(ScriptExecutionContext&, DrainBudget);
void rejectAllCrossVMRequests();
void postMessageErrorToWorkerObject(String&& message);
void postMessageErrorToWorkerObject(String&& message, RefPtr<SerializedScriptValue>&& errorForDefaultReport);
bool postSerializedErrorToWorkerObject(Zig::GlobalObject&, JSC::JSValue error);

// Parent thread only.
Expand Down
159 changes: 159 additions & 0 deletions test/js/web/workers/worker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,165 @@ describe("web worker", () => {
const [err] = await once(worker, "error");
expect(err.message).toBe(`BuildMessage: ModuleNotFound resolving "${specifier}" (entry point)`);
});

// With no 'error' listener on the Worker object the error is reported as uncaught in the parent
// (printed, exit code 1) instead of vanishing. Each body is a different producer inside the worker.
for (const [name, body] of [
["throw at evaluation", `throw new Error("WEB-WORKER-UNCAUGHT")`],
["rejection at evaluation", `Promise.reject(new Error("WEB-WORKER-UNCAUGHT"))`],
[
"throw in a timer",
`setInterval(() => {}, 1000); setTimeout(() => { throw new Error("WEB-WORKER-UNCAUGHT") }, 1)`,
],
[
"rejection in a timer",
`setInterval(() => {}, 1000); setTimeout(() => { Promise.reject(new Error("WEB-WORKER-UNCAUGHT")) }, 1)`,
],
]) {
test.concurrent(`with no listener is reported to the parent (${name})`, async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", `new Worker("data:text/javascript," + encodeURIComponent(${JSON.stringify(body)}));`],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("error: WEB-WORKER-UNCAUGHT");
expect(exitCode).toBe(1);
});
}

test.concurrent("with no listener is caught by the parent's process.on('uncaughtException')", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`process.on("uncaughtException", err => console.log("caught", err.constructor.name, err.message));
const w = new Worker("data:text/javascript," + encodeURIComponent('throw new TypeError("WEB-WORKER-UNCAUGHT")'));
w.addEventListener("close", e => console.log("worker closed", e.code));`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).not.toContain("WEB-WORKER-UNCAUGHT");
expect(stdout).toContain("caught TypeError WEB-WORKER-UNCAUGHT");
expect(stdout).toContain("worker closed 1");
if (exitCode !== 0) expect(stderr).toBe("");
expect(exitCode).toBe(0);
});

// A worker whose own nested worker dies with no listener sees that as its own uncaught error, so
// the report repeats one level up until something listens or the main thread prints it.
test.concurrent("with no listener on a nested worker propagates to that worker's parent", async () => {
using dir = tempDir("worker-nested-error", {
"inner.js": `setTimeout(() => { throw new Error("WEB-WORKER-NESTED-UNCAUGHT") }, 1);`,
"middle.js": `new Worker(new URL("inner.js", import.meta.url).href);`,
"main-no-listener.js": `new Worker(new URL("middle.js", import.meta.url).href);`,
"main-listener.js": `const w = new Worker(new URL("middle.js", import.meta.url).href);
w.addEventListener("error", e => console.log("main saw", e.message.includes("WEB-WORKER-NESTED-UNCAUGHT")));
w.addEventListener("close", e => console.log("middle closed", e.code));`,
});
const run = async (entry: string) => {
await using proc = Bun.spawn({
cmd: [bunExe(), entry],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { stdout, stderr, exitCode };
};
// Nobody listens anywhere: the main thread prints it and exits 1.
{
const { stderr, exitCode } = await run("main-no-listener.js");
expect(stderr).toContain("error: WEB-WORKER-NESTED-UNCAUGHT");
expect(exitCode).toBe(1);
}
// The main thread listens on the middle worker: it receives inner's error as middle's, and
// middle itself exits 1 because the error was uncaught there.
{
const { stdout, stderr, exitCode } = await run("main-listener.js");
expect(stderr).not.toContain("WEB-WORKER-NESTED-UNCAUGHT");
expect(stdout).toContain("main saw true");
expect(stdout).toContain("middle closed 1");
if (exitCode !== 0) expect(stderr).toBe("");
expect(exitCode).toBe(0);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
});

test.concurrent("with a listener present is not reported as unhandled", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`process.on("uncaughtException", err => { console.log("caught"); process.exitCode = 2; });
const w = new Worker("data:text/javascript," + encodeURIComponent('throw new Error("WEB-WORKER-UNCAUGHT")'));
w.addEventListener("error", e => console.log("listener saw", e.message.includes("WEB-WORKER-UNCAUGHT"), "cancelable", e.cancelable));
w.addEventListener("close", e => console.log("worker closed", e.code));`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).not.toContain("WEB-WORKER-UNCAUGHT");
expect(stdout).toContain("listener saw true cancelable true");
expect(stdout).not.toContain("caught");
expect(stdout).toContain("worker closed 1");
if (exitCode !== 0) expect(stderr).toBe("");
expect(exitCode).toBe(0);
});

test.concurrent("with no listener is not reported after terminate()", async () => {
// Whether each in-flight error task lands before or after terminate(), it must never surface
// as an unhandled exception on the parent.
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`process.on("uncaughtException", err => { console.error("UNHANDLED", err.message); process.exitCode = 2; });
const src = "data:text/javascript," + encodeURIComponent('throw new Error("POST-TERMINATE")');
const N = 32;
let closed = 0;
const { promise, resolve } = Promise.withResolvers();
for (let i = 0; i < N; i++) {
const w = new Worker(src);
w.addEventListener("close", () => { if (++closed === N) resolve(); });
w.terminate();
}
await promise;
console.log("done");`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).not.toContain("POST-TERMINATE");
expect(stdout).toContain("done");
if (exitCode !== 0) expect(stderr).toBe("");
expect(exitCode).toBe(0);
});

test.concurrent("node:worker_threads with no listener prints and exits 1 (control)", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const { Worker } = require("node:worker_threads");
new Worker('throw new Error("NODE-WORKER-UNCAUGHT")', { eval: true });`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("NODE-WORKER-UNCAUGHT");
expect(exitCode).toBe(1);
});
});

// As in browsers (and Node's Web Worker), the worker's implicit port opens once the entry's
Expand Down