fix(session-replay-browser): prevent broken styles while recording with more CSS-in-JS libraries #1499
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes broken CSS styles which can occur while recording a session replay. It happens when the client is using a CSS-in-JS library to create styles, and there is a single bad CSS rule somewhere.
Session replay already has explicit support for styled-components, but it has two problems:
CSSGroupingRule.insertRule, such as stitchesThis is a critical issue for my team, as we cannot use session replay at all if there's a possibility broken styles occur while recording. Even worse, the failure seems to cascade, producing more and more broken styles.
Details
After investigation and testing, I believe I have a full understanding of the problem:
CSSStyleSheet.insertRuleCSSGroupingRule.insertRuleAn example bad selector is this:
The problem code in the session replay implementation:
Amplitude-TypeScript/packages/session-replay-browser/src/session-replay.ts
Lines 636 to 639 in 752b4c0
As an example, here is where the problem breaks stitches:
https://github.com/stitchesjs/stitches/blob/50fd8a1adc6360340fe348a8b3ebc8b06d38e230/packages/core/src/sheet.js#L189
Proposed solution
A desired solution would be one that supports both
CSSStyleSheet.insertRuleandCSSGroupingRule.insertRule, while also supporting the various error formats created by the different browsers.An even better solution would be one which avoids parsing string values from return errors. However, in lieu of a bigger refactor to do so, I've examined the various returned browser errors and have come up with this simple change:
Here's an example error in each browser for comparison:
Checklist
Note
Improves compatibility with CSS-in-JS during capture to prevent suppressed errors from breaking styles.
errorHandlerto rethrow whentypedError.stackincludesinsertRule, covering bothCSSStyleSheet.insertRuleandCSSGroupingRule.insertRuleWritten by Cursor Bugbot for commit 77a9556. This will update automatically on new commits. Configure here.