8000 beforeBreadcrumb can no longer access original Request object in v8 · Issue #12132 · getsentry/sentry-javascript · GitHub
[go: up one dir, main page]
Skip to content

beforeBreadcrumb can no longer access original Request object in v8 #12132

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.

Sign up for GitHub

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

Jump to bottom
Closed
3 tasks done
TheHolyWaffle opened this issue May 21, 2024 · 5 comments · Fixed by #12137
Closed
3 tasks done

beforeBreadcrumb can no longer access original Request object in v8 #12132

TheHolyWaffle opened this issue May 21, 2024 · 5 comments · Fixed by #12137
Labels
Package: node Issues related to the Sentry Node SDK

Comments

@TheHolyWaffle
Copy link
TheHolyWaffle commented May 21, 2024 •
edited by mydea
Loading

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

8.2.1

Framework Version

No response

Link to Sentry event

No response

SDK Setup

Sentry.init({
    dsn: '....',
    beforeBreadcrumb: beforeBreadcrumb,
    enableTracing: true,
    sampleRate: 1,
    integrations: [Sentry.httpIntegration(), Sentry.nativeNodeFetchIntegration()],
});


export function beforeBreadcrumb(breadcrumb: Breadcrumb, hint?: BreadcrumbHint): Breadcrumb | null {
    if (breadcrumb.type !== 'http' || breadcrumb.category !== 'http' || !hint) { // <---- hint is undefined here
        return breadcrumb;
    }
    // .....
}

Steps to Reproduce

  1. Make an native node fetch api call
  2. Throw an error so that beforeBreadcrumb is triggered

Expected Result

The beforeBreadcrumb handler should receive the original fetch Request object as a hint parameter. This is used to extract the fetch request headers and apply this information to enrich the breadcrumb.

Actual Result

The beforeBreadcrumb handler does not receive the original fetch Request object anymore since Sentry v8

image
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 21, 2024
@github-actions github-actions bot added the Package: browser Issues related to the Sentry Browser SDK label May 21, 2024
@mydea mydea added Package: node Issues related to the Sentry Node SDK and removed Package: browser Issues related to the Sentry Browser SDK labels May 21, 2024
@mydea
Copy link
Member
mydea commented May 21, 2024

Hey, thanks for writing in - that is indeed not on purpose, we'll add this back, sorry about the inconvenience!

@mydea mydea mentioned this issue May 21, 2024
Merged
@TheHolyWaffle
Copy link
Author

@mydea Thanks for looking into this. Just to double-check this desired behavior is expected for both httpIntegration and nativeNodeFetchIntegration 🙏

@mydea
Copy link
Member
mydea commented May 21, 2024

Yeah, you are right! While looking into this some more I noticed that we have some general issues there, mainly that breadcrumbs were (incorrectly) tied to spans, meaning that if performance was disabled we would not get proper breadcrumbs. I opened a PR to fix this: #12137 and ensure this is consistent, including passing the request/response as hints!

@TheHolyWaffle
Copy link
Author
TheHolyWaffle commented May 21, 2024 •
edited
Loading

This may be related: I had to set enableTracing: true in order for beforeBreadcrumb to be called. Before v8, this flag wasn't necessary.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 21, 2024
@mydea
Copy link
Member
mydea commented May 21, 2024

This may be related: I had to set enableTracing: true in order for beforeBreadcrumb to be called. Before v8, this flag wasn't necessary.

Yeah, this is also fixed in my PR - should not be necessary!

mydea added a commit that referenced this issue May 22, 2024
@mydea
fix(node): Ensure fetch/http breadcrumbs are created correctly (#12137)
a7aa8a0 
This PR ensures that (node) fetch and http requests generate breadcrumbs
correctly, even if tracing is disabled.

It also adds tests for this, and ensures we pass the request/response to
the hint correctly.

Fixes #12132
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Package: node Issues related to the Sentry Node SDK
Projects
GitHub Issues with 👀 3
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(node): Ensure fetch/http breadcrumbs are created correctly getsentry/sentry-javascript
2 participants
@TheHolyWaffle @mydea
0