Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sentry JavaScript SDK ignoreErrors option not documented #1137

Closed
MilllerTime opened this issue Jul 23, 2019 · 42 comments
Closed

Sentry JavaScript SDK ignoreErrors option not documented #1137

MilllerTime opened this issue Jul 23, 2019 · 42 comments

Comments

@MilllerTime
Copy link

I'm currently migrating from raven-js to @sentry/browser, and I've hit a few snags figuring out how to map my old config to the new config. (I searched high and low for a migration guide that covered all raven options and how to implement them with the new SDK, but couldn't find anything like that. If I missed something, I'd love to know!)

One such option was ignoreErrors, which seemingly disappeared. I eventually found this old issue which suggested it was still an option, but the referenced file didn't exist on master and that issue was closed 9 months ago. I did some digging and found the code here:

https://github.com/getsentry/sentry-javascript/blob/de9b23ac9b22284dca4e55d33678edb447180a49/packages/types/src/options.ts#L40-L44

So it looks like the option still exists and is usable just as before, but it is not documented. Is there any reason I shouldn't be using this option any more, or does it just need to be added to the new docs?

FWIW, my use case is ignoring errors from random plugins/extensions via:

ignoreErrors: ['top.GLOBALS']

While this migration overall felt a bit rough, Sentry is doing so many cool things. Thank you!

@kamilogorek kamilogorek transferred this issue from getsentry/sentry-javascript Jul 30, 2019
@kamilogorek kamilogorek changed the title ignoreErrors option not documented Sentry JavaScript SDK ignoreErrors option not documented Jul 30, 2019
@kamilogorek
Copy link
Contributor

Hey, agree that it wasn't perfect, but it was very hard to keep track everything with the number of changes we introduced.

Errors ignoring is now part of the integration called InboundFilters - https://docs.sentry.io/platforms/javascript/#inboundfilters

However, as you already realized, we kept top-level config options so it's kinda easier to migrate them.

Is there any reason I shouldn't be using this option any more, or does it just need to be added to the new docs?

No, it's totally fine.

We are in the process of rewriting/refining our docs for all the SDKs and this effort will take a while, but we'll eventually get there. Thanks for pointing this issue out, we'll make sure it's reflected during an update.

While this migration overall felt a bit rough, Sentry is doing so many cool things. Thank you!

Thanks!

@MilllerTime
Copy link
Author

I appreciate the follow up! Glad to hear this is being worked on.

Errors ignoring is now part of the integration called InboundFilters - https://docs.sentry.io/platforms/javascript/#inboundfilters

The fact this explanation mentions ignoreErrors, blacklistUrls, and whitelistUrls makes sense now that I know they still exist. However since those terms don't link anywhere and do not appear anywhere else on that page, if I didn't know better I'd be pretty confused about what that meant / how those options work.

However, as you already realized, we kept top-level config options so it's kinda easier to migrate them.

This point alone may be great for the docs. Knowing the specific configuration options from raven-js that will continue to "just work" with the new SDK would have been invaluable. For each option, a link to any of the new integrations that are relevant would make for a friendly introduction to the new APIs for anyone who has used raven-js.

I didn't know any of this, so I ended up studying the new docs pretty thoroughly in order to build a mental model of the new architecture so I could confidently reimplement what I was doing with raven-js. The knowledge is nice, but since it was essentially a brute-force approach, it did take more time than I hoped.

@lukeburden
Copy link

I also found the migration pretty rough to the new JavaScript SDK - specifically also around ignoring the blast of useless errors or errors not occurring in your own JavaScript.

@kamilogorek great to hear you're updating your docs - might I also suggest you update several blog posts? These were excellent when Raven.js was a thing, but are now more confusing than helpful for people. The topics are still highly relevant though so they could just use an update.

https://blog.sentry.io/2017/03/27/tips-for-reducing-javascript-error-noise
https://blog.sentry.io/2016/05/17/what-is-script-error

Thanks for your efforts!

@kamilogorek
Copy link
Contributor

@lukeburden sure! I checked both of those links and the first one has a large "outdated" banner which points to https://docs.sentry.io/platforms/javascript/ And when you Ctrl+F for whitelistUrls (basically first line of code in the blogpost) you'll be able to find the answer right away. Same goes for ignoreErrors.
And second blog-post doesn't have any code-snippets 😅

Is there anything we could do better there?

@lukeburden
Copy link

@kamilogorek Right, I saw the banner - let me rephrase.

The topics covered by those posts are still relevant today to usage of your current SDK. Keeping track of browser-based errors is horribly noisy without some sensible use of whitelisting and/or ignoring. So rather than presenting these as outdated posts, I'm suggesting they be cleanly updated so they go seamlessly hand with your current product offering.

@lukeburden
Copy link

Ie, probably not mentioning Raven.js .. or at least no more than to say "these posts were originally written when Raven.js was a thing and have been adapted - upgrade if you're using Raven.js!"

@kamilogorek
Copy link
Contributor

@lukeburden will let appropriate people know! :)

@deleteeeeeeeeeeeed
Copy link

no syntax for test ???

@justinperkins
Copy link

Came here from searching after having 2 back-to-back months of spiked events due to the lack of this information in the SDK docs. I can't figure out how to stop pointless events (that we don't care about) from going into Sentry. We're constantly in spike protection mode. This has been a really frustrating experience ever since we added Sentry to our application frontends.

@MilllerTime
Copy link
Author

@justinperkins Yeah the docs still seem to be a little confusing around this, compared to the old raven-js docs. There is an ignoreErrors config option that still works great though. If you're getting a bunch of errors with a specific message, you can filter them out like this:

{
  ignoreErrors: ['fb_xd_fragment', 'another_error']
}

If you're getting a bunch of errors from other domains, you can easily filter those out by whitelisting only the domains you care about. The example below also includes any subdomains.

{
  whitelistUrls: [
    /(?:[a-z]+\.)?yourdomain\.com/
  ]
}

The reality of the situation is errors can be messy, especially if you haven't written all the code yourself. I've personally taken a few passes at building a custom config that works best for a specific app. It's required some creativity, both in the app code and in the Sentry config, and it's still not 100% perfect but it's pretty close. I believe it's normal for this to require some work. Hopefully the docs keep improving in the meantime.

@justinperkins
Copy link

Thanks @MilllerTime, we actually have ignoreErrors configured to ignore our most common errors like this:

    ignoreErrors: [
      "Unable to get property 'attr' of undefined or null reference",
      "e.cssHooks[i].set is not a function",
      // from iframes attempting to access parent window. IE/Edge only
      "Can't execute code from a freed script"
    ],

The above code has been in our production environment for over a month, but unfortunately we still are receiving Can't execute code from a freed script errors (as many as 321k as of right now) as recently as 4 hours ago. It's really frustrating because I've done everything the docs are telling me to do, including contacting support to get assistance with our paid account. Not to mention all the work searching around for help, using their forums (ghost town) and lots of local debugging.

I'm ok with things taking work, but either I'm missing something really obvious, or I'm in crazytown. It shouldn't be this much work.

@MilllerTime
Copy link
Author

@justinperkins That sure sounds frustrating. I don't know what's going on, but as an alternative solution, can you catch those errors in your app (and don't re-throw) to prevent them from bubbling up to window.onerror?

@justinperkins
Copy link

justinperkins commented Apr 6, 2020

The "Can't execute code ..." is coming from a third party script (Intercom) that has added an iframe to the page, and that iframe is attempting to access the parent window. We don't have control over the code or the ability to catch it.

I'm guessing we need to implement something custom using beforeSend. That has come up in my searches but it sure feels strange that there's seemingly two ways to do the same thing. I bet we'll have better luck with that one though.

@MilllerTime
Copy link
Author

Some more thoughts, then I think I'm out of ideas:

  • If the error is coming from a script/iframe loaded from another domain, can you block errors from that domain with whitelistUrls or blacklistUrls?
  • Not sure if you're getting many repeat errors from each user, or you just have a lot of users, but if it's the former it's probably worth adding the following integration if you don't already have it:
import * as Integrations from '@sentry/integrations';

Sentry.init({
  [...],
  integrations: [
    new Integrations.Dedupe()
  ]
});

I'm honestly surprised your paid support has not helped with this, as that's definitely a more appropriate channel than this GitHub issue.

@justinperkins
Copy link

Thanks @MilllerTime, we'll check that out. I've got a working solution now, this is what I needed to do:

  const ignoreErrors = [
    "Can't execute code from a freed script"
  ]

  Sentry.init({
    beforeSend(event, hint) {
      if (event && event.message && ignoreErrors.includes(event.message)){
        return null
      }
      
      return event
    },
    whitelistUrls: [
      /https:\/\/www.company.(com|net)/
    ],
    ignoreErrors: ignoreErrors,
    dsn: process.env.RAVEN_JS_URL,
    environment: process.env.RAILS_ENV
  });

At this point, I don't get the impression ignoreErrors is doing anything for us, but I'm passing in the ignore array JIC.

Oh, and this is what paid support recommended: ignoreErrors: [Can't execute code from a freed script] 😂

@MilllerTime
Copy link
Author

Great idea using beforeSend to filter yourself, but yeah that's exactly what ignoreErrors should do for you. Weird. Also ooof, that recommendation is... not even valid code 🤦‍♂😂

@kamilogorek
Copy link
Contributor

If one of you can provide a reproducible example, where ignoreErrors doesn't do its job, I'm more than happy to take a look at it :)

Oh, and this is what paid support recommended: ignoreErrors: [Can't execute code from a freed script] 😂
Also ooof, that recommendation is... not even valid code 🤦‍♂😂

It's just missing quotes, don't be so snarky.

@justinperkins
Copy link

If one of you can provide a reproducible example, where ignoreErrors doesn't do its job, I'm more than happy to take a look at it :)

I don't have anything reproducable at the moment besides my latest Sentry init snippet above, but it's been in production for over a week, and unfortunately we're still getting the exception events coming in. I have spent many days on this and it's not worth it. We're pulling Sentry from our frontends in our next release.

This screenshot is from this morning, the code to ignore the event has been in place since I posted my last comment above (7 days).

sentry-event

@kevich
Copy link

kevich commented Apr 23, 2020

@justinperkins @kamilogorek

Same for us here, we were trying to exclude Can't execute code from a freed script error with ignoreErrors but this seems to be not working sometimes.

I know how to reproduce error in IE11, but when I do this on my instance - I do not have any sent events for this error to sentry, as it expected to be. But seems other users still send this error to sentry.

@kamilogorek
Copy link
Contributor

Hey @kevich, can you provide the repro here?

I know how to reproduce error in IE11

@justinperkins
Copy link

justinperkins commented Apr 23, 2020

Funny this was brought up again, turns out we're still getting events into Sentry despite everything we've done to suppress them. We're deploying this soon as a last ditch effort, if it doesn't work, we're abandoning Sentry completely, been a huge waste of time.

  Sentry.init({
    dsn: process.env.RAVEN_JS_URL,
    environment: process.env.RAILS_ENV,
    // disable global error capturing!
    // sentry is way too eager for our tastes
    onunhandledrejection: false,
    defaultIntegrations: false
  });

The onunhandledrejection is new. We deployed without it this week, but with the defaultIntegrations config set to false, and that still didn't do the trick.

@kamilogorek
Copy link
Contributor

There's no such thing as onunhandledrejection in our configuration options. defaultIntegrations: false will stop all handlers from attaching, so it'll be enough.

@justinperkins
Copy link

There's no such thing as onunhandledrejection in our configuration options. defaultIntegrations: false will stop all handlers from attaching, so it'll be enough.

It is not enough. I misread this documentation. I will put some more time into figuring it out I guess.

@lobsterkatie
Copy link
Member

lobsterkatie commented Apr 23, 2020

@justinperkins Can you please post a link to your Can’t execute code from a freed script issue in Sentry? Also, can you please include a release value in your Sentry.init(), so we can be extra extra sure that events which are coming through are coming from code that has defautlIntegrations: false in it? Finally, can you confirm that your beforeSend solution worked to block this error?

(FWIW, I have heard of this particular error somehow bypassing ignoreErrors for others as well, and if in fact you're correct that turning off default integrations still doesn't block it, then IE is doing something fishy... though at this point I have no idea what.)

@kevich
Copy link

kevich commented Apr 24, 2020

@kamilogorek I cannot share repo directly, since it's our customer sensitive data. But I will try to reproduce with some simpler example later on.

Not sure that I dig out fully, but for now it seems that the error happens only when sentry is attached to project. Without it the error does not happen. And the reason for this look like this:

  • Let's assume that we have some ads network, that puts itself code into iframe with no src.

  • Then at some point I ask ad network to reload banner, which leads to iframe node removal, and creation of new one.

  • After that Sentry caughts error while trying to evaluate code, which was within this removed iframe node
    image

  • The function called in this error seems to be addEventListener()

@kevich
Copy link

kevich commented Apr 24, 2020

I've made pretty simple example with this case (you need to press refresh button once or twice to reload iframe script to cause an error):
http://kevich.ru/sentryTest.html

So it really happening as I described before. Sentry wraps all event handlers to it's own handlers, but doesn't handle right if the wrapped script is gone.

Here's the small video demo how this happens in IE:
https://monosnap.com/file/XnVigoj0C0oU7Yi46K3igFDssqRl5T

And last not yet resolved thing - why Sentry sometimes catches this errors with ignoreErrors but for some users - not. And still could not reproduce locally so that even with ignoreErrors it will send it to Sentry.

And without sentry no errors occurs at all in IE11:
http://kevich.ru/test.html

@kamilogorek
Copy link
Contributor

kamilogorek commented Apr 24, 2020

It is not enough.

@justinperkins when you set defaultIntegrations: false, our SDK will capture and send events only when you call Sentry.captureException or Sentry.captureMessage yourself. There's no other mechanism in place that would capture anything for you when integrations are turned off.


@kevich thanks, that's very useful. I did some digging and the issue here is as you described it. However, underlying problem is not SDK itself, but lack of removeEventListener code on the iframe side.
If you remove the SDK completely and place a simple console.log or window.foo = 'bar' in your iframe event handler, you'll still get errors. It didn't surface because it was empty and was optimized away.

image


I also checked the ignoreErrors configuration, and both Edge and IE11 are filtering this error correctly, see:
Screenshot 2020-04-24 at 12 59 58

Network tab is empty and all logs are confirming event drop.


There should also be __serialized__ field attached to your events, which will tell you what type of event and to what target it was attached, which should help to find offending handler.

@kevich
Copy link

kevich commented Apr 24, 2020

@kamilogorek Thanks for further digging!

If you remove the SDK completely and place a simple console.log or window.foo = 'bar' in your iframe event handler, you'll still get errors

Yep, seems if there will be some interaction with window, then there will be problems. But seems our third party ad server only stores events to their side, with no interactions with window object. I've modified badScript.js at http://kevich.ru/badScript.js which now stores events on it's own objects to use them if needed for example. So in this case without a sentry there won't be any problems, but with Sentry still will be. But yeah, of course it's kind of related to not unbinded event handler.

I also checked the ignoreErrors configuration, and both Edge and IE11 are filtering this error correctly

Yeah, that thing I could not figure out. When error first came in, we've added Can’t execute code from a freed script to ignoreErrors, checked on my local instance, and confirmed, that error happens, but does not sent to Sentry itself. So I thought that problem is solved. But after release of this exclusion, I still received errors like this, same as @justinperkins. So on our side it seems to be solved (with self browser testing), but some users still spam us with requests. But this part unfortunately I could not yet reproduce...

There should also be serialized field attached to your events, which will tell you what type of event and to what target it was attached, which should help to find offending handler.

Yes, I already know the source of this event handler, but it's some third party script of ad network =) If it would be our code, we've could fix this, but for now I've only sent this info about not proper unbinding of event to this ad service.

That's why in my example was not "perfect code", but some interpolation of what external ad network script does =)

@justinperkins
Copy link

justinperkins commented Apr 24, 2020

@justinperkins when you set defaultIntegrations: false, our SDK will capture and send events only when you call Sentry.captureException or Sentry.captureMessage yourself. There's no other mechanism in place that would capture anything for you when integrations are turned off.

Our own data in our Sentry frontend project says otherwise. We deployed that change on Tuesday and we've received "Can't execute code from a freed script" events as recently as a few hours ago (326k to-date since we added Sentry to our frontends). We have 4 frontends and I've confirmed they all have the latest code. I can look into adding a release to the init code.

I can't share a link to our private events publicly, but happy to do so via email or something else. LMK

@kamilogorek
Copy link
Contributor

kamilogorek commented Apr 24, 2020

@justinperkins shoot me up an email at [email protected]

@kevich I'll try to spend some time on this and see what I can find, thanks for the info

@lobsterkatie
Copy link
Member

lobsterkatie commented Apr 24, 2020

I can't share a link to our private events publicly, but happy to do so via email or something else. LMK

@justinperkins Of course - not trying to expose anything that shouldn't be. FYI for the future, any event link is only accessible to members of the org and Sentry employees, but you're also always welcome to email such things to [email protected] and reference a GH issue. In any case, for this one, you're in good hands with Kamil. 🙂

Also, if this is threatening to put you over quota, please do write to support and they can help.

@justinperkins
Copy link

Thanks @lobsterkatie, just didn't want one of our private URLs out in the wild, password-protected or not. I've written to support numerous times and they have assisted with quota overages (we've gone over every month since we added Sentry to our frontends). Thank you 👍

@asbjornh
Copy link

We have 4 frontends and I've confirmed they all have the latest code. I can look into adding a release to the init code.

@justinperkins We are experiencing the same thing (issues fixed weeks ago still getting new issues in the dashboard). Just a few minutes ago I got a new issue for a bug that was fixed in a deployment 10 days ago.

We recently set up releases, and it turns out that some people are actually still running weeks-old releases even though we do webpack hasing and all that stuff. Apparently some folks leave their browser tab open for weeks on end :) Having releases hasn't reduced the number of issues but now we're able to know whether specific fixes worked or not.

@kamilogorek
Copy link
Contributor

@asbjornh thanks for the feedback. This was my best guess as well, as I have a tendency to open some tabs for weeks if not months myself 😅

Having releases hasn't reduced the number of issues but now we're able to know whether specific fixes worked or not.

Do you have any specific issues that we might help with right now?

@asbjornh
Copy link

asbjornh commented May 19, 2020

Do you have any specific issues that we might help with right now?

EDIT: No I don't! I just spent 9 hours googling, stack overflowing and looking at open source code just to realize that I had typed TypError in one of my ignoreError patterns.

Original post Maybe! I'm also having issues with `ignoreErrors`. We're getting a lot of `TypeError: cancelled`, which seems to be [some iOS specific thing](https://stackoverflow.com/questions/55738408/javascript-typeerror-cancelled-error-when-calling-fetch-on-ios).

We've initiated the browser client with the following in ignoreErrors, but we're still getting these errors in the latest release (confirmed by comparing the release field in the Sentry issue with the actual latest deployment):

Sentry.init({
  ... // DSN etc
  ignoreErrors: [
    'TypeError: cancelled',
    'TypeError: avbrutt',
    'TypError: avbruten', // EDIT: OMG Typ Error 😭
  ]
});

I suspect this is not nearly enough information for you to be able to do anything, and we also can't share access to the space. I'm considering trying the beforeSend workaround mentioned above.

One thing that'd be helpful is if you could point me in the direction of the source code that does the filtering of errors using ignoreErrors. Maybe I can figure ut out that way :) Edit: I now realize that GitHub has a search function and I found the relevant places in the source code.

Edit: Also, I think it would be really good if it was documented somewhere how error objects are compared to the strings in ignoreErrors. It's not clear whether or not you should include the error name or if you need to type an entire error message or if substrings are also sufficient. If this is already documented and I missed it I apologize in advance.

@kevich
Copy link

kevich commented May 19, 2020

@asbjornh Can you constantly reproduce not properly applied ignoreErrors configuration? Where error shouldn't be sent to Sentry, but it's still sent.

Because in our case there was exactly problem to reproduce this behaviour locally. Meaning when I try it myself - the error happening is properly filtered, and not sent to Sentry. But for some users it still happens, and error passthrough the ignoreErrors filter.

@asbjornh
Copy link

asbjornh commented May 19, 2020

To be honest I haven't been able to reproduce my specific error locally at all, since it apparently is specific to iOS and very rare (~50 events for the past month with thousands of page views).

Edit: Removed groundless speculation around issue caused by my own inability to spell words correctly.

@asbjornh
Copy link

asbjornh commented May 19, 2020

@kevich I did a local test manually throwing an error like this:

<button onClick={() => { throw new TypeError('cancelled'); }} />

This error was not reported to Sentry, so it seems like my ignore rule of "TypeError: cancelled" successfully filtered out this error. If I change the error message to some other string, the error appears in Sentry as expected. EDIT: So yes, I can reproduce errors being consistently ignored locally. I'm guessing the reason you might still be seeing the errors is the same as mine: people browsing your page with cached javascript files.

@justinperkins
Copy link

We recently set up releases, and it turns out that some people are actually still running weeks-old releases even though we do webpack hasing and all that stuff. Apparently some folks leave their browser tab open for weeks on end :) Having releases hasn't reduced the number of issues but now we're able to know whether specific fixes worked or not.

It took 10+ days post-release for the errors to finally dissipate, I'm not sure why it takes that long given a modern JS stack with MD5 stamped URLs, but it does. We auto log people out after 24 hours, which leads me to believe all these errors should stem from the login page, but not entirely sure.

As per advice from Sentry, adding a custom release version tag to events would go a long way towards troubleshooting the source of rogue events in Sentry. We haven't done it yet, but it's on the list.

@kamilogorek
Copy link
Contributor

Edit: Also, I think it would be really good if it was documented somewhere how error objects are compared to the strings in ignoreErrors. It's not clear whether or not you should include the error name or if you need to type an entire error message or if substrings are also sufficient. If this is already documented and I missed it I apologize in advance.

@asbjornh totally agree! PR sent #1698

@corny
Copy link

corny commented Jun 17, 2020

PR has been accepted. Can this issue be closed?

@kamilogorek
Copy link
Contributor

You are correct, merged and deployed :)

@github-actions github-actions bot locked and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants