Sentry 9.2 ignores allowUrls and reports errors anyway

[patroza]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

9.2.0

Framework Version

No response

Link to Sentry event

https://macs-holding.sentry.io/issues/6317718880/events/d9099849308c471aab0ac0a2c05ed783/?project=4504671195299840

Reproduction Example/SDK Setup

Sentry.init({
  ...,
  allowUrls: ["https://" + window.location.host]
})

Steps to Reproduce

Error happens in 3rd party script

Expected Result

No report

Actual Result

Got report

[lforst]

The event you linked is on version 9.1.0 but in your issue you state 9.2.0 has an issue. Is it possible the error is coming from an old deployment/old site on somebody's device?

[patroza]

@lforst the behavior is equal: https://macs-holding.sentry.io/issues/6317718880/events/latest/?project=4504671195299840&referrer=latest-event

[lforst]

Alright, we gotta dig deeper. Would you mind sharing the rest of your Sentry init? Thanks!

[lforst]

Oh damn I found the issue but it's super fucky and I would have never thought of this. Basically, the issues you linked contain two "linked" exceptions. The root exception is the one you wanted to ignore and the other one (its cause) is an AbortError. In the logic in the SDK we always only check the first recorded exception which in this case is the cause, which doesn't have a stack trace.

We'll do two things:

• I am gonna adjust the logic for allowUrls to account for this. Haven't decided yet on whether we should make it depend on the top frame of all linked exceptions, or whether we should only apply it to the "main" exception.
• I will talk with the UI team to actually show the "linked" exceptions if they don't contain the stacktrace. This is just garbage UX otherwise.

[patroza]

thanks @lforst - it's been let's say.. challenging to get to a "just alert me about the things im interested in" situation..
looking forward putting this finally behind us

[lforst]

Getting rid of noise is a big topic for us. Unfortunately, it's tricky since browsers are surprisingly uncontrolled environments and there are no conventions around letting website owners know that errors come from in-app-browser code, browser extensions, user scripts, ....

If you haven't seen it, you might wanna give this a shot: https://docs.sentry.io/platforms/javascript/configuration/filtering/#using-thirdpartyerrorfilterintegration

[github-actions]

A PR closing this issue has just been released 🚀

This issue was referenced by PR #15521, which was included in the 9.3.0 release.

[patroza]

If you haven't seen it, you might wanna give this a shot: https://docs.sentry.io/platforms/javascript/configuration/filtering/#using-thirdpartyerrorfilterintegration

Thanks! This is really great 👍

[patroza]

If you haven't seen it, you might wanna give this a shot: https://docs.sentry.io/platforms/javascript/configuration/filtering/#using-thirdpartyerrorfilterintegration

@lforst It worked nicely, and then it didn't, why is this not tagged?
https://macs-holding.sentry.io/issues/6317718880/events/latest/?alert_rule_id=15804510&alert_type=issue&notification_uuid=4de87b5f-eeaa-4c91-aca4-bfc5dfbb188f&project=4504671195299840&referrer=latest-event

I've already changed the setting to:
behaviour: "apply-tag-if-contains-third-party-frames"
but no dice :(

[lforst]

@patroza looking at the release of that event it's from a day ago. Is it possible that this is from a client that still has old code loaded and that release didn't entail your changes with the integration yet?

[patroza]

@lforst no that was the first thing I checked, that release is 2 commits later than the behaviour change.
It feels like since 9.3.0 what worked before stopped working.

[patroza]

it actually seems inverted now: the following is internal code but marked third party
https://macs-holding.sentry.io/issues/6216431126/events/latest/?project=4504671195299840&query=third_party_code%3ATrue&referrer=latest-event&statsPeriod=7d&stream_index=0

https://macs-holding.sentry.io/issues/6024909061/events/latest/?project=4504671195299840&query=third_party_code%3ATrue&referrer=latest-event&statsPeriod=7d&stream_index=16

unless node_modules are now also considered third party, in which case we definitely can't use "if-contains-third-party-frames".

[lforst]

@patroza for this integration, never look at the unminified stack trace. All of the logic runs before symbolication can happen. So node_modules are irrelevant in the browser.

For the first one, there are frames with weird filenames (the request and query ones at the bottom). I have no idea how they come to be, I need to assume some of your code is responsible for spitting these in the stack trace. These can of course not be mapped to a filename and are thus considered 3rd party frames.

Same for the second one.

[lforst]

At this point, I should note that this integration is very much best effort and was an experiment of mine. There are false negatives. I think the integration can help at narrowing down, but should be taken with a grain of salt. Maybe I need to put this a bit more prominent in the docs. All of this is very confusing.

I sometimes hate the browser environment because it creates so much noise. Web citicenship of extensions and in app browsers are garbo.

[patroza]

@lforst ah interesting, yes, they are Observability spans of Effect.
can we somehow control the third party filter more from our side?
we would consider perhaps / and protocol:// to be considered third party, not other custom stackframes.

[lforst]

We could add an option/hook to the thirdPartyErrorFilterIntegration that allows iterating through the stack frames of an error and marking individual ones as third party based on custom logic.

We currently don't have the capacity to work on new features ourselves but feel free to contribute. The code is here:

sentry-javascript/packages/core/src/integrations/third-party-errors-filter.ts

Line 39 in 6628ad8

export const thirdPartyErrorFilterIntegration = defineIntegration((options: Options) => {

[patroza]

@lforst thanks, can you at least check why https://macs-holding.sentry.io/issues/6317718880/events/9b2d6ee213a24a4dba727f03e4c7769c/
is not tagged by third party filter?
between 9.3 and 9.2 the report changed from:
"RecordFetchError: Tolgee: Failed to fetch record for "de" and "badge_widget""
to:
"TypeError: NetworkError when attempting to fetch resource."

it seems the stack trace got inverted, perhaps as part of the allowUrls fix?
It does not seem to be related to not getting tagged however, as neither were tagged.

Oh and it didn't get filtered in neither "contains" or "only-contains" modes.

[patroza]

or wait here are two tagged..
https://macs-holding.sentry.io/issues/6344600316/events/b9bd31a4b77e4a849b1890346dddf3b4/
https://macs-holding.sentry.io/issues/6344600316/events/2bd3e89bbfbc41c280ea86bb409b8b62/

[lforst]

As soon as there is an error without stack frames, we don't tag it, as we cannot know whether it is your code or external code.

I'll also look at the inversion. That actually looks pretty bad.