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/angular - @sentry/browser] Setting context data (tags, extra or context) for errors/exceptions is not working in an async context #5417

Closed
3 tasks done
rpanadero opened this issue Jul 13, 2022 · 12 comments
Labels
Package: angular Issues related to the Sentry Angular SDK Type: Bug

Comments

@rpanadero
Copy link

rpanadero commented Jul 13, 2022

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/angular

SDK Version

7.6.0

Framework Version

14.0.0

Link to Sentry event

No response

Steps to Reproduce

Hello,

We are facing some issues with the latest browser/angular SDK version (7.6.0) because the context data (tags, extra, etc), which we just set before capture the event by Sentry SDK, don't appear in the Sentry dashboard along with the error. This problem just shows up when the error happens in an async context like:

setTimeout(() => {
   const c = null;
   const v = c.pop;
}, 1000)

However, if the error happens in a sync context, the error context data does appear in the Sentry dashboard as expected.

In order to set a specific context for an error we have unsuccessfully tried all ways that the documentation says here: https://docs.sentry.io/platforms/javascript/enriching-events/context/#passing-context-directly

For example like that:

Sentry.withScope(scope => {
  scope.setTag('customTag', 'error');
  scope.setExtra('test', 'this is a test');
  scope.setContext('test', { value: 'test context' });
  Sentry.captureException(extractedError);
});

Or like that:

Sentry.captureException(error, {
  tags: {
    section: "articles",
  },
});

For our surprise, this just happens in SDK version higher than 7.0, since we have also tried to do the same with the latest 6x SDK version and it is working property over it (all data is shown up in the Sentry dashboard as expected).

Please, is there some official workaround to deal with this issue?

Thanks in advance,

Expected Result

Sentry SDK should record tags, extra and context we set for the error captured by Sentry SDK and they also should be shown up in the Sentry dashboard.

Actual Result

Sentry SDK doesn't record properly the error tags and context data when the error happens in an async context. It seems that all data set in the error scope is lost.

@rpanadero rpanadero changed the title [@sentry/angular - @sentry/browser] Setting a context data (tags, extra or context) for errors/exceptions is not working in an async context [@sentry/angular - @sentry/browser] Setting context data (tags, extra or context) for errors/exceptions is not working in an async context Jul 13, 2022
@Lms24
Copy link
Member

Lms24 commented Jul 13, 2022

Hi @rpanadero and thanks for reporting! Also thanks for the good description. Would you be able to provide a simple reproduction for us to work on this? I don't think we changed something regarding scopes and contexts in v7 so it would be interesting to find out what's wrong here. Thanks!

@rpanadero
Copy link
Author

rpanadero commented Jul 13, 2022

@Lms24 , of course. I have just created a demo project to reproduce the issue.

https://github.com/rpanadero/sentry-issue-5417

The main project page contains two buttons to raise an error from an async or sync context.

And I will also share with you the details of the error reported in the Sentry Dashboard.

Async error details
https://sentry.io/share/issue/d16a069ce14f486a8aa757649d5060c3/

No tags or other error context data are present in error report.

Sync error details
https://sentry.io/share/issue/ab36f92248f84eb89551f1ccc53b86b0/

Tags and other context data are present in error report.

This issue is breaking our application reports and our customers are complaining, please I would really appreciate a fix or workaround asap. I have played with Hubs and Scope to try to fix the issue, but all workarounds I have found make us loose other important error data.

Thanks!

@Lms24 Lms24 added Package: angular Issues related to the Sentry Angular SDK and removed Status: Needs Reproduction labels Jul 14, 2022
@Lms24
Copy link
Member

Lms24 commented Jul 15, 2022

I took a quick look at the repro and I saw that you're using a custom ErrorHandler. I suspect that one of the reasons for doing this is because you want to attach the tags/context before the errors are sent to Sentry, right?

If so, I have two suggestions:

  • You could try using our Sentry.ErrorHandler as shown in our docs and use the beforeSend hook to attach whatever information you want to attach to the event.
  • As you can see in the code in our ErrorHandler, we're capturing the exception outside the Angular zone. It might be a conflict with Zone but tbh this is more a guess than anything else.

It'd be interesting as to why this apparently started happening with v7 and not with v6 but we're aware that there are certain issues with scopes and async contexts so this might be one of them (but those date back pre-v7).

Hope this helps a little!

@rpanadero
Copy link
Author

rpanadero commented Jul 19, 2022

@Lms24 , you're right, I need to add some tags and context data before sending the errors to Sentry.

Regarding your suggestions, firstly, I have tried to capture the exception outside the Angular zone but this hasn't solved the issue unfortunately (check the demo project again if you need it). While your first workaround is not valid for our use case, since we have to add some context data, which we just have at the moment of capturing the error by Sentry SDK and not in the beforeSend hook.

Thanks for your suggestions, but have you tried to reproduce the issue in v7, verify that it's not happening in v6, and go go deep in this issue? I agree with you that it's really interesting to get to know why this is happening from v6... Like that, we'll probably find a workaround or a fix.

Thanks for your effort.

@Lms24
Copy link
Member

Lms24 commented Jul 19, 2022

Thanks for getting back to me. I agree, this needs a more thorough investigation. Right now though, our priorities are on other tasks but I'll backlog this with high priority. In the meantime, if you happen to find a solution/workaround, please feel free to share it here; maybe we can incorporate it into the SDK then.

To everyone reading this issue: Please react to the issue if you're having the same problem. PRs are always welcome!

@rpanadero
Copy link
Author

Thanks! I hope to hear good news from you asap.

@janvennemann
Copy link

janvennemann commented Jul 25, 2022

@Lms24 i think i'm experiencing the same issue with @sentry/node and the express handlers. I've created a bunch of very simple test cases here: https://github.com/janvennemann/sentry-callback-issue. I'm running Node.js v16.15.0 on MacOS 12.3.1.

As soon as there is a callback based async API involved Sentry seems to not send any additional info for the HTTP request. Interestingly enough, when doing the same using Promise based APIs, it works again. See the mongodb examples in my repo. This is currently a blocker for us using Sentry in our Nodejs backend.

I've also found #1939 which looks similar since i first noticed the issues with passport and mongodb. However, since that issue is quite old i think this is something new.

@Luis-Palacios
Copy link

Luis-Palacios commented Sep 9, 2022

I've run into the same issue on Express I've tried every way to add custom data but to no avail.

My code is like this:

sentry.init(sentryConfig.get(app));

app.use(sentry.Handlers.requestHandler());

app.use((err, req, res, next) => {
  if (err.name === 'SequelizeDatabaseError') {
      
    sentry.configureScope(function(scope) {
      scope.setTag('bad-sql', 'Some bad sql');
      scope.setExtra('bad-sql-extra', 'Some extra bad sql');
    });
  }
  next(err);
});

app.use(sentry.Handlers.errorHandler());

The exception gets logged but I can't see the my custom tag
Have you guys find a workaround?

@Lms24
Copy link
Member

Lms24 commented Mar 3, 2023

Hi @rpanadero sorry for the delayed response on this issue and thanks again for the excellent reproduction!

I used your repro app, reproduced the problem and I think I know why the tags are not applied and how you can work around this:

  • In the sync case, everything works as expected and your error handler (or our error handler if it was used) would catch the error and report it to Sentry. So far so good!
  • For functions that are executed in timer functions (like setTimeout), we use an integration called TryCatch to instrument the callback, catch errors and send them directly to Sentry. This seems to happen before the Angular-internal error handler is called. Hence, your tags are not applied in time.

To fix this, I'd advise to disable the TryCatch default integration. It should be safe to do so as the Angular error handler seems to catch timing-related/async errors. For your repro app, here's the adjusted code:

Sentry.init({
  dsn: '__DSN__',

  // filter out the try/catch integration and add the BrowserTracing integration
  integrations: (defaultIntegrations) => {
    return [
      ...defaultIntegrations.filter((i) => i.name !== 'TryCatch'),
      new BrowserTracing({/*...*/}),
    ];
  },
  // Set tracesSampleRate to 1.0 to capture 100%
  // of transactions for performance monitoring.
  // We recommend adjusting this value in production
  tracesSampleRate: 1.0,
});

We can think about disabling TryCatch by default for the Angular SDK but this has the downside that folks who only partially set up the SDK and don't use an Angular-internal error handler won't see these errors at all.

Let me know if this solution works for you :)

@llgarrido
Copy link

Hi @rpanadero sorry for the delayed response on this issue and thanks again for the excellent reproduction!

I used your repro app, reproduced the problem and I think I know why the tags are not applied and how you can work around this:

* In the sync case, everything works as expected and your error handler (or our error handler if it was used) would catch the error and report it to Sentry. So far so good!

* For functions that are executed in timer functions (like `setTimeout`), we use an integration called [`TryCatch`](https://docs.sentry.io/platforms/javascript/guides/angular/configuration/integrations/default/#trycatch) to instrument the callback, catch errors and send them directly to Sentry. This seems to happen _before_ the Angular-internal error handler is called. Hence, your tags are not applied in time.

To fix this, I'd advise to disable the TryCatch default integration. It should be safe to do so as the Angular error handler seems to catch timing-related/async errors. For your repro app, here's the adjusted code:

Sentry.init({
  dsn: '__DSN__',

  // filter out the try/catch integration and add the BrowserTracing integration
  integrations: (defaultIntegrations) => {
    return [
      ...defaultIntegrations.filter((i) => i.name !== 'TryCatch'),
      new BrowserTracing({/*...*/}),
    ];
  },
  // Set tracesSampleRate to 1.0 to capture 100%
  // of transactions for performance monitoring.
  // We recommend adjusting this value in production
  tracesSampleRate: 1.0,
});

We can think about disabling TryCatch by default for the Angular SDK but this has the downside that folks who only partially set up the SDK and don't use an Angular-internal error handler won't see these errors at all.

Let me know if this solution works for you :)

I have been experiencing the same problem from my own ErrorHandler implementation.

In my case you have solved the issue.

@Lms24
Copy link
Member

Lms24 commented Mar 20, 2023

Thanks for reporting! I'm going to close the issue then in the meantime as the workaround seems to be sufficient. In case folks are still experiencing problems we can always reopen it.

@Lms24 Lms24 closed this as completed Mar 20, 2023
Lms24 added a commit that referenced this issue Jun 20, 2023
The `TryCatch` default integration interferes with the
`SentryErrorHander` error handler of the Angular(-Ivy) SDKs by catching
certain errors too early, before the Angular SDK-specific error handler
can catch them. This caused missing data on the event in some or
duplicated errors in other cases.

This fix filters out the `TryCatch` by default, as long as users didn't
set `defaultIntegrations` in their SDK init. Therefore, it makes the
previously provided
[workaround](#5417 (comment))
obsolete.
@sparky-raccoon
Copy link

@Lms24 I have the same issue as @janvennemann, using promise based apis don't seem to change a thing (I would rather continue to use async anyway). Something like this below :

partnersRouter.get('/debug-sentry', [], async function handler (req, res) {
  await new Promise((resolve) => {
    setTimeout(() => {
      resolve()
    }, 10000)
  })

  throw new Error('debug sentry')
})

Produces an error without HTTP context, which is unfortunate. The demo project provided by jan illustrates this, have you seen it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: angular Issues related to the Sentry Angular SDK Type: Bug
Projects
None yet
Development

No branches or pull requests

6 participants