Repository navigation
beforeBreadcrumb can no longer access original Request object in v8 #12132
Description
Is there an existing issue for this?
- I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
- I have reviewed the documentation https://docs.sentry.io/
- I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases
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
- Make an native node fetch api call
- 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
Activity
mydea commentedon May 21, 2024
Hey, thanks for writing in - that is indeed not on purpose, we'll add this back, sorry about the inconvenience!
TheHolyWaffle commentedon May 21, 2024
@mydea Thanks for looking into this. Just to double-check this desired behavior is expected for both
httpIntegrationandnativeNodeFetchIntegration🙏mydea commentedon 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 commentedon May 21, 2024
This may be related: I had to set
enableTracing: truein order forbeforeBreadcrumbto be called. Before v8, this flag wasn't necessary.mydea commentedon May 21, 2024
Yeah, this is also fixed in my PR - should not be necessary!
fix(node): Ensure fetch/http breadcrumbs are created correctly (#12137)