Skip to content

fix(worker_threads): preserve cloned error metadata #42600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
steipete wants to merge 1 commit into
base: ciro/worker-threads-node-tests
Choose a base branch
from
Open

fix(worker_threads): preserve cloned error metadata #42600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
145 changes: 144 additions & 1 deletion src/jsc/bindings/webcore/WorkerMessagingProxy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
#include "Worker.h"
#include "ZigGlobalObject.h"
#include <JavaScriptCore/JSPromise.h>
#include <JavaScriptCore/PropertyNameArray.h>
#include <wtf/HashSet.h>
#include <wtf/ThreadSafeRefCounted.h>
#include <wtf/TZoneMallocInlines.h>

namespace WebCore {
Expand Down Expand Up @@ -510,6 +513,133 @@ static String errorCodeOf(JSC::JSGlobalObject& globalObject, JSC::JSValue value)
return code;
}

class SerializedWorkerErrorMetadata final : public ThreadSafeRefCounted<SerializedWorkerErrorMetadata> {
public:
static Ref<SerializedWorkerErrorMetadata> create() { return adoptRef(*new SerializedWorkerErrorMetadata); }

RefPtr<SerializedScriptValue> properties;
RefPtr<SerializedScriptValue> cause;
RefPtr<SerializedWorkerErrorMetadata> causeMetadata;
bool causeEnumerable { false };

private:
SerializedWorkerErrorMetadata() = default;
};

static constexpr unsigned maxWorkerErrorMetadataDepth = 32;

static RefPtr<SerializedWorkerErrorMetadata> serializeWorkerErrorMetadata(Zig::GlobalObject& globalObject, JSC::ErrorInstance& error, HashSet<JSC::JSObject*>& seen, bool includeCode, unsigned depth)
{
if (!seen.add(&error).isNewEntry)
return nullptr;

auto& vm = JSC::getVM(&globalObject);
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
auto metadata = SerializedWorkerErrorMetadata::create();

auto* propertiesObject = JSC::constructEmptyObject(&globalObject);
JSC::Strong<JSC::JSObject> protectedProperties(vm, propertiesObject);
JSC::PropertyNameArrayBuilder properties(vm, JSC::PropertyNameMode::Strings, JSC::PrivateSymbolMode::Exclude);
error.methodTable()->getOwnPropertyNames(&error, &globalObject, properties, JSC::DontEnumPropertiesMode::Exclude);
if (!scope.exception()) {
for (const auto& property : properties) {
if (property == vm.propertyNames->cause || (!includeCode && property == WebCore::builtinNames(vm).codePublicName()))
continue;
JSC::JSValue propertyValue = error.get(&globalObject, property);
if (scope.exception()) {
scope.clearException();
continue;
}
if (!propertyValue.isCallable() && !propertyValue.isSymbol()) {
propertiesObject->putDirectMayBeIndex(&globalObject, property, propertyValue);
if (scope.exception())
scope.clearException();
}
}
metadata->properties = SerializedScriptValue::create(globalObject, propertiesObject, SerializationForStorage::No, SerializationErrorMode::NonThrowing);
}
CLEAR_IF_EXCEPTION(scope);

JSC::PropertyDescriptor causeDescriptor;
bool hasCause = error.getOwnPropertyDescriptor(&globalObject, vm.propertyNames->cause, causeDescriptor);
if (scope.exception()) {
scope.clearException();
return metadata;
}
if (!hasCause)
return metadata;

JSC::JSValue cause = error.get(&globalObject, vm.propertyNames->cause);
if (scope.exception()) {
scope.clearException();
return metadata;
}
if (cause.isCallable() || cause.isSymbol())
return metadata;

metadata->cause = SerializedScriptValue::create(globalObject, cause, SerializationForStorage::No, SerializationErrorMode::NonThrowing);
CLEAR_IF_EXCEPTION(scope);
if (!metadata->cause)
return metadata;
metadata->causeEnumerable = causeDescriptor.enumerable();
if (depth + 1 < maxWorkerErrorMetadataDepth) {
if (auto* causeError = dynamicDowncast<JSC::ErrorInstance>(cause))
metadata->causeMetadata = serializeWorkerErrorMetadata(globalObject, *causeError, seen, true, depth + 1);
}
return metadata;
}

static void applyWorkerErrorMetadata(JSC::JSGlobalObject& globalObject, JSC::JSObject& error, const SerializedWorkerErrorMetadata& metadata)
{
auto& vm = JSC::getVM(&globalObject);
auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);

if (metadata.properties) {
JSC::JSValue propertiesValue = metadata.properties->deserialize(globalObject, &globalObject, SerializationErrorMode::NonThrowing);
if (scope.exception()) {
scope.clearException();
} else if (auto* properties = propertiesValue.getObject()) {
JSC::PropertyNameArrayBuilder names(vm, JSC::PropertyNameMode::Strings, JSC::PrivateSymbolMode::Exclude);
properties->methodTable()->getOwnPropertyNames(properties, &globalObject, names, JSC::DontEnumPropertiesMode::Exclude);
if (!scope.exception()) {
for (const auto& name : names) {
JSC::JSValue propertyValue = properties->get(&globalObject, name);
if (scope.exception()) {
scope.clearException();
continue;
}
error.putDirectMayBeIndex(&globalObject, name, propertyValue);
if (scope.exception())
scope.clearException();
}
}
CLEAR_IF_EXCEPTION(scope);
}
}

if (!metadata.cause)
return;
JSC::JSValue cause = metadata.cause->deserialize(globalObject, &globalObject, SerializationErrorMode::NonThrowing);
if (scope.exception()) {
scope.clearException();
return;
}
JSC::PropertyDescriptor descriptor;
descriptor.setValue(cause);
descriptor.setWritable(true);
descriptor.setEnumerable(metadata.causeEnumerable);
descriptor.setConfigurable(true);
error.methodTable()->defineOwnProperty(&error, &globalObject, vm.propertyNames->cause, descriptor, false);
if (scope.exception()) {
scope.clearException();
return;
}
if (metadata.causeMetadata) {
if (auto* causeError = dynamicDowncast<JSC::ErrorInstance>(cause))
applyWorkerErrorMetadata(globalObject, *causeError, *metadata.causeMetadata);
}
}

