Skip to content

Commit 35255bd

Browse files
Fix server autoinstrument mergeOptions behavior in node v20+ (#1136)
1 parent 55cf3ca commit 35255bd

File tree

2 files changed

+28
-6
lines changed

2 files changed

+28
-6
lines changed

src/server/telemetry/urlHelpers.js

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
var url = require('url');
21
var { URL } = require('url');
32
var merge = require('../../merge');
43

@@ -12,11 +11,7 @@ function mergeOptions(input, options, cb) {
1211
if (typeof input === 'string') {
1312
const urlStr = input;
1413
input = urlToHttpOptions(new URL(urlStr));
15-
} else if (
16-
input &&
17-
input[url.searchParamsSymbol] &&
18-
input[url.searchParamsSymbol][url.searchParamsSymbol]
19-
) {
14+
} else if (input && input instanceof URL) {
2015
// url.URL instance
2116
input = urlToHttpOptions(input);
2217
} else {

test/server.telemetry.test.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ var https = require('https');
99

1010
process.env.NODE_ENV = process.env.NODE_ENV || 'test-node-env';
1111
var Rollbar = require('../src/server/rollbar');
12+
var { mergeOptions } = require('../src/server/telemetry/urlHelpers');
13+
const { URL } = require('url');
1214

1315
function wait(ms) {
1416
return new Promise((resolve) => {
@@ -413,4 +415,29 @@ vows
413415
},
414416
},
415417
})
418+
.addBatch({
419+
'while using autoinstrument': {
420+
topic: function () {
421+
const optionsUsingStringUrl = mergeOptions(
422+
'http://example.com/api/users',
423+
{ method: 'GET', headers: testHeaders1 },
424+
);
425+
const optionsUsingClassUrl = mergeOptions(
426+
new URL('http://example.com/api/users'),
427+
{ method: 'GET', headers: testHeaders1 },
428+
);
429+
430+
return {
431+
optionsUsingStringUrl,
432+
optionsUsingClassUrl,
433+
};
434+
},
435+
'mergeOptions should correctly handle URL and options': function ({
436+
optionsUsingStringUrl,
437+
optionsUsingClassUrl,
438+
}) {
439+
assert.deepStrictEqual(optionsUsingStringUrl, optionsUsingClassUrl);
440+
},
441+
},
442+
})
416443
.export(module, { error: false });

0 commit comments

Comments
 (0)