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

Context not being reliably set in withScope. Potential v7 regression. #5261

Closed
3 tasks done
sacummings91 opened this issue Jun 14, 2022 · 9 comments
Closed
3 tasks done
Assignees

Comments

@sacummings91
Copy link

sacummings91 commented Jun 14, 2022

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/react

SDK Version

7.1.1

Framework Version

7.1.1

Link to Sentry event

https://sentry.io/organizations/ternary/issues/3191835938/?environment=test&project=5634786&query=is%3Aunresolved

Steps to Reproduce

I have a small class that wraps sentry for capturing messages and exceptions. As shown below, for some reason, context does not get reliably sent.

public reportMessage(
    message: string,
    options?: { context?: Context; tags?: Tags }
  ) {
    Sentry.withScope((scope) => {
      if (options?.context) {
        scope.setContext("context", this._stringifyContext(options.context))
      }

      if (options?.tags) {
        scope.setTags(options.tags);
      }

      Sentry.captureMessage(message);
    });
  }

Below are two examples where it works every time. And I do not understand why.

If I do anything extra like this console log:

Sentry.withScope((scope) => {
      if (options?.context) {
        console.log("literally anything")
        scope.setContext("context", this._stringifyContext(options.context));
      }

      if (options?.tags) {
        scope.setTags(options.tags);
      }

      Sentry.captureMessage(message);
    });

Or even just setting context to a new variable first like this:

Sentry.withScope((scope) => {
     if (options?.context) {
       const context = this._stringifyContext(options.context);
       scope.setContext("context", context);
     }

     if (options?.tags) {
       scope.setTags(options.tags);
     }

     Sentry.captureMessage(message);
   });

Is this a bug with 7.0? This didn't happen before I upgraded.

Expected Result

Custom context should show up on issue in Sentry.io console

Actual Result

Custom context does not show up in Sentry.io console

@Lms24
Copy link
Member

Lms24 commented Jun 15, 2022

Hi @sacummings91 and thanks for writing in!
Could you clarify what you mean by the context not being "reliably sent"? Do you mean, sometimes it is sent and sometimes not? If that is the case can you confirm that is in fact happening in the exact same situations (e.g. the context is sometimes (not) sent during the same page load)?

The reason I'm asking is because it could have something to do with this line here which I'm suspecting to be ditching contexts as soon as we have a trace context attached to the event:

if (event.contexts && event.contexts.trace) {
normalized.contexts = {};

In case this has something to do with it, #5218 might just fix this issue.

But I might be off here, so any additional unformation you can provide would be appreciated. For example, can you show your Sentry.init config?

@sacummings91
Copy link
Author

sacummings91 commented Jun 15, 2022

Hey @Lms24, thanks for getting to this. Yeah, to clarify, if we send the context every time it only makes it to sentry.io like 1/5 of the times. It is the same situations. I have been testing it to be sure. This is also something that only started happening when we upgraded to v7. The weirdest part to me is that the latter 2 code snippets I posted work 100% of the time. Which makes it feel like a weird race condition or something. The PR you posted definitely looks like it could be related the problem too.

Sentry.init here:

Sentry.init({
    dsn: config.SENTRY_DSN,
    environment: config.SENTRY_ENV,
    integrations: [new Integrations.BrowserTracing()],
    maxValueLength: 2000,
    release: `${config.SENTRY_ENV}|${config.SENTRY_RELEASE_HASH}`,
    tracesSampleRate: 1,
  });

@AbhiPrasad
Copy link
Member

AbhiPrasad commented Jun 15, 2022

Following up on Lukas's work, I made a quick reproduction of your setup, but I was always able to see context attached to events: https://codesandbox.io/s/withscope-context-v7-97eegl?file=/src/index.js

A couple of things to note:

  1. Why are you calling this._stringifyContext? setContext takes an object as per:

setContext(name: string, context: { [key: string]: any } | null): void;

In the reproduction above, I removed _stringifyContext

  1. Could you try adding the debug: true option to your Sentry.init and seeing what happens? In addition, logging out your event in beforeSend will help us see the differences in what is being included and what is not.

  2. Calling scope.setContext("context", this._stringifyContext(options.context)) will put the entire context under the context key. It would be more correct to just create contexts with different keys:

if (options?.context) {
  const contexts = options.context;
  for (const key in contexts) {
    scope.setContext(key, contexts[key]);
  }
}

@sacummings91
Copy link
Author

sacummings91 commented Jun 15, 2022

Hey @AbhiPrasad, the name of _stringifyContext might be slightly misleading. But it doesnt return a string. It returns an object where all the values are stringified. We've had issues in the past getting the data to show up correctly in sentry.io, so that's why we have this helper function. Here's what it looks like:

private _stringifyContext(context: object): Record<string, string> {
    return Object.entries(context).reduce(
      (accum: Record<string, string>, [key, value]) => ({
        ...accum,
        [key]: JSON.stringify(value, null, 2),
      }),
      {}
    );
  }

The link to the sentry event in the original post also shows what it looks like when it's working properly and getting passed sentry.io. If you click the link you can see there is a "context" section and it's not all under one key. Here's another example with some more complex values: https://sentry.io/organizations/ternary/issues/3329348637/?environment=production&project=5634786&query=is%3Aunresolved&statsPeriod=14d#context-context

I will try the debug: true in sentry init and beforeSend and see if that helps get to the bottom of it.

@sacummings91
Copy link
Author

sacummings91 commented Jun 16, 2022

Hey @AbhiPrasad I'm testing some events with debug: true & a console.log(event) in beforeSend callback. I'm not seeing anything out of the ordinary. context is not on the event in the callback. Is there anything I can share here that will help you? Here's a link to an event with the debug changes on: https://sentry.io/organizations/ternary/issues/3191835938/?environment=test&project=5634786&query=is%3Aunresolved

@sacummings91
Copy link
Author

sacummings91 commented Jun 16, 2022

Ok I just did some logging in the Sentry source code and can confirm that context exists up to this point and is then removed here by this logic in core/src/baseClient:

if (event.contexts && event.contexts.trace) {
normalized.contexts = {};

Do you guys have any idea what version the fix for this is planned to be released in?

@sacummings91
Copy link
Author

sacummings91 commented Jun 16, 2022

Also, it looks like this bug was introduced in v7.1.0 with #5171. So I'm just going to roll back to v7.0 for now until it's fixed.

@AbhiPrasad
Copy link
Member

Good catch! We'll be fixing this ASAP and cutting a release very soon.

@AbhiPrasad
Copy link
Member

This should be fixed by 7.2.0: https://github.com/getsentry/sentry-javascript/releases/tag/7.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants