getsentry / rrweb Goto Github PK

Environment

SaaS (https://sentry.io/)

Version

Link

https://sentry.zendesk.com/agent/tickets/79673

DSN

No response

Steps to Reproduce

1. Navigate to linked Replay in Zendesk ticket
2. Skip ahead and then back using the player's slider
3. Observe that styles are not being rendered properly.

Expected Result

Styles will be rendered properly when not requiring to be fetched in a way that violates CORS

Actual Result

Styles are present but not rendered

Preflight Checklist

• I have searched the issue tracker for a bug report that matches the one I want to file, without success.

What package is this bug report for?

rrweb

Version

latest

Expected Behavior

The text node inside of a <noscript> should not be rendered.

Actual Behavior

The text inside of <noscript> is being rendered as a string on playback.

Steps to Reproduce

See this zendesk ticket

Testcase Gist URL

No response

Additional Information

No response

E.g. the following HTML:

<input type="submit" name="tsubmit" id="submit-save_button" value="Continue" class="replaceable">

renders like this:

We should mask the value of such elements when maskAllText === true

Let's pull in our v1 changes into the master branch so we're not diverged too far from v2. This will help us as we transition to v2 (#76).

Sentry Issue: JAVASCRIPT-2FQ5

TypeError: Cannot read properties of undefined (reading 'style')
  at Replayer.applyIncremental (../node_modules/@sentry-internal/rrweb/es/rrweb/packages/rrweb/src/replay/index.js:936:72)
  at castFn (../node_modules/@sentry-internal/rrweb/es/rrweb/packages/rrweb/src/replay/index.js:378:26)
  at castFn (../node_modules/@sentry-internal/rrweb/es/rrweb/packages/rrweb/src/replay/index.js:414:17)
  at <object>.doAction (../node_modules/@sentry-internal/rrweb/es/rrweb/packages/rrweb/src/replay/machine.js:126:33)
  at this.check (../node_modules/@sentry-internal/rrweb/es/rrweb/packages/rrweb/src/replay/timer.js:30:28)
...
(1 additional frame(s) were not displayed)

Apparently, the mutation observer triggers each time an attribute/class is set, even if it does not change.

For example:

el.setAttribute('attr', 'val');
el.setAttribute('attr', 'val');
el.setAttribute('attr', 'val');

or

el.classList.add('my-class');
el.classList.add('my-class');
el.classList.add('my-class');

will each trigger the mutation observer three times, even though the DOM doesn't actually change.

We should check if the value doesn't change and skip doing any work in this case.

Problem Statement

Because of bugs in rrweb, its slow development cycle, and the fact that they're working on v2 (which is also problematic on its own, getsentry/sentry-javascript#6655) we have to apply multiple patches to the library, and bundle it into our @sentry/replay package. This has a number of drawbacks:

• The patch we apply with patch-package has grown enormously (10k lines; see getsentry/sentry-javascript#6749)
• As outlined in getsentry/sentry-javascript#6749, applying this patch correctly involves a lot of steps
• Because of patching, we have to pull the rrweb files into our replay package, which causes isses (#6690)

Solution Brainstorm

In a first step, to make developing patches on rrweb v1 easier, we update our rrweb fork in which we now create and merge PRs which we then use to patch the actual rrweb package.

We now propose to release our rrweb fork (e.g. @sentry-internal/rrweb) to NPM so we can stop patching the official package all together and just use our fork like a regular dependency. The steps to get this done are:

#15 (comment)

#15 (comment)

#15 (comment)

For the most part, we can follow our SDK release checklist to make sure we've covered everything necessary.

Furthermore, we can also take getsentry/sentry-javascript-bundler-plugins#60 as an inspiration, where we recently started releasing to NPM from a new repo.

Our SDKs (+the integrations) should fail silently. So let's try/catch more parts of rrweb whcih are likely to fail:

#25 (comment)

Add other places here if identified

Mutations are capturing file uploads, which cause the replay to become very large. There's no reason to capture this data.

Example snippet:

{
  "type": 3,
  "data": {
    "source": 5,
    "text": "C:\\fakepath\\foo.zip",
    "isChecked": false,
    "id": 10513
  },
  "timestamp": 1674760126020
},
{
  "type": 3,
  "data": {
    "source": 5,
    "text": "",
    "isChecked": false,
    "id": 10513
  },
  "timestamp": 1674760126047
},
{
  "type": 3,
  "data": {
    "source": 0,
    "texts": [],
    "attributes": [
      {
        "id": 10517,
        "attributes": {
          "src": "data:application/x-zip-compressed;base64,UEsDBBQACAgIAMJYOlYAAAAAAAAAAAAAAAATABwAMjAyMzAxMjZfMTQwNDM4LmpwZ3VwGAAB3n1+lzIwMjMwMTI2XzE0MDQzOC5qcGcAAID/f//Y/+EC3EV4aWYAAE1NACoAAAAIAA0BAAADAAAAAQ/AAAABAQADAAAAAQvQAAABDwACAAAACAAAAKoBEAACAAAACQA...

See https://github.com/rrweb-io/rrweb/releases to track release status

Benefits

• Support for canvas
  • getsentry/sentry-javascript#6519
  • Can we treeshake it out if canvas is not opt'ed in
• Support for cross origin iframes
  • getsentry/sentry-javascript#7279
• Less maintenance overhead with our own patched v1
• Fixes
  • #57
  • getsentry/sentry-javascript#8469
  • getsentry/sentry-javascript#8123

Risks:

• Still Alpha
• Package size is ~40% larger

Related:

• getsentry/sentry-javascript#7279
• getsentry/sentry-javascript#6946

Currently, a select box will be masked like this:

<select value="*******">
  <option value="default">First value text</option>
<option value="other-value">Second value text</option>
</select>

This shows wrong/weirdly in the replay, and also doesn't really obscure everything that is relevant. We prob. need to handle this better for maskAllInputs.

When maskAllText is on, attributes are always masked even if the element has a class matching unmaskSelector.

e.g. <input placeholder="This should be unmasked" class="rr-unmask" />

Only typings are being included in the package:

Blocks getsentry/sentry#44629

#172 disabled the flakey test, we should investigate how to make it stable.

Environment

SaaS (https://sentry.io/)

Version

sentry/nextjs 7.31.1

Link

No response

DSN

No response

Steps to Reproduce

We've got an issue in Sentry which seems to point at an error within Replay code.
The reported error is TypeError Cannot read properties of undefined (reading 'prototype') inside styleSheetObserver.
See screenshot below.
We have Replay sampling only turned on for errors, ironically it was able to capture a replay of the error it caused itself.

I can share the issue or replay privately if needed.

Expected Result

I expect Replay to record interactions for errors, not cause errors itself.

Actual Result

Currently, users can write custom unmask selectors to unmask passwords, meaning their users' passwords can be visible in clear text if the website for instance provides a "View Password" funcitonality.

IMO, under no circumstances whatsoever, never! (yup, feeling strongly about this) should we allow this to happen.

After chatting a little bit about this, the probably best way to block this type of unmasking, is to change the unmasking behaviour in rrweb. We should investigate how to handle this best.

Preflight Checklist

• I have searched the issue tracker for a bug report that matches the one I want to file, without success.

What package is this bug report for?

rrweb

Version

2.6.0

Expected Behavior

I would expect that even with a strict CSP policy (e.g. style-src 'nonce-${SOME_NONCE}), I would still be able to successfully capture replays. (See a similar issue in a different package)

Actual Behavior

A CSP error occurs due to use of setAttribute('style', ...) here

Steps to Reproduce

• Set up a vue project that uses Sentry with replays enabled and a strict CSP policy (e.g. style-src 'nonce-${SOME_NONCE})
• Write a basic component that uses `:style="{ 'background-color': someDynamicVariable }"
• Deploy the application, trigger an error so a replay begins and update someDynamicVariable - observe how a CSP error occurs

Testcase Gist URL

No response

Additional Information

There might be a simpler way to reproduce the issue, but I've described the setup we observed the issue in. Without any knowledge of what the culprit code is actually trying to do, its hard for me to provide a more concise reproduction.