getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.95k stars 1.57k forks source link

Sentry in Vue app causes freeze / 100% CPU usage (inf. recursion) [includes minimal example] #2957

Closed Zajozor closed 2 years ago

Zajozor commented 4 years ago

Package + Version

Version:

vue 2.6.12
vue-router 3.4.6
bootstrap-vue 2.17.3
sentry-javscript 5.25

Description

We have a Vue application and we encountered an issue when we added sentry. We can reproduce the situation in Chrome/Chromium/Firefox both Linux,Mac and Windows (sometimes the window just freezes without maxing out cpu usage) so it does not seem to be fully platform dependent. I tried to narrow down the issue and I provide a minimal reproducible example (70 lines of html) along with gif recordings of the problem.

I am open to providing more information, helping with the investigation or even creating a PR if pointed the right way.

The Setup

The Vue application uses these libraries:

Esentially what we have is a bunch of blocks below each other (left) and a navigation with fixed position (right) which includes a button for each of the blocks on the page. We want to achieve a standard behaviour which includes:

So essentially, it works and looks like this (note the cpu usage in bottom right, as expected some load is on the windowserver but all seems fine):

CleanShot 2020-10-07 at 17 54 07

The problem

The moment we add sentry, with an existing dsn, or with the example text like this:

Sentry.init({
  dsn: "https://examplePublicKey@o0.ingest.sentry.io/0",
  integrations: [new Sentry.Integrations.Vue({ Vue, attachProps: true })],
})

This is what happens - after a while, the cpu usage of the browser process jumps to 100% and the page starts to lag:

CleanShot 2020-10-07 at 17 52 05

Eventually (depends on the machine, but on my mbp2017 it is about 30 secs), the browser console starts to fill with a bunch of errors (call stack size limit exceeded and 429+400 from sentry). During this, we also get rate limited by sentry (in our sentry project, running into this resulted in about 1000 events over those 30 seconds).

CleanShot 2020-10-07 at 17 32 53@2x CleanShot 2020-10-07 at 17 33 04@2x

Reproduction

I am providing a minimal reproducible example for this here (single html file, 70 lines): https://github.com/Zajozor/reproducible/blob/main/sentry-javascript/index.html

(Be sure to turn off uBlock or other blocker that might block sentry if you try this 😅 )

The problem happens when you open the page, and just do what is depicted on the gif above for a while (may take from 5 secs to 20 secs for it to kick in). When you comment the lines 66-69 then the problem disappears (ie. the site has expected cpu usage).

It might be that we are doing something wrong in our code, but there is literally no issue with it until you enable sentry.

Thanks for your time :)

xwartz commented 4 years ago

Any updates on this? I have the similar issue when using sentry-react-native(version 1.9.0).

kamilogorek commented 4 years ago
Sentry.init({
  dsn: "https://examplePublicKey@o0.ingest.sentry.io/0",
  integrations: [
    new Sentry.Integrations.TryCatch({ eventTarget: false }),
    new Sentry.Integrations.Vue({ Vue, attachProps: true })
  ]
})

This will fix the issue, for now. However, I need more time to get into the internals and debug what is actually going on between Vue and this feature.

rchl commented 3 years ago

Should this workaround also work when TryCatch integration is registered as the last one in the list?

Asking because when adding it through a Nuxt module, there is currently no way to specify different order.

gaodeng commented 3 years ago

any news?

RyanMulready commented 3 years ago

Could use an update on this one; it is preventing the release of a feature for us.

martin-haryanto commented 3 years ago

This also happens to me multiple times. at different places when I tried to log errors for sentry. any news?

HazAT commented 3 years ago

Does this still happen in the latest version?

RyanMulready commented 3 years ago

Does this still happen in the latest version?

Yes it is still not resolved.

Edit: I should clarify if you're implying it was fixed and we should test the latest version without disabling eventTarget I have not done that.

aguynamedben commented 2 years ago

Is this going to be addressed? I'm concerned that bugs like this in the Sentry SDK (via sentry-electron) will unpredictably crash my app which is unacceptable for my users and engineers.

nmackey commented 2 years ago

I'm also running into this issue. Sentry hangs whenever Vue Router throws the following error: Error: Redirected when going from 'x' to 'y' via a navigation guard.

rmarmitt commented 2 years ago

Any updates?

RyanMulready commented 2 years ago

Any updates?

Still a problem for us, tested last weekish. We use https://github.com/getsentry/sentry-javascript/issues/2957#issuecomment-713505111 as a work around still.

joshstrange commented 2 years ago

We just ran into this same issue. The fix in https://github.com/getsentry/sentry-javascript/issues/2957#issuecomment-713505111 worked though we had to modify our code a bit differently. Posting it here so anyone else with a similar environment/setup to ours knows how to fix it.

Original:

import * as Sentry from '@sentry/vue';
import {Vue} from 'vue-property-decorator';
import {CaptureConsole, ExtraErrorData} from '@sentry/integrations';

Sentry.init({
    Vue,
    dsn: '<removed>',
    integrations: [
        new CaptureConsole({
            levels: ['error', 'warn'],
        }),
        new ExtraErrorData({depth: 20})
    ],
    normalizeDepth: 21,
    tracesSampleRate: 1.0,
});    

Fixed:

import * as Sentry from '@sentry/vue';
import {Vue} from 'vue-property-decorator';
import {CaptureConsole, ExtraErrorData, Vue as SentryVue} from '@sentry/integrations';

Sentry.init({
    // Vue, // <- Note Vue is no longer passed in here
    dsn: '<removed>',
    integrations: [
        new Sentry.Integrations.TryCatch({
            eventTarget: false
        }),
        new SentryVue({Vue, attachProps: true}),
        new CaptureConsole({
            levels: ['error', 'warn'],
        }),
        new ExtraErrorData({depth: 20})
    ],
    normalizeDepth: 21,
    tracesSampleRate: 1.0,
});    

The root of our problem appears to be a console.log(from) inside a router.beforeEach navigation guard. It caused lockup in the browser and inside of a capacitor app (though the capacitor app would reload the webview after being hung for a period of time). My guess is that from had a circular reference in it which caused this (given the issue appears to be related to serialization at some point). That said, it's quite odd that a console.log() (emphasis on the log) would even be "intercepted" by Sentry.

rmarmitt commented 2 years ago

Bump

joshstrange commented 2 years ago

Well I just hit this on a second project. I attempted to lock the sentry versions to the versions in the first project (thinking the problem had gotten worse in a newer version) but no dice. It blows up the console with errors coming from the sentry code the overrides the console. I even tried removing the CaptureConsole plugin but it's still causing the page to lock up.

Honestly I'm at a complete loss here and I'm going to probably need to find an alternative to Sentry. It's incredibly frustrating that a framework that's supposed to help you catch/track bugs is a huge source of bugs. Trying to report a bug should never result in it causing another bug.

This plus the memory leak/bug in the iOS SDK (causes apps to crash, doesn't report an error) gives me very little confidence in a product I've loved for a long time and recommended many times as well.

I hope this gets fixed but it seems like the Sentry team doesn't care about this issue at all.

rchl commented 2 years ago

Use the latest version and if that still reproduces then provide reproduction. How else do you expect that devs would figure out your problem and fix it?

smeubank commented 2 years ago

My fault I let this one slip through. Slamming it on our Jira board and we will keep posted with our analysis and progress

joshstrange commented 2 years ago

Use the latest version and if that still reproduces then provide reproduction. How else do you expect that devs would figure out your problem and fix it?

So this is a very unhelpful comment.

First, the issue has been identified, it’s a problem with their walk function. It doesn’t detect circular references and will keep walking until the browser runs out of memory.

Second, reproduction steps have been provided: use a router guard that redirects to a different page and the error that it throws will cause this issue. Though I expect this can be reproduced in any framework by simply throwing an error that contains circular references.

Third, this is a paid product. I’m grateful that their libraries are open source and people can suggest modifications and see the inner workings but my company pays for this service. I don’t think it’s ridiculous to expect a common use-case (router guards) on a framework that’s in the top 3 (Vue) of frameworks used (along with Angular and React) to be supported.

Lastly, this is a nasty bug that’s been around for 18 months, I don’t think my exasperation/annoyance was uncalled for. As I said before, a bug catching/reporting tool should never be the cause of a bug itself. It would be like if your fire extinguisher spontaneously combusted and burned down your house. Until you’ve spent a good day or more of time tracking down “Your website freezes and causes the tab to eventually crash” only to find it was your bug catching tool then I don’t think you can fully appreciate the frustration involved.

I am very thankful this is being looked into and I know it’s a solvable problem. In fact, I installed rollbar last night as an alternative and it handled this same error without issue (I would prefer to switch back to sentry).

lforst commented 2 years ago

@joshstrange We are currently investigating this. Can we get some more information on your setup? What versions of the Sentry sdk and vue are you using? What packages are you using? The Sentry vue integration or the standalone sentry vue package? Thanks!

Zajozor commented 2 years ago

@lforst I believe my original report contains a link to a minimal (70 lines of code for the whole app) reproduction example.

Is there anything specific I can do to help provide more information?

Thanks!

lforst commented 2 years ago

@Zajozor the repro you provided was already very useful! We want to reproduce this issue with more recent versions of the SDK, because the vue integration (which is used in your example) will soon be discontinued in the upcoming major version of the SDK, which is why we want to direct our fixing efforts to the standalone package. It is unlikely that we will fix this issue retroactively for older versions.

joshstrange commented 2 years ago

@lforst I am using @sentry/vue (6.16.1, but I tried the newest version as well):

    "@sentry/integrations": "6.16.1",
    "@sentry/tracing": "6.16.1",
    "@sentry/vue": "6.16.1",

Those are my sentry-specific packages. I'm using Quasar as my overall framework which is using vue 2.6.14.

Is @sentry/vue what is being discontinued or something else? It looks like the docs still recommend using it but maybe you all haven't fully discontinued it yet.

If you can tell me what the "happy path" is for using Sentry with Vue I'd be happy to spin up a small Vue project that can reproduce.

lforst commented 2 years ago

@joshstrange We will discontinue the Vue integration that lives in @sentry/integrations - the docs shouldn't reference it anymore but the original repro example still uses it (I guess this issue is already quite old 😬). @sentry/vue will live on :)

@sentry/vue is also what we would like to find a fix for. It would be extremely helpful if you could provide just a minimal reproduction using any Vue version and @sentry/vue. "Happy path" would be to follow the setup example as provided in the docs. Thank you very much!

joshstrange commented 2 years ago

@lforst Ok, I've been trying to reproduce in a single html file but I'm not having any luck. However even while using @sentry/vue in my app I can reproduce easily. I'm going to have to spin up a full project to replicate this better but I'm working on it.

joshstrange commented 2 years ago

@lforst Ok, I thought I had done this on my main project to no avail but I attempted to pair down the integrations I used to be minimal and I found that ExtraErrorData is what is causing this for me.

I have a repo here: https://github.com/joshstrange/sentry-bug (You'll have to put in a DSN in src/boot/sentry.ts) and it reproduces the issue I had. The console will blow up and the page will freeze when you press the "Reproduce" button. I use version 6.19.7 of @sentry/vue and @sentry/integrations.

I was unable to reproduce this in a single-page vue-only (no Quasar) app and I understand if you don't want to waste time tracking this down. For my main project I'll just remove the ExtraErrorData integration.

lforst commented 2 years ago

@joshstrange awesome, thank you very much! We will look into it.

lforst commented 2 years ago

@joshstrange I took a look at your reproduction case. Thanks again for providing it! I believe the slowdown happens here because the ExtraErrorData integration attaches data about the matched routes. This data happens to be huge, resulting in a ton of computation when serializing it. Additionally, vue doesn't seem to like it when we try to access certain fields of this data, and we read all the fields, so it logs out a lot of bunch of errors (which might actually cause the slowdown - the CaptureConsole integration will only amplify this).

The following may sound bad but stick with me: We will most likely not adjust this behaviour.

I think we can resolve this issue through configuration. Basically what it boils down to, is that the depth option you passed to the ExtraErrorData integration is too large for the size of the payload - we are trying to serialize way too much data. In the current version of the SDK, we have the issue that depth always needs to be more than normalizeDepth. We identified that this sucks and provided a fix for that in version 7.0.0-beta.1. In that version you can keep normalizeDepth at 20 but set depth to something sane like 3. This will prevent the SDK from walking the entirety of the data and should resolve the slowdown.

Can you try upgrading to version 7.0.0-beta.1, set the depth option to a smaller value (maybe 4 or 5?), and report back whether that helped?

As for the original issue: I believe this is unrelated so we will keep this issue open and investigate further.

joshstrange commented 2 years ago

The following may sound bad but stick with me: We will most likely not adjust this behaviour.

That makes perfect sense given what you found and I really appreciate you all digging into this more. I'm not 100% sure where the depth: 20 came from originally (I want to say the docs but it might have been from some blog/tutorial I found) but I'm glad that we can re-add this integration safely.

I tested 7.0.0-beta.1 and set my depth to 4 and I was unable to reproduce. When 7.0.0 (stable) comes out we will move to that and re-add the ExtraErrorData integration.

Thank you again for your help!

flex-jonghyen commented 2 years ago

@lforst I have same problem on React, I saw your PR to fix this problem. But can i use this fix on 6.x version?

lforst commented 2 years ago

@flex-jonghyen As it stands, it is unlikely that we will cherry-pick the fix onto 6.x (if enough people ask, we might). However, in the upcoming days, we will provide a release candidate for v7 which includes the fix. Currently we recommend updating the SDK to that version to get rid of the issue.

lforst commented 2 years ago

Closing this as fix #5147 will be released in upcoming version.

mirko77 commented 7 months ago

Same issue in @sentry/capacitor. The app freezes after a router.replace(), then it gets killed.

rchl commented 7 months ago

not sure what it has to do with this module

mirko77 commented 7 months ago

not sure what it has to do with this module

Well, we are using Vue in our Ionic app, the docs at https://docs.sentry.io/platforms/javascript/guides/capacitor/ionic/ state we need to include @sentry/capacitor and @sentry/vue (which is probably a wrapper of sentry-javascript), and by doing so, the app freezes then it crashes on a vue router call.

If it is not related at all, we sincerely apologise. However, a Google search "sentry Vue app freezes" sent us here as the first search hit, and now we know why the app crashes. We realised what the issue was by looking at this issue, we removed sentry, it is all good again.

CptJJ commented 5 months ago

This seems to be back in v8.0.0 @lforst

RangeError: Maximum call stack size exceeded
    at BrowserReporter.log (browser.mjs?v=0c2e0812:44:7)
    at Consola._log (core.mjs?v=0c2e0812:381:16)
    at resolveLog (core.mjs?v=0c2e0812:349:14)
    at Consola._logFn (core.mjs?v=0c2e0812:377:5)
    at core.mjs?v=0c2e0812:306:19
    at BrowserReporter.log (browser.mjs?v=0c2e0812:44:7)
    at Consola._log (core.mjs?v=0c2e0812:381:16)
    at resolveLog (core.mjs?v=0c2e0812:349:14)
    at Consola._logFn (core.mjs?v=0c2e0812:377:5)
    at core.mjs?v=0c2e0812:306:19
    at BrowserReporter.log (browser.mjs?v=0c2e0812:44:7)
    at Consola._log (core.mjs?v=0c2e0812:381:16)
    at resolveLog (core.mjs?v=0c2e0812:349:14)
    at Consola._logFn (core.mjs?v=0c2e0812:377:5)
    at core.mjs?v=0c2e0812:306:19
    at BrowserReporter.log (browser.mjs?v=0c2e0812:44:7)
    at Consola._log (core.mjs?v=0c2e0812:381:16)
    at resolveLog (core.mjs?v=0c2e0812:349:14)
    at Consola._logFn (core.mjs?v=0c2e0812:377:5)
    at core.mjs?v=0c2e0812:306:19
    at BrowserReporter.log (browser.mjs?v=0c2e0812:44:7)
    at Consola._log (core.mjs?v=0c2e0812:381:16)
    at resolveLog (core.mjs?v=0c2e0812:349:14)
    at Consola._logFn (core.mjs?v=0c2e0812:377:5)
    at core.mjs?v=0c2e0812:306:19
    at BrowserReporter.log (browser.mjs?v=0c2e0812:44:7)
    at Consola._log (core.mjs?v=0c2e0812:381:16)
    at resolveLog (core.mjs?v=0c2e0812:349:14)
    at Consola._logFn (core.mjs?v=0c2e0812:377:5)
    at core.mjs?v=0c2e0812:306:19
    at BrowserReporter.log (browser.mjs?v=0c2e0812:44:7)
    at Consola._log (core.mjs?v=0c2e0812:381:16)
    at resolveLog (core.mjs?v=0c2e0812:349:14)
    at Consola._logFn (core.mjs?v=0c2e0812:377:5)
    at core.mjs?v=0c2e0812:306:19
    at BrowserReporter.log (browser.mjs?v=0c2e0812:44:7)
    at Consola._log (core.mjs?v=0c2e0812:381:16)
    at resolveLog (core.mjs?v=0c2e0812:349:14)
    at Consola._logFn (core.mjs?v=0c2e0812:377:5)
    at core.mjs?v=0c2e0812:306:19
    at BrowserReporter.log (browser.mjs?v=0c2e0812:44:7)
    at Consola._log (core.mjs?v=0c2e0812:381:16)
    at resolveLog (core.mjs?v=0c2e0812:349:14)
    at Consola._logFn (core.mjs?v=0c2e0812:377:5)
    at core.mjs?v=0c2e0812:306:19
    at BrowserReporter.log (browser.mjs?v=0c2e0812:44:7)
    at Consola._log (core.mjs?v=0c2e0812:381:16)
    at resolveLog (core.mjs?v=0c2e0812:349:14)
    at Consola._logFn (core.mjs?v=0c2e0812:377:5)
    at core.mjs?v=0c2e0812:306:19
    Sentry.init({
    app: nuxtApp.vueApp,
    attachProps: true,
    attachStacktrace: true,
    dsn: config.public.sentryDsn,
    environment: config.public.sentryEnvironment,
    integrations: [
       Sentry.captureConsoleIntegration({ levels: ['error'] }),
      Sentry.browserTracingIntegration({ router }), 
     Sentry.httpClientIntegration(),
      Sentry.replayIntegration({})
    ],

     tracesSampleRate: 0.2, 

    tracePropagationTargets: [ 'localhost','vercel.app'],

  })
lforst commented 5 months ago

@CptJJ That seems odd. Would you mind opening a new issue with reproduction? Thanks!

CptJJ commented 5 months ago

@lforst created a PR over here:

https://github.com/manniL/nuxt3-sentry-recipe/pull/8#issue-2296212585

shaidrv commented 2 months ago

I have exactly the same error as @CptJJ. @lforst What can be done to solve the problem?

lforst commented 2 months ago

@shaidrv do you happen to have minimal reproduction? It might be the captureConsoleIntegration. cc @s1gr1d

lforst commented 2 months ago

Also, this issue is super old and very likely unrelated to anything more recent. For this reason, and because it invites piling on I will lock this issue but please feel free to open a separate issue for any problems you have!