Skip to content

Commit 23a1697

Browse files
authored
fix(node): remove new URL usage in Undici integration (#8147)
1 parent 061eca5 commit 23a1697

File tree

2 files changed

+37
-11
lines changed

2 files changed

+37
-11
lines changed

packages/node/src/integrations/undici/index.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@ import type { EventProcessor, Integration } from '@sentry/types';
33
import {
44
dynamicRequire,
55
dynamicSamplingContextToSentryBaggageHeader,
6+
getSanitizedUrlString,
7+
parseUrl,
68
stringMatchesSomePattern,
7-
stripUrlQueryAndFragment,
89
} from '@sentry/utils';
910
import { LRUMap } from 'lru_map';
1011

@@ -36,6 +37,15 @@ export interface UndiciOptions {
3637
// Please note that you cannot use `console.log` to debug the callbacks registered to the `diagnostics_channel` API.
3738
// To debug, you can use `writeFileSync` to write to a file:
3839
// https://nodejs.org/api/async_hooks.html#printing-in-asynchook-callbacks
40+
//
41+
// import { writeFileSync } from 'fs';
42+
// import { format } from 'util';
43+
//
44+
// function debug(...args: any): void {
45+
// // Use a function like this one when debugging inside an AsyncHook callback
46+
// // @ts-ignore any
47+
// writeFileSync('log.out', `${format(...args)}\n`, { flag: 'a' });
48+
// }
3949

4050
/**
4151
* Instruments outgoing HTTP requests made with the `undici` package via
@@ -113,8 +123,8 @@ export class Undici implements Integration {
113123

114124
const { request } = message as RequestCreateMessage;
115125

116-
const url = new URL(request.path, request.origin);
117-
const stringUrl = url.toString();
126+
const stringUrl = request.origin ? request.origin.toString() + request.path : request.path;
127+
const url = parseUrl(stringUrl);
118128

119129
if (isSentryRequest(stringUrl) || request.__sentry__ !== undefined) {
120130
return;
@@ -133,16 +143,15 @@ export class Undici implements Integration {
133143
const data: Record<string, unknown> = {
134144
'http.method': method,
135145
};
136-
const params = url.searchParams.toString();
137-
if (params) {
138-
data['http.query'] = `?${params}`;
146+
if (url.search) {
147+
data['http.query'] = url.search;
139148
}
140149
if (url.hash) {
141150
data['http.fragment'] = url.hash;
142151
}
143152
const span = activeSpan.startChild({
144153
op: 'http.client',
145-
description: `${method} ${stripUrlQueryAndFragment(stringUrl)}`,
154+
description: `${method} ${getSanitizedUrlString(url)}`,
146155
data,
147156
});
148157
request.__sentry__ = span;
@@ -184,8 +193,7 @@ export class Undici implements Integration {
184193

185194
const { request, response } = message as RequestEndMessage;
186195

187-
const url = new URL(request.path, request.origin);
188-
const stringUrl = url.toString();
196+
const stringUrl = request.origin ? request.origin.toString() + request.path : request.path;
189197

190198
if (isSentryRequest(stringUrl)) {
191199
return;
@@ -225,8 +233,7 @@ export class Undici implements Integration {
225233

226234
const { request } = message as RequestErrorMessage;
227235

228-
const url = new URL(request.path, request.origin);
229-
const stringUrl = url.toString();
236+
const stringUrl = request.origin ? request.origin.toString() + request.path : request.path;
230237

231238
if (isSentryRequest(stringUrl)) {
232239
return;

packages/node/test/integrations/undici.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,25 @@ conditionalTest({ min: 16 })('Undici integration', () => {
132132
expect(span).toEqual(expect.objectContaining({ status: 'internal_error' }));
133133
});
134134

135+
it('creates a span for invalid looking urls', async () => {
136+
const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction;
137+
hub.getScope().setSpan(transaction);
138+
139+
try {
140+
// Intentionally add // to the url
141+
// fetch accepts this URL, but throws an error later on
142+
await fetch('http://a-url-that-no-exists.com//');
143+
} catch (e) {
144+
// ignore
145+
}
146+
147+
expect(transaction.spanRecorder?.spans.length).toBe(2);
148+
149+
const span = transaction.spanRecorder?.spans[1];
150+
expect(span).toEqual(expect.objectContaining({ description: 'GET http://a-url-that-no-exists.com//' }));
151+
expect(span).toEqual(expect.objectContaining({ status: 'internal_error' }));
152+
});
153+
135154
it('does not create a span for sentry requests', async () => {
136155
const transaction = hub.startTransaction({ name: 'test-transaction' }) as Transaction;
137156
hub.getScope().setSpan(transaction);

0 commit comments

Comments
 (0)