fix(sdk): filter third-party script errors out of the frontend SDK - #121862
fix(sdk): filter third-party script errors out of the frontend SDK#121862JoshuaKGoldberg wants to merge 1 commit into
Conversation
Vendor snippets and browser extensions throw on our pages without any frame identifying them as foreign. Injected scripts have no file of their own, so the browser attributes their frames to the document URL and the event reads as ours. Drop an error when no frame in its stack comes from code we serve, and when any frame names a source we know is not ours. Origins are derived from the page and the asset prefix, so no vendor list needs maintaining. A stack with no frames at all is kept, since it cannot be told apart from a real error that lost its stack. Refs DE-1511
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 843e2b6. Configure here.
| it('returns true when every frame is anonymous', () => { | ||
| const event = eventWithFrames( | ||
| '<anonymous>', | ||
| '<anonymous>', | ||
| '<anonymous>', | ||
| '<anonymous>' | ||
| ); | ||
|
|
||
| expect(isThirdPartyScriptEvent(event, ORIGINS)).toBeTruthy(); | ||
| }); |
There was a problem hiding this comment.
Are we sure that this is what we want? are there any cases like fetch() or service-worker; i'm thinking something more event driven where we control the event to an extent but not the early handler code?
There was a problem hiding this comment.
I don't follow - do you mean like if we don't know anything, we should assume it's 100% foreign?
There was a problem hiding this comment.
Well idk what's best... my understanding is that built-in browser code is going to produce these <anonymous> frames?
So i was trying to think of reasonable situations where we'd have a fully anon stack and what that means.
i guess if there's an example of this that we've already captured then i'd look at that, but if we've never caught this case before (which is likely too) then i'm not worried.
| it('returns true when frames mix extension, vendor, and anonymous code', () => { | ||
| const event = eventWithFrames( | ||
| 'chrome-extension://abcdefg/content.js', | ||
| 'https://cdn.example.com/agent.js', | ||
| '<anonymous>', | ||
| 'https://sentry.io/settings/' | ||
| ); | ||
|
|
||
| expect(isThirdPartyScriptEvent(event, ORIGINS)).toBeTruthy(); | ||
| }); |
There was a problem hiding this comment.
This makes sense. extensions can use sentry themselves, which might make it look like our code. But because sandboxing those call stacks cannot interact with react et. al. on the page.
| if ( | ||
| isFilteredRequestErrorEvent(event) || | ||
| isEventWithFileUrl(event) || | ||
| isThirdPartyScriptEvent(event, firstPartyOrigins) |
There was a problem hiding this comment.
We gotta call out the difference between the new code and thirdPartyErrorFilterIntegration
|
I did a bit more digging and I'm seeing basically the same / slightly better results from Using thirdPartyErrorFilterIntegration -> |
Like #121862, but using our own product per its docs. 🥇 See: [Using `thirdPartyErrorFilterIntegration`](https://docs.sentry.io/platforms/javascript/configuration/filtering/#using-thirdpartyerrorfilterintegration) -> `'drop-error-if-contains-third-party-frames'`. Closes DE-1511.
Note
Superseded by #121966.
Right now, we don't differentiate between crashes that are 100% in our own code vs. crashes that are partially (or completely!) third-party code. For example, if a browser extension throws an exception, we track that in our issues. That's a lot of annoying false noise.
beforeSendnow drops an event when the stack has:Note that this is essentially a more powerful Using
thirdPartyErrorFilterIntegration. We'll not just filter out events including third parties (i.e.drop-error-if-contains-third-party-frames), we'll also get ones with no parties. We already usethirdPartyErrorFilterIntegration:sentry/static/app/bootstrap/initializeSdk.tsx
Line 77 in 843e2b6
Based on the attached
measure_third_party_filter.py, the changes within all browser-SDK events in thejavascriptproject are:If this goes in roughly as-is, I'll try to bring this up with the SDK team to see if we can make this a browser SDK feature. This problem is definitely not unique to us.
Closes DE-1511.