Skip to content

feat(error): Added better serialization for DOMException errors (OUT-580) - #420

Open
szanata wants to merge 4 commits into
mainfrom
feat/serialize_dom_exception
Open

szanata wants to merge 4 commits into
mainfrom
feat/serialize_dom_exception

Conversation

@szanata

@szanata szanata commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Added a dedicated serialization for DOMException errors, dropping the legacy 25 enum fields that these errors have, leaving only name, code, message and stack. DOMException are the error type for Abortions. More info at MDN.
  • Fixed a bug that caused name to skip the serialization and possible causing Circular Dependency exceptions.

@szanata
szanata marked this pull request as ready for review September 18, 2026 20:31
@szanata szanata changed the title feat(error): Added better serialization for DOMException errors feat(error): Added better serialization for DOMException errors (OUT-580) Sep 18, 2026
Comment thread sdk/core/src/helpers/error_serializer.js Outdated
@github-actions

This comment was marked as outdated.

Comment thread sdk/core/src/helpers/error_serializer.js Outdated
Comment thread sdk/core/src/helpers/error_serializer.spec.js Outdated
@github-actions

This comment was marked as outdated.

…ist; Fixed issue that caused name to skip serialization
Comment thread sdk/core/src/helpers/error_serializer.js
@github-actions

This comment was marked as outdated.

Comment thread sdk/core/src/helpers/error_serializer.js Outdated
Comment thread sdk/core/src/helpers/error_serializer.js
@github-actions

This comment was marked as outdated.

@szanata
szanata force-pushed the feat/serialize_dom_exception branch from 71252b7 to 3264fce Compare September 18, 2026 22:39
Comment thread sdk/core/src/helpers/error_serializer.js
Comment thread sdk/core/src/helpers/error_serializer.js
@github-actions

Copy link
Copy Markdown
Contributor

PR review

Verdict

✅ PASS

Findings

  1. Nice-to-have (Design): The DOMException allowlist in serializeValue also strips own properties, not just the legacy prototype constants — the new spec asserts error.attempt = 2 vanishes. Since the 25 constants live only on DOMException.prototype, applying the filter to inherited keys (or denylisting the constants) would drop the noise while keeping caller-attached context on abort reasons in traces.

  2. Nice-to-have (Tests): 'detects DOMExceptions where the global is unavailable, as in the workflow sandbox' stubs a global that isDomException never reads, so it does not exercise the cross-realm behavior its name claims, and its serializeError assertion duplicates the dedicated DOMException test. It only guards against a future refactor to instanceof — worth renaming to say that, or folding in.

  3. Nice-to-have (Quality): evaluateSpecialObjectsKeysList returning null plus the Array.isArray( strictKeysList ) guard at the call site is a dispatcher built for one case (YAGNI). A module-level DOM_EXCEPTION_KEYS const with an inline isDomException( target ) check is shorter and matches the terse style of the surrounding helpers.

  4. Nice-to-have (Changeset): .changeset/tidy-aborts-serialize.md only describes the DOMException change. The second user-visible fix in this PR — props.name now being truncated/recursively serialized, so a circular or oversized error.name yields [Circular Reference] / a truncated string instead of a raw value — is not mentioned.

Categories

  • Design: ✅ PASS
  • Quality: ✅ PASS
  • Correctness: ✅ PASS
  • Documentation: ✅ PASS
  • Changeset: ✅ PASS
  • Tests: ✅ PASS
  • Security: ✅ PASS
  • Compatibility: ✅ PASS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant