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

Attach stack trace to events from HTTPClient integration so they are not merged (fingerprinted) into a single issue #8353

Closed
3 tasks done
harry-gocity opened this issue Jun 19, 2023 · 11 comments · Fixed by #14515
Assignees
Labels
Meta: Good First Issue Package: browser Issues related to the Sentry Browser SDK Type: Bug

Comments

@harry-gocity
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/nextjs

SDK Version

7.53.1

Framework Version

React 17.0.2, Next.js 12.1.6

Link to Sentry event

https://go-city.sentry.io/issues/4231630878/?project=4503958434480128

SDK Setup

const { publicRuntimeConfig } = getConfig();

const isProduction = process.env.NODE_ENV === 'production';

Sentry.init({
    // when dsn is null, no errors will be sent to Sentry
    dsn: isProduction ? publicRuntimeConfig.SENTRY_DSN : null,
    environment: publicRuntimeConfig.ENVIRONMENT ?? 'local',
    release: isProduction ? publicRuntimeConfig.SENTRY_RELEASE : `development-${publicRuntimeConfig.USER}`,
    ignoreErrors: ['jQuery'],
    allowUrls: isProduction
        ? [
              // Match our app 
              /https://our-site-here.com/,
              // Match gtm error
              /gtm.js/,
          ]
        : [],
    tracesSampler,
    beforeSend: (event: Event): Event => {
        // append LogRocket Session url to Sentry extras, for easier debugging
        const logRocketSession = LogRocket.sessionURL;
        if (logRocketSession !== null) {
            event.extra['LogRocket'] = logRocketSession;
        }
        return event;
    },
    integrations: [
        new HttpClient(),
    ],
});

Steps to Reproduce

We added the HTTP Client Integration to our Sentry setup:

    integrations: [
        new HttpClient(),
    ],

After this, we started to see events being tracked in Sentry for 500, 502 and 504 status codes in responses from our site.

Expected Result

We would be able to triage requests to different URLS separately. Sentry would group HTTPClient-captured events as separate issues. We thought that would happen by default using the request url and response status code.

Actual Result

All the events are grouped under the same Sentry issue, regardless of the error code or request URL.

For the issue linked above:

image

Under the 'all events' tab there several different issues all being grouped together (URLs cropped intentionally):

image

AFAIK we have not accidentally merged any issues manually.

image

@lforst
Copy link
Member

lforst commented Jun 20, 2023

Thanks for writing in! This is a very valid point. We'll try to fix this.

@harry-gocity
Copy link
Author

Hi @lforst any update on this?

@lforst
Copy link
Member

lforst commented Oct 9, 2023

@harry-gocity nope you would see it in this issue!

@harry-gocity
Copy link
Author

harry-gocity commented Feb 1, 2024

Hi @lforst just wondering if you could give any estimates as to if/when this would hit roadmap for a Sentry release. We currently have 8.4k events all grouped in Sentry under the same HTTP Client Error with status code: 500 issue. This is completely unmanageable and untriageable for us.

I'm currently upgrading us from 7.77 to 7.99 and saw that HttpClient is deprecated. Are there any changes in the httpClientIntegration that can help us? Otherwise, should we be separating these via a custom beforeSend and applying a fingerprint ourselves? Is there any other recommended approach to handling this?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Feb 1, 2024
@lforst
Copy link
Member

lforst commented Feb 1, 2024

@harry-gocity There are no real changes besides the renaming of it. We are currently not working on this issue and likely won't do so for the coming month. I cannot give a timeline.

We do not have any specific recommendations for grouping. Adding custom grouping in beforeSend seems fine to me.

@bryanjtc
Copy link

bryanjtc commented May 1, 2024

Is there any solution for this or an eta for a fix?

@getsantry getsantry bot moved this from Waiting for: Community to Waiting for: Product Owner in GitHub Issues with 👀 2 May 1, 2024
@lforst
Copy link
Member

lforst commented May 2, 2024

@bryanjtc No, this feature just needs some love. It's not really a bug, but the created errors need proper stacktraces and grouping.

@bryanjtc
Copy link

bryanjtc commented May 2, 2024

@bryanjtc No, this feature just needs some love. It's not really a bug, but the created errors need proper stacktraces and grouping.

Do you have an example for a better grouping strategy using beforeSend?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 May 2, 2024
@lforst
Copy link
Member

lforst commented May 2, 2024

@bryanjtc I guess you could look at the request context and looks at the url, method, headers, and cookies value and group however you like.

const event: SentryEvent = {
message,
exception: {
values: [
{
type: 'Error',
value: message,
},
],
},
request: {
url: data.url,
method: data.method,
headers: data.requestHeaders,
cookies: data.requestCookies,
},
contexts: {
response: {
status_code: data.status,
headers: data.responseHeaders,
cookies: data.responseCookies,
body_size: _getResponseSizeFromHeaders(data.responseHeaders),
},
},
};

@getsantry getsantry bot removed the status in GitHub Issues with 👀 2 May 2, 2024
@AbhiPrasad AbhiPrasad added the Package: nextjs Issues related to the Sentry Nextjs SDK label Jun 13, 2024
@lforst lforst added Package: browser Issues related to the Sentry Browser SDK Meta: Good First Issue and removed Package: nextjs Issues related to the Sentry Nextjs SDK labels Jun 25, 2024
@lforst lforst changed the title Events from HTTPClient integration incorrectly merged into single issue Attach stack trace to events from HTTPClient integration so they are not merged (fingerprinted) into a single issue Jun 25, 2024
@onurtemizkan onurtemizkan self-assigned this Nov 4, 2024
@alexmelagrano
Copy link

For anyone else who finds this -- transforming the default ErrorEvent objects was quite easy, and is working well for us.

Here's a simplified version of what I set up using beforeSend:

beforeSend(event, hint) {
  ...

  // Naively looking for events with the default `HTTP Client Error with status code: 500` message
  if (event.message.match(HTTP_SENTRY_ERROR_REGEX) {
    if (event.contexts?.response && event.request) {
      const requestStatusCode = errorEvent.contexts.response.status_code;
      const requestMethod = errorEvent.request.method;
      const requestUrlPath = errorEvent.request.url ? new URL(errorEvent.request.url).pathname : 'unknown';
      // Util for scrubbing path IDs
      const cleanedRequestUrlPath = cleanUrlPath(requestUrlPath);

      const newErrorMessage = `HTTP Client Error: ${requestMethod} ${cleanedRequestUrlPath} returned with code ${requestStatusCode}`;

      // Override the displayed message
      event.message = newErrorMessage;

      // Override the actual exception value; this is how Events are categorized into Issues
      if (event.exception?.values && event.exception?.values[0].value?.match(HTTP_SENTRY_ERROR_REGEX)) {
        event.exception.values[0].value = newErrorMessage;
      }
    }
  }

  ...

  return event;
}

Error message string is obviously up to you, but I figured the method/path/code were the important bits I wanted these Issues to group themselves by. Below is a screenshot of our React project's related issues before & after we deployed this V1 solution; you can see the generic error at the top, then the more detailed errors splitting out from that.

Image

I'll also note that we're running this solution in our React Native app, and the solution was identical. The only annoyance is that the types for beforeSend don't line up by default, despite having all the same information.

Copy link
Contributor

A PR closing this issue has just been released 🚀

This issue was referenced by PR #14515, which was included in the 8.45.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta: Good First Issue Package: browser Issues related to the Sentry Browser SDK Type: Bug
Projects
Archived in project
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

8 participants