From 6d45f230e6293084a2198b7efd6b5e0a10e136e7 Mon Sep 17 00:00:00 2001 From: Bernd Fuhrmann Date: Mon, 18 Nov 2024 10:51:37 +0100 Subject: [PATCH 1/5] fix 648 url object --- sdk_contrib/fetch/lib/fetch_p.js | 2 +- .../fetch/test/integration/fetch_p.test.js | 20 +++++++++++++++++++ sdk_contrib/fetch/test/unit/fetch_p.test.js | 5 +++-- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/sdk_contrib/fetch/lib/fetch_p.js b/sdk_contrib/fetch/lib/fetch_p.js index dd7c4f2c..dc8b1dca 100644 --- a/sdk_contrib/fetch/lib/fetch_p.js +++ b/sdk_contrib/fetch/lib/fetch_p.js @@ -71,7 +71,7 @@ function enableCapture(baseFetchFunction, requestClass, downstreamXRayEnabled, s const thisDownstreamXRayEnabled = !!downstreamXRayEnabled; const thisSubsegmentCallback = subsegmentCallback; // Standardize request information - const request = typeof args[0] === 'object' ? + const request = args[0] instanceof requestClass ? args[0] : new requestClass(...args); diff --git a/sdk_contrib/fetch/test/integration/fetch_p.test.js b/sdk_contrib/fetch/test/integration/fetch_p.test.js index 3d633e38..25ffccc2 100644 --- a/sdk_contrib/fetch/test/integration/fetch_p.test.js +++ b/sdk_contrib/fetch/test/integration/fetch_p.test.js @@ -111,6 +111,26 @@ describe('Integration tests', function () { stubClose.should.have.been.calledOnce; }); + it('works with stringifyable objects', async function () { + const spyCallback = sandbox.spy(); + const fetch = captureFetchGlobal(true, spyCallback); + const response = await fetch(new URL(goodUrl), { + headers: { + 'foo': 'bar' + } + }); + response.status.should.equal(200); + receivedHeaders.should.to.have.property('x-amzn-trace-id'); + receivedHeaders.should.to.have.property('foo', 'bar'); + (await response.text()).should.contain('Example'); + stubIsAutomaticMode.should.have.been.called; + stubAddNewSubsegment.should.have.been.calledOnce; + stubResolveSegment.should.have.been.calledOnce; + stubAddFetchRequestData.should.have.been.calledOnce; + stubAddErrorFlag.should.not.have.been.calledOnce; + stubClose.should.have.been.calledOnce; + }); + it('sets error flag on failed fetch when global fetch exists', async function () { const spyCallback = sandbox.spy(); const fetch = captureFetchGlobal(true, spyCallback); diff --git a/sdk_contrib/fetch/test/unit/fetch_p.test.js b/sdk_contrib/fetch/test/unit/fetch_p.test.js index 4dde8d52..0d008036 100644 --- a/sdk_contrib/fetch/test/unit/fetch_p.test.js +++ b/sdk_contrib/fetch/test/unit/fetch_p.test.js @@ -1,4 +1,5 @@ const { Subsegment } = require('aws-xray-sdk-core'); +const { Agent } = require('http'); const fetch = require('node-fetch'); describe('Unit tests', function () { @@ -157,13 +158,13 @@ describe('Unit tests', function () { it('short circuits if headers include trace ID', async function () { const activeFetch = captureFetch(true); - const request = new fetchModule.Request('https://www.foo.com', { + const request = new Request('https://www.foo.com', { headers: { 'X-Amzn-Trace-Id': '12345' } }); await activeFetch(request); - stubFetch.should.have.been.calledOnceWith(request); + stubFetch.should.have.been.calledOnceWith(sinon.match({ url: 'https://www.foo.com/'})); stubResolveSegment.should.not.have.been.called; }); From cec94c3a01eb10cf00281bd98bb8b34ea19ef15f Mon Sep 17 00:00:00 2001 From: Bernd Fuhrmann Date: Mon, 18 Nov 2024 11:18:36 +0100 Subject: [PATCH 2/5] fix 650 dispatcher --- sdk_contrib/fetch/lib/fetch_p.js | 26 ++++++++++++++++++++- sdk_contrib/fetch/test/unit/fetch_p.test.js | 22 +++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/sdk_contrib/fetch/lib/fetch_p.js b/sdk_contrib/fetch/lib/fetch_p.js index dd7c4f2c..22aa2ccd 100644 --- a/sdk_contrib/fetch/lib/fetch_p.js +++ b/sdk_contrib/fetch/lib/fetch_p.js @@ -54,6 +54,27 @@ function captureFetchModule(module, downstreamXRayEnabled, subsegmentCallback) { return module.default; } +function transferDispatcherNodeJSBuggyUndici(request, requestClone) { + const dispatcherSymbol = Object.getOwnPropertySymbols(request).find(symbol => symbol.description === 'dispatcher'); + if (dispatcherSymbol) { + requestClone[dispatcherSymbol] = request[dispatcherSymbol]; + } +} + +function getTransferDispatcherNodeJSBuggyUndici(requestClass) { + try { + const { Agent } = require('http'); + const request = new requestClass({ dispatcher: new Agent }); + const requestClone = request.clone(); + const dispatcherSymbol = Object.getOwnPropertySymbols(request).find(symbol => symbol.description === 'dispatcher'); + if (request[dispatcherSymbol] && !requestClone[dispatcherSymbol]) { + return transferDispatcherNodeJSBuggyUndici; + } + } catch { + } + return function() {}; +} + /** * Return a fetch function that will pass segment information to the target host. * This does not change any globals @@ -66,7 +87,7 @@ function captureFetchModule(module, downstreamXRayEnabled, subsegmentCallback) { * @returns Response */ function enableCapture(baseFetchFunction, requestClass, downstreamXRayEnabled, subsegmentCallback) { - + const transferDispatcher = getTransferDispatcherNodeJSBuggyUndici(requestClass); const overridenFetchAsync = async (...args) => { const thisDownstreamXRayEnabled = !!downstreamXRayEnabled; const thisSubsegmentCallback = subsegmentCallback; @@ -125,6 +146,9 @@ function enableCapture(baseFetchFunction, requestClass, downstreamXRayEnabled, s // Set up fetch call and capture any thrown errors const capturedFetch = async () => { const requestClone = request.clone(); + + transferDispatcher(request, requestClone); + let response; try { response = await baseFetchFunction(requestClone); diff --git a/sdk_contrib/fetch/test/unit/fetch_p.test.js b/sdk_contrib/fetch/test/unit/fetch_p.test.js index 4dde8d52..55af8dfd 100644 --- a/sdk_contrib/fetch/test/unit/fetch_p.test.js +++ b/sdk_contrib/fetch/test/unit/fetch_p.test.js @@ -1,4 +1,5 @@ const { Subsegment } = require('aws-xray-sdk-core'); +const { Agent } = require('http'); const fetch = require('node-fetch'); describe('Unit tests', function () { @@ -167,6 +168,27 @@ describe('Unit tests', function () { stubResolveSegment.should.not.have.been.called; }); + it('transfers dispatcher property for undici', async function () { + const activeFetch = captureFetch(true); + const agent = new Agent({ + maxSockets: 1234 + }); + await activeFetch('https://www.foo.com', { + dispatcher: agent + }); + stubFetch.should.have.been.calledOnceWith(sinon.match({ url: 'https://www.foo.com/' })); + stubResolveSegment.should.have.been.called; + + // check if dispatcher was transferred + const dummyRequest = new Request('https://www.foo.com', { + dispatcher: agent + }); + const dispatcherSymbol = Object.getOwnPropertySymbols(dummyRequest).find(symbol => symbol.description === 'dispatcher'); + if (dispatcherSymbol) { + stubFetch.should.have.been.calledOnceWith(sinon.match({ [dispatcherSymbol]: agent })); + } + }); + it('calls base function when no parent and automatic mode', async function () { const activeFetch = captureFetch(true); stubResolveSegment.returns(null); From 4f05b21044bc9cf220a2d6eaba9608a01220576c Mon Sep 17 00:00:00 2001 From: Bernd Fuhrmann Date: Mon, 18 Nov 2024 11:19:09 +0100 Subject: [PATCH 3/5] fix linter warning --- sdk_contrib/fetch/test/unit/fetch_p.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk_contrib/fetch/test/unit/fetch_p.test.js b/sdk_contrib/fetch/test/unit/fetch_p.test.js index 0d008036..858689f8 100644 --- a/sdk_contrib/fetch/test/unit/fetch_p.test.js +++ b/sdk_contrib/fetch/test/unit/fetch_p.test.js @@ -1,5 +1,4 @@ const { Subsegment } = require('aws-xray-sdk-core'); -const { Agent } = require('http'); const fetch = require('node-fetch'); describe('Unit tests', function () { From c5008527f8bc7484471dcdef3f08388abfd60be7 Mon Sep 17 00:00:00 2001 From: Bernd Fuhrmann Date: Mon, 18 Nov 2024 14:01:59 +0100 Subject: [PATCH 4/5] fix case where Request and init was supplied --- sdk_contrib/fetch/lib/fetch_p.js | 2 +- sdk_contrib/fetch/test/unit/fetch_p.test.js | 42 +++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/sdk_contrib/fetch/lib/fetch_p.js b/sdk_contrib/fetch/lib/fetch_p.js index 95af39ff..4686078c 100644 --- a/sdk_contrib/fetch/lib/fetch_p.js +++ b/sdk_contrib/fetch/lib/fetch_p.js @@ -92,7 +92,7 @@ function enableCapture(baseFetchFunction, requestClass, downstreamXRayEnabled, s const thisDownstreamXRayEnabled = !!downstreamXRayEnabled; const thisSubsegmentCallback = subsegmentCallback; // Standardize request information - const request = args[0] instanceof requestClass ? + const request = (args[0] instanceof requestClass && args.length === 1) ? args[0] : new requestClass(...args); diff --git a/sdk_contrib/fetch/test/unit/fetch_p.test.js b/sdk_contrib/fetch/test/unit/fetch_p.test.js index c1cb11b0..8441c45d 100644 --- a/sdk_contrib/fetch/test/unit/fetch_p.test.js +++ b/sdk_contrib/fetch/test/unit/fetch_p.test.js @@ -189,6 +189,48 @@ describe('Unit tests', function () { } }); + it('transfers dispatcher property for undici with Request object', async function () { + const activeFetch = captureFetch(true); + const agent = new Agent({ + maxSockets: 1234 + }); + await activeFetch(new Request('https://www.foo.com'), { + dispatcher: agent + }); + stubFetch.should.have.been.calledOnceWith(sinon.match({ url: 'https://www.foo.com/' })); + stubResolveSegment.should.have.been.called; + + // check if dispatcher was transferred + const dummyRequest = new Request('https://www.foo.com', { + dispatcher: agent + }); + const dispatcherSymbol = Object.getOwnPropertySymbols(dummyRequest).find(symbol => symbol.description === 'dispatcher'); + if (dispatcherSymbol) { + stubFetch.should.have.been.calledOnceWith(sinon.match({ [dispatcherSymbol]: agent })); + } + }); + + it('transfers dispatcher property for undici with url and init', async function () { + const activeFetch = captureFetch(true); + const agent = new Agent({ + maxSockets: 1234 + }); + await activeFetch('https://www.foo.com', { + dispatcher: agent + }); + stubFetch.should.have.been.calledOnceWith(sinon.match({ url: 'https://www.foo.com/' })); + stubResolveSegment.should.have.been.called; + + // check if dispatcher was transferred + const dummyRequest = new Request('https://www.foo.com', { + dispatcher: agent + }); + const dispatcherSymbol = Object.getOwnPropertySymbols(dummyRequest).find(symbol => symbol.description === 'dispatcher'); + if (dispatcherSymbol) { + stubFetch.should.have.been.calledOnceWith(sinon.match({ [dispatcherSymbol]: agent })); + } + }); + it('calls base function when no parent and automatic mode', async function () { const activeFetch = captureFetch(true); stubResolveSegment.returns(null); From 26f5e6d16cbc848aef0da7fa594b77fb65930586 Mon Sep 17 00:00:00 2001 From: Bernd Fuhrmann Date: Fri, 22 Nov 2024 08:47:42 +0100 Subject: [PATCH 5/5] fix test --- sdk_contrib/fetch/test/unit/fetch_p.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk_contrib/fetch/test/unit/fetch_p.test.js b/sdk_contrib/fetch/test/unit/fetch_p.test.js index 8441c45d..d8b74912 100644 --- a/sdk_contrib/fetch/test/unit/fetch_p.test.js +++ b/sdk_contrib/fetch/test/unit/fetch_p.test.js @@ -158,7 +158,7 @@ describe('Unit tests', function () { it('short circuits if headers include trace ID', async function () { const activeFetch = captureFetch(true); - const request = new Request('https://www.foo.com', { + const request = new FetchRequest('https://www.foo.com', { headers: { 'X-Amzn-Trace-Id': '12345' } @@ -180,7 +180,7 @@ describe('Unit tests', function () { stubResolveSegment.should.have.been.called; // check if dispatcher was transferred - const dummyRequest = new Request('https://www.foo.com', { + const dummyRequest = new FetchRequest('https://www.foo.com', { dispatcher: agent }); const dispatcherSymbol = Object.getOwnPropertySymbols(dummyRequest).find(symbol => symbol.description === 'dispatcher'); @@ -194,14 +194,14 @@ describe('Unit tests', function () { const agent = new Agent({ maxSockets: 1234 }); - await activeFetch(new Request('https://www.foo.com'), { + await activeFetch(new FetchRequest('https://www.foo.com'), { dispatcher: agent }); stubFetch.should.have.been.calledOnceWith(sinon.match({ url: 'https://www.foo.com/' })); stubResolveSegment.should.have.been.called; // check if dispatcher was transferred - const dummyRequest = new Request('https://www.foo.com', { + const dummyRequest = new FetchRequest('https://www.foo.com', { dispatcher: agent }); const dispatcherSymbol = Object.getOwnPropertySymbols(dummyRequest).find(symbol => symbol.description === 'dispatcher'); @@ -222,7 +222,7 @@ describe('Unit tests', function () { stubResolveSegment.should.have.been.called; // check if dispatcher was transferred - const dummyRequest = new Request('https://www.foo.com', { + const dummyRequest = new FetchRequest('https://www.foo.com', { dispatcher: agent }); const dispatcherSymbol = Object.getOwnPropertySymbols(dummyRequest).find(symbol => symbol.description === 'dispatcher');