bool WorkerMessagingProxy::postSerializedErrorToWorkerObject(Zig::GlobalObject& workerGlobalObject, JSC::JSValue value)
{
// Top of the worker's error-dispatch stack: neither the structured clone (which can run script
Expand Down Expand Up @@ -537,7 +667,16 @@ bool WorkerMessagingProxy::postSerializedErrorToWorkerObject(Zig::GlobalObject&
// preserves a string `error.code` (lib/internal/error_serdes.js).
String errorCode = errorCodeOf(workerGlobalObject, value);

return ScriptExecutionContext::postTaskTo(m_loaderContextIdentifier, m_loaderLoopKind, [protectedThis = Ref { *this }, serialized = serialized.releaseNonNull(), errorCode = WTF::move(errorCode).isolatedCopy()](ScriptExecutionContext& context) {
// Node's worker error serializer preserves an Error's own enumerable metadata and recursively
// serializes `cause`. The ordinary structured-clone Error path intentionally keeps only
// standard Error fields, so carry these worker-only additions separately.
RefPtr<SerializedWorkerErrorMetadata> serializedMetadata;
if (auto* errorInstance = dynamicDowncast<JSC::ErrorInstance>(value)) {
HashSet<JSC::JSObject*> seen;
serializedMetadata = serializeWorkerErrorMetadata(workerGlobalObject, *errorInstance, seen, false, 0);
}

return ScriptExecutionContext::postTaskTo(m_loaderContextIdentifier, m_loaderLoopKind, [protectedThis = Ref { *this }, serialized = serialized.releaseNonNull(), serializedMetadata = WTF::move(serializedMetadata), errorCode = WTF::move(errorCode).isolatedCopy()](ScriptExecutionContext& context) {
RefPtr workerObject = protectedThis->m_workerObject;
if (!workerObject)
return;
Expand All @@ -550,6 +689,10 @@ bool WorkerMessagingProxy::postSerializedErrorToWorkerObject(Zig::GlobalObject&
if (auto* errorObject = deserialized.getObject())
errorObject->putDirect(vm, WebCore::builtinNames(vm).codePublicName(), JSC::jsString(vm, errorCode));
}
if (serializedMetadata) {
if (auto* errorObject = deserialized.getObject())
applyWorkerErrorMetadata(*globalObject, *errorObject, *serializedMetadata);
}
ErrorEvent::Init init;
init.error = deserialized;
workerObject->dispatchEvent(ErrorEvent::create(eventNames().errorEvent, init, EventIsTrusted::Yes));
Expand Down
76 changes: 76 additions & 0 deletions test/js/node/worker_threads/worker_threads.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,82 @@ describe("error event", () => {
expect(err.message).toBe("oh no");
});

test("preserves Error cause and enumerable metadata", async () => {
const worker = new Worker(
`const cause = new RangeError("root cause", { cause: new URIError("deep cause") });
cause.code = "E_CAUSE";
cause.details = { stage: "inner" };
const error = new TypeError("outer", { cause });
error.code = "E_WORKER";
error.details = { retryable: true, attempts: 2 };
error.requestId = 42;
error[0] = "numeric metadata";
throw error;`,
{ eval: true },
);
const [err] = await once(worker, "error");
expect(err).toBeInstanceOf(TypeError);
expect(err.message).toBe("outer");
expect(err.code).toBe("E_WORKER");
expect(err.cause).toBeInstanceOf(RangeError);
expect(err.cause.message).toBe("root cause");
expect(err.cause.code).toBe("E_CAUSE");
expect(err.cause.details).toEqual({ stage: "inner" });
expect(err.cause.cause).toBeInstanceOf(URIError);
expect(err.cause.cause.message).toBe("deep cause");
expect(err.details).toEqual({ retryable: true, attempts: 2 });
expect(err.requestId).toBe(42);
expect(err[0]).toBe("numeric metadata");
expect(Object.getOwnPropertyDescriptor(err, "cause")?.enumerable).toBe(false);
});

test("preserves an assigned cause as enumerable", async () => {
const worker = new Worker(
`const error = new Error("outer");
error.cause = "assigned cause";
throw error;`,
{ eval: true },
);
const [err] = await once(worker, "error");
expect(err.cause).toBe("assigned cause");
expect(Object.getOwnPropertyDescriptor(err, "cause")?.enumerable).toBe(true);
expect(Object.keys(err)).toContain("cause");
});

test("an uncloneable cause does not suppress cloneable metadata", async () => {
const worker = new Worker(
`const { MessageChannel } = require("node:worker_threads");
const error = new Error("outer", { cause: new MessageChannel().port1 });
error.details = { preserved: true };
throw error;`,
{ eval: true },
);
const [err] = await once(worker, "error");
expect(err).toBeInstanceOf(Error);
expect(err.message).toBe("outer");
expect(err.details).toEqual({ preserved: true });
});

test("bounds deeply nested cause metadata", async () => {
const worker = new Worker(
`let error = new Error("leaf");
for (let i = 0; i < 10_000; i++) error = new Error("level " + i, { cause: error });
throw error;`,
{ eval: true },
);
const [err] = await once(worker, "error");
expect(err).toBeInstanceOf(Error);
expect(err.message).toBe("level 9999");
let cause = err;
let depth = 0;
while (cause instanceof Error) {
cause = cause.cause;
depth++;
}
expect(depth).toBeGreaterThan(1);
expect(depth).toBeLessThan(100);
});

test("falls back to string when the error cannot be serialized", async () => {
const worker = new Worker(
/* js */ `
Expand Down
Loading