fix(replay): Fix potential broken CSS in styled-components - #9234

Merged
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule
Oct 13, 2023
Merged

fix(replay): Fix potential broken CSS in styled-components#9234
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule

Conversation

@billyvg

@billyvgbillyvg commented Oct 12, 2023

Copy link
Copy Markdown
Member

Fixes an issue where the Replay integration can potentially break applications that use styled-components. styled-componentsrelies on an exception being throw for CSS rules that are not supported by the browser engine. However, our SDK suppressed any exceptions thrown from within rrweb, so styled-components assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.

This was a regression from v1 as we were always re-throwing the exception

Fixes#9170

@billyvg
billyvg marked this pull request as ready for review October 12, 2023 18:28
@github-actions

github-actionsBot commented Oct 12, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)84.26 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.43 KB (0%)
@sentry/browser - Webpack (gzipped)22.02 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)78.78 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.61 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)21.02 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)254.48 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.76 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)62.45 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.48 KB (-0.01% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)84.29 KB (-0.01% 🔽)
@sentry/react - Webpack (gzipped)22.06 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)102.2 KB (-0.01% 🔽)
@sentry/nextjs Client - Webpack (gzipped)50.96 KB (0%)

Comment threadpackages/replay/src/integration.ts Outdated
@AbhiPrasad

Copy link
Copy Markdown
Contributor

We can do a release tomorrow with this.

Comment threadpackages/replay/src/integration.ts Outdated
// return true to suppress throwing the error inside of rrweb
return true;
errorHandler: (err: Error & {__rrweb__?: boolean}) => {
err.__rrweb__ = true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm so one problem here is that we may throw/error on trying to set this on something that is e.g. frozen or similar. Can we instead still try-catch here and just re-throw certain kinds of exceptions, maybe?

I mean, maybe this is actually fine, but we could be getting other errors because of this, at some point, too 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually, can we just do:

errorHandler: (err: Error)=>{try{// @ts-expect-error Set this so that replay SDK can ignore errors originating from rrweberr.__rrweb__=true;}catch{// avoid any potential hazards here}

So still try-catch setting this prop, but bubble the error up to rrweb 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this like this, so we can merge & release this!

billyvgand others added 4 commits October 13, 2023 10:12
NOTE: This requires a bump to rrweb library
Fixes an issue where the Replay integration can potentially break applications that use `styled-components`. `styled-components` [relies on an exception being throw](https://github.com/styled-components/styled-components/blob/b7b374bb1ceff1699f7035b15881bc807110199a/packages/styled-components/src/sheet/Tag.ts#L32-L40) for CSS rules that are not supported by the browser engine. However, our SDK suppresses any exceptions thrown from within rrweb, so `styled-components` assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.
getsentry/rrweb#111 introduces a change the adds metadata to exceptions that occur when calling `insertRule`, and this PR will re-throw those exceptions that will then bubble up to `styled-components`.
Fixes#9170
@mydea
mydeaforce-pushed the fix-replay-fix-catching-exceptions-from-insertRule branch from b1876c5 to a66168fCompareOctober 13, 2023 08:16
@mydea
mydea merged commit 73a808a into developOct 13, 2023
@mydea
mydea deleted the fix-replay-fix-catching-exceptions-from-insertRule branch October 13, 2023 09:14
Sign up for freeto 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.

Sentry 7.73.0 causes styled-components to fail to render with styles/CSS

3 participants

@billyvg@AbhiPrasad@mydea
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(replay): Fix potential broken CSS in styled-components - #9234

Merged
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule
Oct 13, 2023
Merged

fix(replay): Fix potential broken CSS in styled-components#9234
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule

Conversation

@billyvg

@billyvgbillyvg commented Oct 12, 2023

Copy link
Copy Markdown
Member

Fixes an issue where the Replay integration can potentially break applications that use styled-components. styled-componentsrelies on an exception being throw for CSS rules that are not supported by the browser engine. However, our SDK suppressed any exceptions thrown from within rrweb, so styled-components assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.

This was a regression from v1 as we were always re-throwing the exception

Fixes#9170

@billyvg
billyvg marked this pull request as ready for review October 12, 2023 18:28
@github-actions

github-actionsBot commented Oct 12, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)84.26 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.43 KB (0%)
@sentry/browser - Webpack (gzipped)22.02 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)78.78 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.61 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)21.02 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)254.48 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.76 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)62.45 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.48 KB (-0.01% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)84.29 KB (-0.01% 🔽)
@sentry/react - Webpack (gzipped)22.06 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)102.2 KB (-0.01% 🔽)
@sentry/nextjs Client - Webpack (gzipped)50.96 KB (0%)

Comment threadpackages/replay/src/integration.ts Outdated
@AbhiPrasad

Copy link
Copy Markdown
Contributor

We can do a release tomorrow with this.

Comment threadpackages/replay/src/integration.ts Outdated
// return true to suppress throwing the error inside of rrweb
return true;
errorHandler: (err: Error & {__rrweb__?: boolean}) => {
err.__rrweb__ = true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm so one problem here is that we may throw/error on trying to set this on something that is e.g. frozen or similar. Can we instead still try-catch here and just re-throw certain kinds of exceptions, maybe?

I mean, maybe this is actually fine, but we could be getting other errors because of this, at some point, too 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually, can we just do:

errorHandler: (err: Error)=>{try{// @ts-expect-error Set this so that replay SDK can ignore errors originating from rrweberr.__rrweb__=true;}catch{// avoid any potential hazards here}

So still try-catch setting this prop, but bubble the error up to rrweb 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this like this, so we can merge & release this!

billyvgand others added 4 commits October 13, 2023 10:12
NOTE: This requires a bump to rrweb library
Fixes an issue where the Replay integration can potentially break applications that use `styled-components`. `styled-components` [relies on an exception being throw](https://github.com/styled-components/styled-components/blob/b7b374bb1ceff1699f7035b15881bc807110199a/packages/styled-components/src/sheet/Tag.ts#L32-L40) for CSS rules that are not supported by the browser engine. However, our SDK suppresses any exceptions thrown from within rrweb, so `styled-components` assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.
getsentry/rrweb#111 introduces a change the adds metadata to exceptions that occur when calling `insertRule`, and this PR will re-throw those exceptions that will then bubble up to `styled-components`.
Fixes#9170
@mydea
mydeaforce-pushed the fix-replay-fix-catching-exceptions-from-insertRule branch from b1876c5 to a66168fCompareOctober 13, 2023 08:16
@mydea
mydea merged commit 73a808a into developOct 13, 2023
@mydea
mydea deleted the fix-replay-fix-catching-exceptions-from-insertRule branch October 13, 2023 09:14
Sign up for freeto 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.

Sentry 7.73.0 causes styled-components to fail to render with styles/CSS

3 participants

@billyvg@AbhiPrasad@mydea
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(replay): Fix potential broken CSS in styled-components - #9234

Merged
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule
Oct 13, 2023
Merged

fix(replay): Fix potential broken CSS in styled-components#9234
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule

Conversation

@billyvg

@billyvgbillyvg commented Oct 12, 2023

Copy link
Copy Markdown
Member

Fixes an issue where the Replay integration can potentially break applications that use styled-components. styled-componentsrelies on an exception being throw for CSS rules that are not supported by the browser engine. However, our SDK suppressed any exceptions thrown from within rrweb, so styled-components assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.

This was a regression from v1 as we were always re-throwing the exception

Fixes#9170

@billyvg
billyvg marked this pull request as ready for review October 12, 2023 18:28
@github-actions

github-actionsBot commented Oct 12, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)84.26 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.43 KB (0%)
@sentry/browser - Webpack (gzipped)22.02 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)78.78 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.61 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)21.02 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)254.48 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.76 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)62.45 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.48 KB (-0.01% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)84.29 KB (-0.01% 🔽)
@sentry/react - Webpack (gzipped)22.06 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)102.2 KB (-0.01% 🔽)
@sentry/nextjs Client - Webpack (gzipped)50.96 KB (0%)

Comment threadpackages/replay/src/integration.ts Outdated
@AbhiPrasad

Copy link
Copy Markdown
Contributor

We can do a release tomorrow with this.

Comment threadpackages/replay/src/integration.ts Outdated
// return true to suppress throwing the error inside of rrweb
return true;
errorHandler: (err: Error & {__rrweb__?: boolean}) => {
err.__rrweb__ = true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm so one problem here is that we may throw/error on trying to set this on something that is e.g. frozen or similar. Can we instead still try-catch here and just re-throw certain kinds of exceptions, maybe?

I mean, maybe this is actually fine, but we could be getting other errors because of this, at some point, too 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually, can we just do:

errorHandler: (err: Error)=>{try{// @ts-expect-error Set this so that replay SDK can ignore errors originating from rrweberr.__rrweb__=true;}catch{// avoid any potential hazards here}

So still try-catch setting this prop, but bubble the error up to rrweb 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this like this, so we can merge & release this!

billyvgand others added 4 commits October 13, 2023 10:12
NOTE: This requires a bump to rrweb library
Fixes an issue where the Replay integration can potentially break applications that use `styled-components`. `styled-components` [relies on an exception being throw](https://github.com/styled-components/styled-components/blob/b7b374bb1ceff1699f7035b15881bc807110199a/packages/styled-components/src/sheet/Tag.ts#L32-L40) for CSS rules that are not supported by the browser engine. However, our SDK suppresses any exceptions thrown from within rrweb, so `styled-components` assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.
getsentry/rrweb#111 introduces a change the adds metadata to exceptions that occur when calling `insertRule`, and this PR will re-throw those exceptions that will then bubble up to `styled-components`.
Fixes#9170
@mydea
mydeaforce-pushed the fix-replay-fix-catching-exceptions-from-insertRule branch from b1876c5 to a66168fCompareOctober 13, 2023 08:16
@mydea
mydea merged commit 73a808a into developOct 13, 2023
@mydea
mydea deleted the fix-replay-fix-catching-exceptions-from-insertRule branch October 13, 2023 09:14
Sign up for freeto 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.

Sentry 7.73.0 causes styled-components to fail to render with styles/CSS

3 participants

@billyvg@AbhiPrasad@mydea
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(replay): Fix potential broken CSS in styled-components - #9234

Merged
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule
Oct 13, 2023
Merged

fix(replay): Fix potential broken CSS in styled-components#9234
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule

Conversation

@billyvg

@billyvgbillyvg commented Oct 12, 2023

Copy link
Copy Markdown
Member

Fixes an issue where the Replay integration can potentially break applications that use styled-components. styled-componentsrelies on an exception being throw for CSS rules that are not supported by the browser engine. However, our SDK suppressed any exceptions thrown from within rrweb, so styled-components assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.

This was a regression from v1 as we were always re-throwing the exception

Fixes#9170

@billyvg
billyvg marked this pull request as ready for review October 12, 2023 18:28
@github-actions

github-actionsBot commented Oct 12, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)84.26 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.43 KB (0%)
@sentry/browser - Webpack (gzipped)22.02 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)78.78 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.61 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)21.02 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)254.48 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.76 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)62.45 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.48 KB (-0.01% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)84.29 KB (-0.01% 🔽)
@sentry/react - Webpack (gzipped)22.06 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)102.2 KB (-0.01% 🔽)
@sentry/nextjs Client - Webpack (gzipped)50.96 KB (0%)

Comment threadpackages/replay/src/integration.ts Outdated
@AbhiPrasad

Copy link
Copy Markdown
Contributor

We can do a release tomorrow with this.

Comment threadpackages/replay/src/integration.ts Outdated
// return true to suppress throwing the error inside of rrweb
return true;
errorHandler: (err: Error & {__rrweb__?: boolean}) => {
err.__rrweb__ = true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm so one problem here is that we may throw/error on trying to set this on something that is e.g. frozen or similar. Can we instead still try-catch here and just re-throw certain kinds of exceptions, maybe?

I mean, maybe this is actually fine, but we could be getting other errors because of this, at some point, too 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually, can we just do:

errorHandler: (err: Error)=>{try{// @ts-expect-error Set this so that replay SDK can ignore errors originating from rrweberr.__rrweb__=true;}catch{// avoid any potential hazards here}

So still try-catch setting this prop, but bubble the error up to rrweb 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this like this, so we can merge & release this!

billyvgand others added 4 commits October 13, 2023 10:12
NOTE: This requires a bump to rrweb library
Fixes an issue where the Replay integration can potentially break applications that use `styled-components`. `styled-components` [relies on an exception being throw](https://github.com/styled-components/styled-components/blob/b7b374bb1ceff1699f7035b15881bc807110199a/packages/styled-components/src/sheet/Tag.ts#L32-L40) for CSS rules that are not supported by the browser engine. However, our SDK suppresses any exceptions thrown from within rrweb, so `styled-components` assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.
getsentry/rrweb#111 introduces a change the adds metadata to exceptions that occur when calling `insertRule`, and this PR will re-throw those exceptions that will then bubble up to `styled-components`.
Fixes#9170
@mydea
mydeaforce-pushed the fix-replay-fix-catching-exceptions-from-insertRule branch from b1876c5 to a66168fCompareOctober 13, 2023 08:16
@mydea
mydea merged commit 73a808a into developOct 13, 2023
@mydea
mydea deleted the fix-replay-fix-catching-exceptions-from-insertRule branch October 13, 2023 09:14
Sign up for freeto 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.

Sentry 7.73.0 causes styled-components to fail to render with styles/CSS

3 participants

@billyvg@AbhiPrasad@mydea
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(replay): Fix potential broken CSS in styled-components - #9234

Merged
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule
Oct 13, 2023
Merged

fix(replay): Fix potential broken CSS in styled-components#9234
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule

Conversation

@billyvg

@billyvgbillyvg commented Oct 12, 2023

Copy link
Copy Markdown
Member

Fixes an issue where the Replay integration can potentially break applications that use styled-components. styled-componentsrelies on an exception being throw for CSS rules that are not supported by the browser engine. However, our SDK suppressed any exceptions thrown from within rrweb, so styled-components assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.

This was a regression from v1 as we were always re-throwing the exception

Fixes#9170

@billyvg
billyvg marked this pull request as ready for review October 12, 2023 18:28
@github-actions

github-actionsBot commented Oct 12, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)84.26 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.43 KB (0%)
@sentry/browser - Webpack (gzipped)22.02 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)78.78 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.61 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)21.02 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)254.48 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.76 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)62.45 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.48 KB (-0.01% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)84.29 KB (-0.01% 🔽)
@sentry/react - Webpack (gzipped)22.06 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)102.2 KB (-0.01% 🔽)
@sentry/nextjs Client - Webpack (gzipped)50.96 KB (0%)

Comment threadpackages/replay/src/integration.ts Outdated
@AbhiPrasad

Copy link
Copy Markdown
Contributor

We can do a release tomorrow with this.

Comment threadpackages/replay/src/integration.ts Outdated
// return true to suppress throwing the error inside of rrweb
return true;
errorHandler: (err: Error & {__rrweb__?: boolean}) => {
err.__rrweb__ = true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm so one problem here is that we may throw/error on trying to set this on something that is e.g. frozen or similar. Can we instead still try-catch here and just re-throw certain kinds of exceptions, maybe?

I mean, maybe this is actually fine, but we could be getting other errors because of this, at some point, too 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually, can we just do:

errorHandler: (err: Error)=>{try{// @ts-expect-error Set this so that replay SDK can ignore errors originating from rrweberr.__rrweb__=true;}catch{// avoid any potential hazards here}

So still try-catch setting this prop, but bubble the error up to rrweb 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this like this, so we can merge & release this!

billyvgand others added 4 commits October 13, 2023 10:12
NOTE: This requires a bump to rrweb library
Fixes an issue where the Replay integration can potentially break applications that use `styled-components`. `styled-components` [relies on an exception being throw](https://github.com/styled-components/styled-components/blob/b7b374bb1ceff1699f7035b15881bc807110199a/packages/styled-components/src/sheet/Tag.ts#L32-L40) for CSS rules that are not supported by the browser engine. However, our SDK suppresses any exceptions thrown from within rrweb, so `styled-components` assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.
getsentry/rrweb#111 introduces a change the adds metadata to exceptions that occur when calling `insertRule`, and this PR will re-throw those exceptions that will then bubble up to `styled-components`.
Fixes#9170
@mydea
mydeaforce-pushed the fix-replay-fix-catching-exceptions-from-insertRule branch from b1876c5 to a66168fCompareOctober 13, 2023 08:16
@mydea
mydea merged commit 73a808a into developOct 13, 2023
@mydea
mydea deleted the fix-replay-fix-catching-exceptions-from-insertRule branch October 13, 2023 09:14
Sign up for freeto 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.

Sentry 7.73.0 causes styled-components to fail to render with styles/CSS

3 participants

@billyvg@AbhiPrasad@mydea
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(replay): Fix potential broken CSS in styled-components - #9234

Merged
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule
Oct 13, 2023
Merged

fix(replay): Fix potential broken CSS in styled-components#9234
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule

Conversation

@billyvg

@billyvgbillyvg commented Oct 12, 2023

Copy link
Copy Markdown
Member

Fixes an issue where the Replay integration can potentially break applications that use styled-components. styled-componentsrelies on an exception being throw for CSS rules that are not supported by the browser engine. However, our SDK suppressed any exceptions thrown from within rrweb, so styled-components assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.

This was a regression from v1 as we were always re-throwing the exception

Fixes#9170

@billyvg
billyvg marked this pull request as ready for review October 12, 2023 18:28
@github-actions

github-actionsBot commented Oct 12, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)84.26 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.43 KB (0%)
@sentry/browser - Webpack (gzipped)22.02 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)78.78 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.61 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)21.02 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)254.48 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.76 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)62.45 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.48 KB (-0.01% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)84.29 KB (-0.01% 🔽)
@sentry/react - Webpack (gzipped)22.06 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)102.2 KB (-0.01% 🔽)
@sentry/nextjs Client - Webpack (gzipped)50.96 KB (0%)

Comment threadpackages/replay/src/integration.ts Outdated
@AbhiPrasad

Copy link
Copy Markdown
Contributor

We can do a release tomorrow with this.

