getsentry / rrweb

record and replay the web
https://www.rrweb.io/
Other
9 stars 5 forks source link

fix: Ignore errors in style observers #16

Closed mydea closed 1 year ago

mydea commented 1 year ago

We keep getting errors from the stylesheet observer callback. So let's try catch these to be safe.

mydea commented 1 year ago

The problem is that there is nothing about these lines of codes that should error out. So it's kind of hard to fix this... I want to open an issue in rrweb to ask what the whole stylesheet observer thing is actually needed for, what we're not anyhow getting by the mutation observer. That would be interesting to know!

billyvg commented 1 year ago

Yeah for whatever reason, win.CSSStyleSheet becomes undefined when we stop the observers.

I don't think MutationObserver can detect changes the the CSS Object Model, that's why the CSS observer is needed.

mydea commented 1 year ago

I don't think MutationObserver can detect changes the the CSS Object Model, that's why the CSS observer is needed.

True, but this really only covers the case where somebody uses the CSS style sheet API directly, e.g.:

let style = window.styleSheets[0];
style.insertRule('body { color: red; }');

Which honestly I didn't even know existed as an API, and I suspect is something used rather rarely (but then again, maybe I am completely missing something here, who knows!). Mutation observers do catch the IMHO more common cases to adjust styles:

  1. Added/changed/removed <style> tags
  2. Added/changed/removes style='xxx' attributes
  3. Added/changed/removed <link href="style.css"> tags

One option (hypothetical) could also be to require users to opt-in to having the style observers. This may reduce overhead and avoid this category of bugs for the majority of users that don't actually need them 🤔

mydea commented 1 year ago

Okay, looking at rrweb history, I guess we probably need this 😅 https://github.com/rrweb-io/rrweb/pull/177