pass test-worker-nested-uncaught.js

Author: Ben Grant

src/bun.js/bindings/webcore/JSWorker.cpp

@@ -138,13 +138,13 @@ template<> JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES JSWorkerDOMConstructor::
     }
     RETURN_IF_EXCEPTION(throwScope, {});
     EnsureStillAliveScope argument1 = callFrame->argument(1);
+
+    WorkerOptions options {};
     JSValue nodeWorkerObject {};
     if (callFrame->argumentCount() == 3) {
         nodeWorkerObject = callFrame->argument(2);
+        options.kind = WorkerOptions::Kind::Node;
     }
-    RETURN_IF_EXCEPTION(throwScope, {});
-
-    auto options = WorkerOptions {};
     JSValue workerData = jsUndefined();
     Vector<JSC::Strong<JSC::JSObject>> transferList;
 

src/bun.js/bindings/webcore/Worker.cpp

@@ -389,12 +389,10 @@ void Worker::fireEarlyMessages(Zig::GlobalObject* workerGlobalObject)
     }
 }
 
-void Worker::dispatchError(WTF::String message)
+void Worker::dispatchErrorWithMessage(WTF::String message)
 {
-
     auto* ctx = scriptExecutionContext();
-    if (!ctx)
-        return;
+    if (!ctx) return;
 
     ScriptExecutionContext::postTaskTo(ctx->identifier(), [protectedThis = Ref { *this }, message = message.isolatedCopy()](ScriptExecutionContext& context) -> void {
         ErrorEvent::Init init;
@@ -404,6 +402,27 @@ void Worker::dispatchError(WTF::String message)
         protectedThis->dispatchEvent(event);
     });
 }
+
+bool Worker::dispatchErrorWithValue(Zig::GlobalObject* workerGlobalObject, JSValue value)
+{
+    auto* ctx = scriptExecutionContext();
+    if (!ctx) return false;
+    auto serialized = SerializedScriptValue::create(*workerGlobalObject, value, SerializationForStorage::No, SerializationErrorMode::NonThrowing);
+    if (!serialized) return false;
+
+    ScriptExecutionContext::postTaskTo(ctx->identifier(), [protectedThis = Ref { *this }, serialized](ScriptExecutionContext& context) -> void {
+        auto* globalObject = context.globalObject();
+        ErrorEvent::Init init;
+        JSValue deserialized = serialized->deserialize(*globalObject, globalObject, SerializationErrorMode::NonThrowing);
+        if (!deserialized) return;
+        init.error = deserialized;
+
+        auto event = ErrorEvent::create(eventNames().errorEvent, init, EventIsTrusted::Yes);
+        protectedThis->dispatchEvent(event);
+    });
+    return true;
+}
+
 void Worker::dispatchExit(int32_t exitCode)
 {
     auto* ctx = scriptExecutionContext();
@@ -483,7 +502,16 @@ extern "C" void WebWorker__dispatchError(Zig::GlobalObject* globalObject, Worker
     init.bubbles = false;
 
     globalObject->globalEventScope->dispatchEvent(ErrorEvent::create(eventNames().errorEvent, init, EventIsTrusted::Yes));
-    worker->dispatchError(message.toWTFString(BunString::ZeroCopy));
+    switch (worker->options().kind) {
+    case WorkerOptions::Kind::Web:
+        return worker->dispatchErrorWithMessage(message.toWTFString(BunString::ZeroCopy));
+    case WorkerOptions::Kind::Node:
+        if (!worker->dispatchErrorWithValue(globalObject, error)) {
+            // If serialization threw an error, use the string instead
+            worker->dispatchErrorWithMessage(message.toWTFString(BunString::ZeroCopy));
+        }
+        return;
+    }
 }
 
 extern "C" WebCore::Worker* WebWorker__getParentWorker(void* bunVM);

src/bun.js/bindings/webcore/Worker.h

@@ -84,7 +84,9 @@ class Worker final : public ThreadSafeRefCounted<Worker>, public EventTargetWith
     void dispatchOnline(Zig::GlobalObject* workerGlobalObject);
     // Fire a 'message' event in the Worker for messages that were sent before the Worker started running
     void fireEarlyMessages(Zig::GlobalObject* workerGlobalObject);
-    void dispatchError(WTF::String message);
+    void dispatchErrorWithMessage(WTF::String message);
+    // true if successful
+    bool dispatchErrorWithValue(Zig::GlobalObject* workerGlobalObject, JSValue value);
     void dispatchExit(int32_t exitCode);
     ScriptExecutionContext* scriptExecutionContext() const final { return ContextDestructionObserver::scriptExecutionContext(); }
     ScriptExecutionContextIdentifier clientIdentifier() const { return m_clientIdentifier; }

src/bun.js/bindings/webcore/WorkerOptions.h

@@ -8,6 +8,13 @@
 namespace WebCore {
 
 struct WorkerOptions {
+    enum class Kind : uint8_t {
+        // Created by the global Worker constructor
+        Web,
+        // Created by the `require("node:worker_threads").Worker` constructor
+        Node,
+    };
+
     String name;
     bool mini { false };
     bool unref { false };
@@ -16,6 +23,7 @@ struct WorkerOptions {
     // true, then we need to make sure that `process.argv` contains "[worker eval]" instead of the
     // Blob URL.
     bool evalMode { false };
+    Kind kind { Kind::Web };
     // Serialized array containing [workerData, environmentData]
     // (environmentData is always a Map)
     RefPtr<SerializedScriptValue> workerDataAndEnvironmentData;

src/js/node/worker_threads.ts

@@ -362,7 +362,9 @@ class Worker extends EventEmitter {
   #onError(event: ErrorEvent) {
     this.#isRunning = false;
     let error = event?.error;
-    if (!error) {
+    // if the thrown value serialized successfully, the message will be empty
+    // if not the message is the actual error
+    if (event.message !== "") {
       error = new Error(event.message, { cause: event });
       const stack = event?.stack;
       if (stack) {

test/js/node/test/parallel/test-worker-nested-uncaught.js

@@ -0,0 +1,14 @@
+'use strict';
+const common = require('../common');
+const { Worker } = require('worker_threads');
+
+// Regression test for https://github.com/nodejs/node/issues/34309
+
+const w = new Worker(
+  `const { Worker } = require('worker_threads');
+  new Worker("throw new Error('uncaught')", { eval:true })`,
+  { eval: true });
+w.on('error', common.expectsError({
+  name: 'Error',
+  message: 'uncaught'
+}));

test/js/node/worker_threads/worker_threads.test.ts

@@ -1,4 +1,5 @@
 import { bunEnv, bunExe } from "harness";
+import { once } from "node:events";
 import fs from "node:fs";
 import { join, relative, resolve } from "node:path";
 import wt, {
@@ -305,3 +306,25 @@ test("worker event", () => {
   expect(called).toBeFalse();
   return promise;
 });
+
+describe("error event", () => {
+  test("is fired with a copy of the error value", async () => {
+    const worker = new Worker("throw new TypeError('oh no')", { eval: true });
+    const [err] = await once(worker, "error");
+    expect(err).toBeInstanceOf(TypeError);
+    expect(err.message).toBe("oh no");
+  });
+
+  test("falls back to string when the error cannot be serialized", async () => {
+    const worker = new Worker(
+      /* js */ `
+      import { MessageChannel } from "node:worker_threads";
+      const { port1 } = new MessageChannel();
+      throw port1;`,
+      { eval: true },
+    );
+    const [err] = await once(worker, "error");
+    expect(err).toBeInstanceOf(Error);
+    expect(err.message).toMatch(/MessagePort \{.*\}/s);
+  });
+});

test/js/web/workers/worker.test.ts

@@ -286,6 +286,16 @@ describe("web worker", () => {
       return promise;
     });
   });
+
+  describe("error event", () => {
+    test("is fired with a string of the error", async () => {
+      const worker = new Worker("data:text/javascript,throw 5");
+      const [err] = await once(worker, "error");
+      expect(err.type).toBe("error");
+      expect(err.message).toBe("5");
+      expect(err.error).toBe(null);
+    });
+  });
 });
 
 // TODO: move to node:worker_threads tests directory