fix(worker): hand worker_threads 'error' listeners an Error rebuilt from the thrown value

A worker_threads 'error' listener received the runtime's ErrorEvent, not an Error, and the message it carried was V8's "Uncaught Error: ..." text. The worker now reads the thrown value's name and message and forwards them with the error payload. The parent rebuilds an Error from them with the worker's stack, using the built-in constructor the name belongs to, and the parent's global error event carries that same Error when nothing handled it.

The Worker shim cancels an error its listeners took with preventDefault() rather than through the truthy-return contract of onerror.

Author: adrian-niculescu

docs/ns-builtin-modules.md

@@ -732,12 +732,13 @@ resolvers read the same table differently:
   Seven rows are public today: `ns:module`, `ns:runtime`, `ns:util`,
   `node:module`, `node:url`, `node:util`, `node:worker_threads`.
 - The **internal require** builtins receive (previous section) is the only
-  thing that can name an internal-only row. Five rows are marked that way:
+  thing that can name an internal-only row. Six rows are marked that way:
   `internal/broadcast-channel`, `internal/dom-exception`, `internal/events`,
-  `internal/message-channel`, `internal/message-event`. Their exports carry
-  capabilities app code must not hold — listener-accounting hook keys, the
-  error-reporter setter, base classes that must be the runtime's own rather
-  than whatever a global currently names.
+  `internal/message-channel`, `internal/message-event`,
+  `internal/worker-events`. Their exports carry capabilities app code must not
+  hold — listener-accounting hook keys, the error-reporter setter, the key a
+  worker's rebuilt error travels under, base classes that must be the
+  runtime's own rather than whatever a global currently names.
 - Builtins with **no row at all** (the intrinsics snapshot, the require
   factory, the console formatter) are invoked straight from their native call
   sites. There is no specifier that could reach them and nothing to mark.

docs/worker-threads.md

@@ -54,7 +54,7 @@ means deliberately unsupported.
 | `threadName` | shim | Always `undefined`. |
 | `workerData` | shim | Always `null` — see below. |
 | `parentPort` | shim | `null` on the main isolate. Inside a worker, a `MessagePort`-shaped `EventTarget` over the worker's existing parent channel: `postMessage` forwards to the global `postMessage`, `message`/`messageerror` are re-dispatched from the worker global scope, `start()` and `close()` are no-ops. It is **not** a real port: not transferable, no queue of its own. |
-| `Worker` | shim | A class over the runtime's global `Worker` with a small Node-style emitter (`on`/`once`/`off`/`removeListener`) for `message`, `messageerror`, `error`, `online` and `exit`. `postMessage(value, transfer)` and `terminate()` forward. `online` is emitted off a microtask after construction, not from the thread. Unsupported options throw a `TypeError` naming the option: `workerData`, `env`, `eval`, `transferList`, and `stdin`/`stdout`/`stderr` when explicitly truthy. The runtime's own `Worker` options (`androidPriority`) ride along untouched — the native constructor ignores keys it does not know. |
+| `Worker` | shim | A class over the runtime's global `Worker` with a small Node-style emitter (`on`/`once`/`off`/`removeListener`) for `message`, `messageerror`, `error`, `online` and `exit`. An `error` listener receives the worker's error rebuilt as an `Error` (see below). `postMessage(value, transfer)` and `terminate()` forward. `online` is emitted off a microtask after construction, not from the thread. Unsupported options throw a `TypeError` naming the option: `workerData`, `env`, `eval`, `transferList`, and `stdin`/`stdout`/`stderr` when explicitly truthy. The runtime's own `Worker` options (`androidPriority`) ride along untouched — the native constructor ignores keys it does not know. |
 | `postMessageToThread` | throws | `Error: postMessageToThread is not supported in this runtime`. |
 | `moveMessagePortToContext` | throws | `Error: moveMessagePortToContext is not supported in this runtime`. |
 | `locks` | absent | Web Locks are not implemented; the property does not exist. |
@@ -79,6 +79,22 @@ finished. `terminate()` therefore resolves with `0` and emits `exit` with code
 `0` on the way, and that is the only path that emits it. A worker that ends by
 its own `close()` produces no `exit`.
 
+### An `error` listener receives a rebuilt `Error`
+
+Node hands an `error` listener the worker's error deserialized on the parent.
+Only strings cross the isolate boundary here, so the parent rebuilds it from
+the thrown value's `name` and `message`, with the worker's stack as its
+`stack`. A built-in name (`TypeError`, `RangeError`, …) rebuilds with that
+constructor, so `instanceof` holds; any other name, a subclass's or a
+`DOMException`'s, is an own `name` on an `Error`. Other properties, such as a
+`code` or a `cause`, are not carried. A thrown value with no string `message`,
+such as a string or a number, arrives as an `Error` whose message is its string
+form, where Node hands over the value itself.
+
+Once an `error` listener has received it, the error counts as handled and is
+not reported on the parent's global scope. With no `error` listener, the
+parent's global `error` event carries the same rebuilt `Error`.
+
 ### A worker error carries no `error` object, and the worker scope's `onerror` is not an event
 
 An error the worker scope leaves unhandled reaches the parent as a real