Comment threadpackages/replay/src/integration.ts Outdated
// return true to suppress throwing the error inside of rrweb
return true;
errorHandler: (err: Error & {__rrweb__?: boolean}) => {
err.__rrweb__ = true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm so one problem here is that we may throw/error on trying to set this on something that is e.g. frozen or similar. Can we instead still try-catch here and just re-throw certain kinds of exceptions, maybe?

I mean, maybe this is actually fine, but we could be getting other errors because of this, at some point, too 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually, can we just do:

errorHandler: (err: Error)=>{try{// @ts-expect-error Set this so that replay SDK can ignore errors originating from rrweberr.__rrweb__=true;}catch{// avoid any potential hazards here}

So still try-catch setting this prop, but bubble the error up to rrweb 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this like this, so we can merge & release this!

billyvgand others added 4 commits October 13, 2023 10:12
NOTE: This requires a bump to rrweb library
Fixes an issue where the Replay integration can potentially break applications that use `styled-components`. `styled-components` [relies on an exception being throw](https://github.com/styled-components/styled-components/blob/b7b374bb1ceff1699f7035b15881bc807110199a/packages/styled-components/src/sheet/Tag.ts#L32-L40) for CSS rules that are not supported by the browser engine. However, our SDK suppresses any exceptions thrown from within rrweb, so `styled-components` assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.
getsentry/rrweb#111 introduces a change the adds metadata to exceptions that occur when calling `insertRule`, and this PR will re-throw those exceptions that will then bubble up to `styled-components`.
Fixes#9170
@mydea
mydeaforce-pushed the fix-replay-fix-catching-exceptions-from-insertRule branch from b1876c5 to a66168fCompareOctober 13, 2023 08:16
@mydea
mydea merged commit 73a808a into developOct 13, 2023
@mydea
mydea deleted the fix-replay-fix-catching-exceptions-from-insertRule branch October 13, 2023 09:14
Sign up for freeto 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.

Sentry 7.73.0 causes styled-components to fail to render with styles/CSS

3 participants

@billyvg@AbhiPrasad@mydea
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(replay): Fix potential broken CSS in styled-components - #9234

Merged
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule
Oct 13, 2023
Merged

fix(replay): Fix potential broken CSS in styled-components#9234
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule

Conversation

@billyvg

@billyvgbillyvg commented Oct 12, 2023

Copy link
Copy Markdown
Member

Fixes an issue where the Replay integration can potentially break applications that use styled-components. styled-componentsrelies on an exception being throw for CSS rules that are not supported by the browser engine. However, our SDK suppressed any exceptions thrown from within rrweb, so styled-components assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.

This was a regression from v1 as we were always re-throwing the exception

Fixes#9170

@billyvg
billyvg marked this pull request as ready for review October 12, 2023 18:28
@github-actions

github-actionsBot commented Oct 12, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)84.26 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.43 KB (0%)
@sentry/browser - Webpack (gzipped)22.02 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)78.78 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.61 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)21.02 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)254.48 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.76 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)62.45 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.48 KB (-0.01% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)84.29 KB (-0.01% 🔽)
@sentry/react - Webpack (gzipped)22.06 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)102.2 KB (-0.01% 🔽)
@sentry/nextjs Client - Webpack (gzipped)50.96 KB (0%)

Comment threadpackages/replay/src/integration.ts Outdated
@AbhiPrasad

Copy link
Copy Markdown
Contributor

We can do a release tomorrow with this.

Comment threadpackages/replay/src/integration.ts Outdated
// return true to suppress throwing the error inside of rrweb
return true;
errorHandler: (err: Error & {__rrweb__?: boolean}) => {
err.__rrweb__ = true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm so one problem here is that we may throw/error on trying to set this on something that is e.g. frozen or similar. Can we instead still try-catch here and just re-throw certain kinds of exceptions, maybe?

I mean, maybe this is actually fine, but we could be getting other errors because of this, at some point, too 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually, can we just do:

errorHandler: (err: Error)=>{try{// @ts-expect-error Set this so that replay SDK can ignore errors originating from rrweberr.__rrweb__=true;}catch{// avoid any potential hazards here}

So still try-catch setting this prop, but bubble the error up to rrweb 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this like this, so we can merge & release this!

billyvgand others added 4 commits October 13, 2023 10:12
NOTE: This requires a bump to rrweb library
Fixes an issue where the Replay integration can potentially break applications that use `styled-components`. `styled-components` [relies on an exception being throw](https://github.com/styled-components/styled-components/blob/b7b374bb1ceff1699f7035b15881bc807110199a/packages/styled-components/src/sheet/Tag.ts#L32-L40) for CSS rules that are not supported by the browser engine. However, our SDK suppresses any exceptions thrown from within rrweb, so `styled-components` assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.
getsentry/rrweb#111 introduces a change the adds metadata to exceptions that occur when calling `insertRule`, and this PR will re-throw those exceptions that will then bubble up to `styled-components`.
Fixes#9170
@mydea
mydeaforce-pushed the fix-replay-fix-catching-exceptions-from-insertRule branch from b1876c5 to a66168fCompareOctober 13, 2023 08:16
@mydea
mydea merged commit 73a808a into developOct 13, 2023
@mydea
mydea deleted the fix-replay-fix-catching-exceptions-from-insertRule branch October 13, 2023 09:14
Sign up for freeto 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.

Sentry 7.73.0 causes styled-components to fail to render with styles/CSS

3 participants

@billyvg@AbhiPrasad@mydea
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(replay): Fix potential broken CSS in styled-components - #9234

Merged
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule
Oct 13, 2023
Merged

fix(replay): Fix potential broken CSS in styled-components#9234
mydea merged 4 commits into
developfrom
fix-replay-fix-catching-exceptions-from-insertRule

Conversation

@billyvg

@billyvgbillyvg commented Oct 12, 2023

Copy link
Copy Markdown
Member

Fixes an issue where the Replay integration can potentially break applications that use styled-components. styled-componentsrelies on an exception being throw for CSS rules that are not supported by the browser engine. However, our SDK suppressed any exceptions thrown from within rrweb, so styled-components assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.

This was a regression from v1 as we were always re-throwing the exception

Fixes#9170

@billyvg
billyvg marked this pull request as ready for review October 12, 2023 18:28
@github-actions

github-actionsBot commented Oct 12, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)84.26 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - Webpack (gzipped)31.43 KB (0%)
@sentry/browser - Webpack (gzipped)22.02 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)78.78 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)28.61 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped)21.02 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)254.48 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)86.76 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)62.45 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)31.48 KB (-0.01% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)84.29 KB (-0.01% 🔽)
@sentry/react - Webpack (gzipped)22.06 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)102.2 KB (-0.01% 🔽)
@sentry/nextjs Client - Webpack (gzipped)50.96 KB (0%)

