-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix(node): fill in span data from http request options object #9112
fix(node): fill in span data from http request options object #9112
Conversation
d78a83f
to
60169cd
Compare
60169cd
to
6c4c1d1
Compare
@@ -168,6 +168,16 @@ export function normalizeRequestArgs( | |||
requestOptions = urlToOptions(requestArgs[0]); | |||
} else { | |||
requestOptions = requestArgs[0]; | |||
|
|||
if (!requestOptions.pathname || !requestOptions.search) { | |||
const parsed = new URL( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things:
- Let's try-catch this to ensure this does not fail if we cannot generate an URL there, just to be on the safe side
- Let's instead set the parsed values only if they aren't set.
So something like this:
try {
const parsed = new URL(...);
requestOptions = {
pathname: parsed.pathname,
search: parsed.search,
hash: parsed.hash,
...requestOptions
};
} catch (e) {
// ignore errors here
}
Thank you for the PR! That makes sense to me, I left one comment, if you could address this I can merge this in - thanks a lot! |
6c4c1d1
to
4de669b
Compare
Thanks @mydea! I've made the changes |
Thank you for the PR! 🚀 |
Currently, span data is missing for Node.js requests created with options object as the only argument, such as `http.request(options)`.
This PR was released with https://github.com/getsentry/sentry-javascript/releases/tag/7.73.0. Thanks for your help @vlad-zhukov! |
Currently, span data is missing for Node.js requests created with options object as the only argument, such as
http.request(options)
.