Skip to content

fix(dom): don't assign to error.message in handleErrors (getter-only errors) - #2372

Open
aryanku-dev wants to merge 1 commit into
masterfrom
fix/PER-10368-dom-handle-errors-getter
Open

fix(dom): don't assign to error.message in handleErrors (getter-only errors)#2372
aryanku-dev wants to merge 1 commit into
masterfrom
fix/PER-10368-dom-handle-errors-getter

Conversation

@aryanku-dev

Copy link
Copy Markdown
Contributor

Fixes PER-10368.

Problem

Customer reports snapshots being dropped from builds with inconsistent snapshot counts between runs. Their log:

[percy] Could not take DOM snapshot "form controls — comfortable editable — file uploads"
[percy] Error: page.evaluate: TypeError: Cannot set property message of  which has only a getter
    at handleErrors (eval at evaluate (:290:30), <anonymous>:90:21)
    at serializeCanvas (eval at evaluate (:290:30), <anonymous>:604:13)
    at serializeElements (eval at evaluate (:290:30), <anonymous>:1494:9)
    at Object.serializeDOM (eval at evaluate (:290:30), <anonymous>:1583:9)

Root cause

packages/dom/src/utils.jshandleErrors enriches an error by mutating it:

let message = error.message;
message += `\n${prefixMessage} \n${JSON.stringify(additionalData)}`;
message += '\n Please validate that your DOM is as per W3C standards using any online tool';
error.message = message;   // <-- throws for DOMException

DOMException declares message as a getter-only accessor (WebIDL readonly attribute DOMString message). Assigning to it inside this strict-mode bundle throws a TypeError:

$ node -e "..."
DOMException.prototype.message descriptor: {"get":true,"set":"UNDEFINED (getter-only)"}
ASSIGN THROWS -> TypeError: Cannot set property message of  which has only a getter

That reproduction is character-for-character identical to the customer's log, including the double space where the object tag would render.

And DOMException is precisely what the guarded operations throw — canvas.toDataURL() raises SecurityError on a canvas tainted by cross-origin data. So the chain is:

  1. canvas.toDataURL() throws SecurityError (a real, actionable, well-understood condition)
  2. handleErrors tries to enrich it → TypeError, masking the original error
  3. The TypeError propagates through serializeElementsserializeDOMpage.evaluate rejects
  4. Could not take DOM snapshot — the entire snapshot is dropped

Because it depends on which canvases happen to be tainted on a given run, snapshots disappear non-deterministically — the reported inconsistent counts.

This affects all seven handleErrors call sites, not just canvas: serialize-cssom (cross-origin stylesheet access also throws SecurityError), serialize-dialog, serialize-video, serialize-inputs, clone-dom, and styleSheetFromNode.

Fix

Enrich in place when the assignment succeeds; otherwise throw a new Error carrying the enriched message plus the original's name and a cause reference. The original error is never lost.

The error.message !== message re-check after the try also covers sloppy-mode callers, where the assignment fails silently instead of throwing, and frozen/sealed errors.

Testing

yarn workspace @percy/dom test500 tests pass. Five new specs, and there were previously no tests for handleErrors at all.

Verified these are genuine regression tests — reverting only src/utils.js and re-running:

handleErrors
  ✔ enriches the message in place on a plain Error
  ✔ includes element data when an element is passed
  ✖ does not throw a TypeError when the error message is getter-only
  ✖ preserves the original message, name, and cause when message is getter-only
  ✖ handles a frozen error without throwing a TypeError

All five pass with the fix applied.

Note

After this change, a tainted-canvas snapshot surfaces the real SecurityError instead of a TypeError. That is the intended outcome — the underlying condition is genuinely actionable, and users who want to tolerate it already have the ignoreCanvasSerializationErrors config option, which bypasses handleErrors entirely.

🤖 Generated with Claude Code

…errors)

`handleErrors` enriches an error by assigning to `error.message`. DOMException
declares `message` as a getter-only accessor (WebIDL `readonly attribute`), so
in this strict-mode bundle the assignment throws:

  TypeError: Cannot set property message of #<DOMException> which has only a getter

DOMException is exactly what the guarded calls throw — `canvas.toDataURL()` on a
tainted canvas raises SecurityError. So the enrichment step replaces the real,
actionable error with a confusing TypeError, which propagates out of serializeDOM
and fails the whole snapshot ("Could not take DOM snapshot"). Snapshots then drop
out of builds non-deterministically, giving inconsistent snapshot counts.

All seven handleErrors call sites are affected (canvas, cssom, dialog, video,
inputs, clone-dom, styleSheetFromNode), not just canvas.

Enrich in place when the assignment succeeds; otherwise throw a new Error that
carries the enriched message plus the original's name and a `cause` reference.
Also covers frozen/sealed errors and sloppy-mode callers, where the assignment
fails silently rather than throwing.

Fixes PER-10368

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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