pass test-worker-nested-uncaught.js (#19509)

src/bun.js/bindings/ProcessIdentifier.cpp

@@ -31,23 +31,12 @@
 namespace WebCore {
 namespace Process {
 
-static std::optional<ProcessIdentifier> globalIdentifier;
-
-void setIdentifier(ProcessIdentifier processIdentifier)
-{
-    ASSERT(isUIThread());
-    globalIdentifier = processIdentifier;
-}
+// Bun only has 1 process
+static ProcessIdentifier globalIdentifier { 1 };
 
 ProcessIdentifier identifier()
 {
-    static std::once_flag onceFlag;
-    std::call_once(onceFlag, [] {
-        if (!globalIdentifier)
-            globalIdentifier = ProcessIdentifier::generate();
-    });
-
-    return *globalIdentifier;
+    return globalIdentifier;
 }
 
 } // namespace ProcessIdent

src/bun.js/bindings/ProcessIdentifier.h

@@ -34,7 +34,6 @@ using ProcessIdentifier = ObjectIdentifier<ProcessIdentifierType>;
 
 namespace Process {
 
-WEBCORE_EXPORT void setIdentifier(ProcessIdentifier);
 WEBCORE_EXPORT ProcessIdentifier identifier();
 
 } // namespace Process

src/bun.js/bindings/ZigGlobalObject.cpp

@@ -143,7 +143,6 @@
 #include "Performance.h"
 #include "ProcessBindingConstants.h"
 #include "ProcessBindingTTYWrap.h"
-#include "ProcessIdentifier.h"
 #include "ReadableStream.h"
 #include "SerializedScriptValue.h"
 #include "StructuredClone.h"

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

@@ -27,10 +27,7 @@
 
 #include "ActiveDOMObject.h"
 #include "EventTarget.h"
-// #include "MessagePort.h"
 #include "WorkerOptions.h"
-// #include "WorkerScriptLoaderClient.h"
-// #include "WorkerType.h"
 #include <JavaScriptCore/RuntimeFlags.h>
 #include <wtf/Deque.h>
 #include <wtf/MonotonicTime.h>
@@ -50,7 +47,6 @@ class RTCRtpScriptTransform;
 class RTCRtpScriptTransformer;
 class ScriptExecutionContext;
 class WorkerGlobalScopeProxy;
-// class WorkerScriptLoader;
 
 struct StructuredSerializeOptions;
 struct WorkerOptions;
@@ -80,15 +76,6 @@ public:
     void dispatchCloseEvent(Event&);
     void setKeepAlive(bool);
 
-#if ENABLE(WEB_RTC)
-    void createRTCRtpScriptTransformer(RTCRtpScriptTransform&, MessageWithMessagePorts&&);
-#endif
-
-    // WorkerType type() const
-    // {
-    //     return m_options.type;
-    // }
-
     void postTaskToWorkerGlobalScope(Function<void(ScriptExecutionContext&)>&&);
 
     static void forEachWorker(const Function<Function<void(ScriptExecutionContext&)>()>&);
@@ -97,7 +84,9 @@ public:
     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; }
@@ -111,16 +100,6 @@ private:
     void derefEventTarget() final { deref(); }
     void eventListenersDidChange() final {};
 
-    // void didReceiveResponse(ResourceLoaderIdentifier, const ResourceResponse&) final;
-    // void notifyFinished() final;
-
-    // ActiveDOMObject.
-    // void stop() final;
-    // void suspend(ReasonForSuspension) final;
-    // void resume() final;
-    // const char* activeDOMObjectName() const final;
-    // bool virtualHasPendingActivity() const final;
-
     static void networkStateChanged(bool isOnLine);
 
     static constexpr uint8_t OnlineFlag = 1 << 0;
@@ -128,15 +107,9 @@ private:
     static constexpr uint8_t TerminateRequestedFlag = 1 << 0;
     static constexpr uint8_t TerminatedFlag = 1 << 1;
 
-    // RefPtr<WorkerScriptLoader> m_scriptLoader;
     WorkerOptions m_options;
     String m_identifier;
-    // WorkerGlobalScopeProxy& m_contextProxy; // The proxy outlives the worker to perform thread shutdown.
-    // std::optional<ContentSecurityPolicyResponseHeaders> m_contentSecurityPolicyResponseHeaders;
     MonotonicTime m_workerCreationTime;
-    // bool m_shouldBypassMainWorldContentSecurityPolicy { false };
-    // bool m_isSuspendedForBackForwardCache { false };
-    // JSC::RuntimeFlags m_runtimeFlags;
     Deque<RefPtr<Event>> m_pendingEvents;
     Lock m_pendingTasksMutex;
     Deque<Function<void(ScriptExecutionContext&)>> m_pendingTasks;

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/bun.js/webcore.zig

@@ -14,7 +14,7 @@ pub const ByteListPool = bun.ObjectPool(bun.ByteList, null, true, 8);
 
 pub const Crypto = @import("webcore/Crypto.zig");
 pub const AbortSignal = @import("bindings/AbortSignal.zig").AbortSignal;
-pub const WebWorker = @import("web_worker.zig").WebWorker;
+pub const WebWorker = @import("web_worker.zig");
 pub const AutoFlusher = @import("webcore/AutoFlusher.zig");
 pub const EncodingLabel = @import("webcore/EncodingLabel.zig").EncodingLabel;
 pub const Fetch = @import("webcore/fetch.zig");

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

@@ -399,3 +399,25 @@ describe("environmentData", () => {
     expect(proc.exitCode).toBe(0);
   });
 });
+
+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