Skip to content

feat(clone): add structuredClone support for Error objects and relate…#5280

Open
akshatnathani wants to merge 2 commits intoboa-dev:mainfrom
akshatnathani:fix/structured-clone-error-support
Open

feat(clone): add structuredClone support for Error objects and relate…#5280
akshatnathani wants to merge 2 commits intoboa-dev:mainfrom
akshatnathani:fix/structured-clone-error-support

Conversation

@akshatnathani
Copy link
Copy Markdown

This Pull Request fixes/closes #5279.

It changes the following:

  • Added Error object support in the JsValueStore serialization path so structuredClone(new Error("boom")) and postMessage(new Error("boom")) no longer fail during cloning.
  • Implemented Error object reconstruction in the deserialization path using stored error metadata (kind, name, message, and related fields), instead of returning Not yet implemented.
  • Added regression tests for both clone paths:
    • clone::tests::clones_error_objects
    • message::tests::basic_error_message
  • Removed the stale #[expect(unused)] on the ValueStoreInner::Error variant, since it is now actively used.
  • Verified consistency with project standards and checks:
    • cargo fmt --all -- --check
    • cargo clippy -p boa_runtime --tests -- -D warnings
    • cargo test -p boa_runtime clones_error_objects -- --nocapture
    • cargo test -p boa_runtime basic_error_message -- --nocapture

@akshatnathani akshatnathani requested a review from a team as a code owner March 28, 2026 17:14
@github-actions github-actions bot added C-Tests Issues and PRs related to the tests. C-Runtime Issues and PRs related to Boa's runtime features Waiting On Review Waiting on reviews from the maintainers labels Mar 28, 2026
@github-actions github-actions bot added this to the v1.0.0 milestone Mar 28, 2026

let name = to_optional_string("name", context)?;
let stack = to_optional_string("stack", context)?;
let cause = to_optional_string("cause", context)?;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cause is getting flattened into a string here before it goes into the store. that works for simple text causes, but it breaks real cases like new Error("boom", { cause: { code: 7 } }), where the cloned error should still keep the original object as its cause.

Copy link
Copy Markdown
Author

@akshatnathani akshatnathani Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch. I fixed the cause handling so that it is preserved as structured data rather than stringified. Added regression coverage for both structuredClone and postMessage paths, and also looking into the AggregateError issue you pointed out, and will follow up with the patch/tests shortly.

let (kind, name, message, stack, cause) = error_data;
let message = message.to_js_string().to_std_string_escaped();
let native = match kind {
ErrorKind::Aggregate => JsNativeError::aggregate(Vec::new()),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this rebuilds AggregateError with an empty list, so the cloned value loses its errors property on the way back out. i checked this with a quick repro and structuredClone(new AggregateError([new Error("inner")], "agg")) comes back with errors.length === 0.

@akshatnathani akshatnathani force-pushed the fix/structured-clone-error-support branch from 0f3a392 to 98a2ce6 Compare March 29, 2026 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Runtime Issues and PRs related to Boa's runtime features C-Tests Issues and PRs related to the tests. Waiting On Review Waiting on reviews from the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

structuredClone and postMessage reject Error objects

2 participants