From 35255bd71df5d7f6a4dc422c43934fa5f479ddac Mon Sep 17 00:00:00 2001 From: Jarid Margolin Date: Mon, 18 Mar 2024 10:40:38 -0700 Subject: [PATCH] Fix server autoinstrument mergeOptions behavior in node v20+ (#1136) --- src/server/telemetry/urlHelpers.js | 7 +------ test/server.telemetry.test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/server/telemetry/urlHelpers.js b/src/server/telemetry/urlHelpers.js index 8747a926..607ad099 100644 --- a/src/server/telemetry/urlHelpers.js +++ b/src/server/telemetry/urlHelpers.js @@ -1,4 +1,3 @@ -var url = require('url'); var { URL } = require('url'); var merge = require('../../merge'); @@ -12,11 +11,7 @@ function mergeOptions(input, options, cb) { if (typeof input === 'string') { const urlStr = input; input = urlToHttpOptions(new URL(urlStr)); - } else if ( - input && - input[url.searchParamsSymbol] && - input[url.searchParamsSymbol][url.searchParamsSymbol] - ) { + } else if (input && input instanceof URL) { // url.URL instance input = urlToHttpOptions(input); } else { diff --git a/test/server.telemetry.test.js b/test/server.telemetry.test.js index 98e49f77..4a522c27 100644 --- a/test/server.telemetry.test.js +++ b/test/server.telemetry.test.js @@ -9,6 +9,8 @@ var https = require('https'); process.env.NODE_ENV = process.env.NODE_ENV || 'test-node-env'; var Rollbar = require('../src/server/rollbar'); +var { mergeOptions } = require('../src/server/telemetry/urlHelpers'); +const { URL } = require('url'); function wait(ms) { return new Promise((resolve) => { @@ -413,4 +415,29 @@ vows }, }, }) + .addBatch({ + 'while using autoinstrument': { + topic: function () { + const optionsUsingStringUrl = mergeOptions( + 'http://example.com/api/users', + { method: 'GET', headers: testHeaders1 }, + ); + const optionsUsingClassUrl = mergeOptions( + new URL('http://example.com/api/users'), + { method: 'GET', headers: testHeaders1 }, + ); + + return { + optionsUsingStringUrl, + optionsUsingClassUrl, + }; + }, + 'mergeOptions should correctly handle URL and options': function ({ + optionsUsingStringUrl, + optionsUsingClassUrl, + }) { + assert.deepStrictEqual(optionsUsingStringUrl, optionsUsingClassUrl); + }, + }, + }) .export(module, { error: false });