Comment threadpackages/replay/src/integration.ts Outdated
@AbhiPrasad

Copy link
Copy Markdown
Contributor

We can do a release tomorrow with this.

Comment threadpackages/replay/src/integration.ts Outdated
// return true to suppress throwing the error inside of rrweb
return true;
errorHandler: (err: Error & {__rrweb__?: boolean}) => {
err.__rrweb__ = true;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm so one problem here is that we may throw/error on trying to set this on something that is e.g. frozen or similar. Can we instead still try-catch here and just re-throw certain kinds of exceptions, maybe?

I mean, maybe this is actually fine, but we could be getting other errors because of this, at some point, too 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually, can we just do:

errorHandler: (err: Error)=>{try{// @ts-expect-error Set this so that replay SDK can ignore errors originating from rrweberr.__rrweb__=true;}catch{// avoid any potential hazards here}

So still try-catch setting this prop, but bubble the error up to rrweb 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this like this, so we can merge & release this!

billyvgand others added 4 commits October 13, 2023 10:12
NOTE: This requires a bump to rrweb library
Fixes an issue where the Replay integration can potentially break applications that use `styled-components`. `styled-components` [relies on an exception being throw](https://github.com/styled-components/styled-components/blob/b7b374bb1ceff1699f7035b15881bc807110199a/packages/styled-components/src/sheet/Tag.ts#L32-L40) for CSS rules that are not supported by the browser engine. However, our SDK suppresses any exceptions thrown from within rrweb, so `styled-components` assumes that an unsupported rule was inserted successfully and increases a rule index, which causes following inserted rules to fail due to an out-of-bounds error.
getsentry/rrweb#111 introduces a change the adds metadata to exceptions that occur when calling `insertRule`, and this PR will re-throw those exceptions that will then bubble up to `styled-components`.
Fixes#9170
@mydea
mydeaforce-pushed the fix-replay-fix-catching-exceptions-from-insertRule branch from b1876c5 to a66168fCompareOctober 13, 2023 08:16
@mydea
mydea merged commit 73a808a into developOct 13, 2023
@mydea
mydea deleted the fix-replay-fix-catching-exceptions-from-insertRule branch October 13, 2023 09:14
Sign up for freeto 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.

Sentry 7.73.0 causes styled-components to fail to render with styles/CSS

3 participants

@billyvg@AbhiPrasad@mydea