eslint.config.mjs

@@ -32,7 +32,7 @@ const capturedStatics = [
 
 // Captured constructors. A destructure from `primordials` shadows the global,
 // so these only fire on the unguarded reference.
-const restrictedGlobals = ['Date', 'FinalizationRegistry', 'Map', 'Number', 'Promise', 'Proxy', 'RangeError', 'Set', 'String', 'TypeError', 'Uint8Array', 'Uint32Array', 'WeakRef', 'WeakSet'].map((name) => ({
+const restrictedGlobals = ['Date', 'EvalError', 'FinalizationRegistry', 'Map', 'Number', 'Promise', 'Proxy', 'RangeError', 'ReferenceError', 'Set', 'String', 'SyntaxError', 'TypeError', 'Uint8Array', 'Uint32Array', 'URIError', 'WeakRef', 'WeakSet'].map((name) => ({
   name,
   message: `Destructure ${name} from primordials — builtins must not read intrinsics off globals user code can replace.`,
 }));

test-app/app/src/main/assets/app/tests/messaging/domExceptionThrowingWorker.js

@@ -0,0 +1 @@
+throw new DOMException("aborted in a worker", "AbortError");

test-app/app/src/main/assets/app/tests/messaging/parentPortThrowingWorker.js

@@ -1,4 +1,4 @@
 var parentPort = require("node:worker_threads").parentPort;
 parentPort.on("message", function () {
-    throw new Error("thrown by a parentPort listener");
+    throw new TypeError("thrown by a parentPort listener");
 });

test-app/app/src/main/assets/app/tests/testMessaging.js

@@ -232,6 +232,7 @@ describe("Messaging runtime edges", function () {
                 expect(seen.length).toBe(1);
                 expect(seen[0].message).toContain("boom from worker");
                 expect(seen[0].error instanceof Error).toBe(true);
+                expect(seen[0].error.message).toBe("boom from worker");
                 worker.terminate();
                 done();
             };
@@ -268,14 +269,29 @@ describe("Messaging runtime edges", function () {
             worker.on("error", function (error) {
                 setTimeout(function () {
                     removeEventListener("error", listener);
-                    expect(error.message).toContain("boom from worker");
+                    expect(error instanceof Error).toBe(true);
+                    expect(error.name).toBe("Error");
+                    expect(error.message).toBe("boom from worker");
+                    expect(error.stack).toContain("boom from worker");
                     expect(globalErrors).toEqual([]);
                     worker.terminate();
                     done();
                 }, SETTLE);
             });
         });
 
+        it("hands a node:worker_threads 'error' listener a thrown DOMException's name and message", function (done) {
+            var wt = require("node:worker_threads");
+            var worker = new wt.Worker("~/tests/messaging/domExceptionThrowingWorker.js");
+            worker.on("error", function (error) {
+                expect(error instanceof Error).toBe(true);
+                expect(error.name).toBe("AbortError");
+                expect(error.message).toBe("aborted in a worker");
+                worker.terminate();
+                done();
+            });
+        });
+
         it("calls a node:worker_threads once listener once when an earlier listener emits again", function () {
             var wt = require("node:worker_threads");
             var worker = new wt.Worker("~/tests/eventLoopEchoWorker.js");
@@ -296,18 +312,20 @@ describe("Messaging runtime edges", function () {
         it("routes a throw from a parentPort listener to the parent's 'error' listeners", function (done) {
             var wt = require("node:worker_threads");
             var worker = new wt.Worker("~/tests/messaging/parentPortThrowingWorker.js");
-            var messages = [];
+            var errors = [];
             var finish = function () {
-                expect(messages.length).toBe(1);
-                expect(messages[0]).toContain("thrown by a parentPort listener");
+                expect(errors.length).toBe(1);
+                expect(errors[0] instanceof TypeError).toBe(true);
+                expect(errors[0].name).toBe("TypeError");
+                expect(errors[0].message).toBe("thrown by a parentPort listener");
                 worker.terminate();
                 done();
             };
             // Nothing else settles the spec when the error never arrives.
             var guard = setTimeout(finish, 10000);
             worker.on("error", function (error) {
-                messages.push(error.message);
-                if (messages.length === 1) {
+                errors.push(error);
+                if (errors.length === 1) {
                     clearTimeout(guard);
                     setTimeout(finish, SETTLE);
                 }

test-app/runtime/src/main/cpp/CallbackHandlers.cpp

@@ -1902,14 +1902,16 @@ void CallbackHandlers::CallWorkerScopeOnErrorHandle(Isolate *isolate, TryCatch &
         // parent sees the handler's own error, and only that one.
         if (innerTc.HasCaught()) {
             ExtractTryCatchInfo(isolate, context, innerTc, message, source, stackTrace, lineno);
-            wrapper->PassUncaughtExceptionFromWorkerToParent(message, source, stackTrace, lineno);
+            wrapper->PassUncaughtExceptionFromWorkerToParent(message, source, stackTrace, lineno,
+                                                             innerTc.Exception());
             return;
         }
 
         // Unhandled at the worker scope - including when there is no scope
         // handler at all - so it becomes the parent's error event.
         ExtractTryCatchInfo(isolate, context, tc, message, source, stackTrace, lineno);
-        wrapper->PassUncaughtExceptionFromWorkerToParent(message, source, stackTrace, lineno);
+        wrapper->PassUncaughtExceptionFromWorkerToParent(message, source, stackTrace, lineno,
+                                                         tc.Exception());
     } catch (NativeScriptException &ex) {
         ex.ReThrowToV8();
     } catch (std::exception e) {

test-app/runtime/src/main/cpp/NativeScriptException.cpp

@@ -918,7 +918,9 @@ void PromiseRejectionTracker::Drain() {
               !workerWrapper->IsTerminating() && !workerWrapper->IsDisposed()) {
             string forwarded = message;
             string forwardedStack = stackTrace;
+            Local<Value> forwardedValue = reason;
             if (!thrown.IsEmpty()) {
+              forwardedValue = thrown;
               forwarded = ToDetailString(isolate, thrown);
               // The handler's own stack replaces the reason's; an accessor
               // that throws costs the stack, never the forward.
@@ -930,7 +932,7 @@ void PromiseRejectionTracker::Drain() {
               }
             }
             workerWrapper->PassUncaughtExceptionFromWorkerToParent(
-                forwarded, "", forwardedStack, 0);
+                forwarded, "", forwardedStack, 0, forwardedValue);
           }
         }
       } else {

test-app/runtime/src/main/cpp/NsBuiltinModules.cpp

@@ -68,6 +68,7 @@ constexpr Registration kRegistry[] = {
         {"internal/events", BuiltinId::kEvents, nullptr, true},
         {"internal/message-channel", BuiltinId::kMessageChannel, messaging::CreateBinding, true},
         {"internal/message-event", BuiltinId::kMessageEvent, nullptr, true},
+        {"internal/worker-events", BuiltinId::kWorkerEvents, nullptr, true},
 };
 
 constexpr const char* kDebugKey = "debug";

test-app/runtime/src/main/cpp/WorkerEvents.cpp

@@ -98,28 +98,28 @@ void WorkerEvents::EmitMessage(Isolate* isolate, Local<Object> receiver,
     (void)state->emitMessage.Get(isolate)->Call(context, receiver, 3, args).ToLocal(&result);
 }
 
-bool WorkerEvents::EmitError(Isolate* isolate, Local<Object> receiver,
-                             const std::string& message, const std::string& source,
-                             const std::string& stackTrace, int lineNumber) {
+MaybeLocal<Value> WorkerEvents::EmitError(Isolate* isolate, Local<Object> receiver,
+                                          const std::string& message, const std::string& source,
+                                          const std::string& stackTrace, int lineNumber,
+                                          const std::string& errorName,
+                                          const std::string& errorMessage) {
     auto* state = RuntimeState::For<WorkerEventsState>(isolate);
     if (state == nullptr || state->emitError.IsEmpty()) {
-        return false;
+        return MaybeLocal<Value>();
     }
     Runtime* runtime = Runtime::TryGetRuntime(isolate);
     if (runtime == nullptr) {
-        return false;
+        return MaybeLocal<Value>();
     }
     Local<Context> context = runtime->GetContext();
 
-    Local<Value> args[4]{ArgConverter::ConvertToV8String(isolate, message),
+    Local<Value> args[6]{ArgConverter::ConvertToV8String(isolate, message),
                          ArgConverter::ConvertToV8String(isolate, source),
                          Number::New(isolate, lineNumber),
-                         ArgConverter::ConvertToV8String(isolate, stackTrace)};
-    Local<Value> result;
-    if (!state->emitError.Get(isolate)->Call(context, receiver, 4, args).ToLocal(&result)) {
-        return false;
-    }
-    return result->BooleanValue(isolate);
+                         ArgConverter::ConvertToV8String(isolate, stackTrace),
+                         ArgConverter::ConvertToV8String(isolate, errorName),
+                         ArgConverter::ConvertToV8String(isolate, errorMessage)};
+    return state->emitError.Get(isolate)->Call(context, receiver, 6, args);
 }
 
 }  // namespace tns