fix: swallow CSS insertRule parse errors#1818
fix: swallow CSS insertRule parse errors#1818megboehlert wants to merge 1 commit intorrweb-io:masterfrom
Conversation
🦋 Changeset detectedLatest commit: ae3f445 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR updates rrweb’s stylesheet rule observation to tolerate insertRule parse errors (e.g., vendor-specific selectors unsupported in the recording browser) so recording can continue, and adds tests to validate the behavior.
Changes:
- Wrap
CSSStyleSheet.prototype.insertRuleand nested ruleinsertRulecalls with error swallowing behavior. - Add vitest/puppeteer coverage for unsupported vendor-prefixed rules at top-level and within nested grouping rules.
- Add a changeset entry for a patch release.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/rrweb/src/record/observer.ts | Swallows insertRule exceptions in proxied stylesheet and nested-rule insertion paths. |
| packages/rrweb/test/record.test.ts | Adds tests asserting unsupported vendor-prefixed rules don’t break recording and are still emitted as events. |
| .changeset/wicked-sheep-unite.md | Declares a patch release note for swallowing insertRule parse errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| adds: [{ rule, index }], | ||
| }); | ||
| } | ||
| return target.apply(thisArg, argumentsList); | ||
| try { | ||
| return target.apply(thisArg, argumentsList); |
There was a problem hiding this comment.
The Proxy for CSSStyleSheet.prototype.insertRule now swallows all exceptions from the native insertRule call. This changes page/runtime semantics (e.g., it will hide IndexSizeError, SecurityError, etc.), and it also means a StyleSheetRule event may be emitted even when the underlying rule was not actually inserted. Consider only swallowing parse errors (typically DOMException/SyntaxError) and rethrowing other exceptions; and only emitting styleSheetRuleCb after a successful insert (or emitting + swallowing only for parse errors). Also ensure the return value stays consistent with insertRule’s numeric contract when you swallow an error.
| try { | ||
| return target.apply(thisArg, argumentsList); | ||
| } catch (e) { | ||
| // Rules valid in one browser may be rejected by another. | ||
| // Consume the parse error |
There was a problem hiding this comment.
Same concern for nested CSS rule insertRule monkey-patching: catching and swallowing all errors can mask non-parse failures and will still record adds even if insertion failed. Restrict swallowing to parse errors (SyntaxError) and rethrow other exceptions; ideally only emit styleSheetRuleCb when the insert succeeded (or only emit + swallow for parse errors).
No description provided.