Skip to content

Commit e28c1af

Browse files
Fix missing headers for fetch (#687)
* fix: test typo * attempt to fix #679 * export enableCapture * fix type * fix type for undici * edits to package-lock.json(s) --------- Co-authored-by: jjllee <[email protected]>
1 parent 9552603 commit e28c1af

File tree

7 files changed

+75
-21
lines changed

7 files changed

+75
-21
lines changed

Diff for: package-lock.json

+3-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: sdk_contrib/fetch/README.md

+1
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,4 @@ I could find that this will happen, but throwing it out there "just in case".
4343
## Contributors
4444

4545
- [Jason Terando](https://github.com/jasonterando)
46+
- [Bernd Fuhrmann](https://github.com/berndfuhrmann)

Diff for: sdk_contrib/fetch/lib/fetch_p.d.ts

+6
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,9 @@ export function captureFetchModule(
1717
subsegmentCallback?: (subsegment: AWSXRay.Subsegment, req: fetchModule.Request, res: fetchModule.Response | null, error?: Error | undefined) => void):
1818
(url: URL | fetchModule.RequestInfo, init?: fetchModule.RequestInit | undefined) => Promise<fetchModule.Response>;
1919

20+
export function enableCapture<Fetch, Request>(
21+
fetch: Fetch,
22+
request: Request,
23+
downstreamXRayEnabled?: boolean,
24+
subsegmentCallback?: (subsegment: AWSXRay.Subsegment, req: Request, res: Response | null, error?: Error | undefined) => void
25+
): Fetch;

Diff for: sdk_contrib/fetch/lib/fetch_p.js

+15-4
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,18 @@ function captureFetchModule(module, downstreamXRayEnabled, subsegmentCallback) {
5454
return module.default;
5555
}
5656

57-
const enableCapture = function enableCapture(baseFetchFunction, requestClass, downstreamXRayEnabled, subsegmentCallback) {
57+
/**
58+
* Return a fetch function that will pass segment information to the target host.
59+
* This does not change any globals
60+
* @param {function} baseFetchFunction fetch function to use as basis
61+
* @param {function} requestClass Request class to use. This should correspond to the supplied fetch function.
62+
* @param {boolean} downstreamXRayEnabled - when true, adds a "traced:true" property to the subsegment
63+
* so the AWS X-Ray service expects a corresponding segment from the downstream service.
64+
* @param {function} subsegmentCallback - a callback that is called with the subsegment, the fetch request,
65+
* the fetch response and any error issued, allowing custom annotations and metadata to be added.
66+
* @returns Response
67+
*/
68+
function enableCapture(baseFetchFunction, requestClass, downstreamXRayEnabled, subsegmentCallback) {
5869

5970
const overridenFetchAsync = async (...args) => {
6071
const thisDownstreamXRayEnabled = !!downstreamXRayEnabled;
@@ -116,7 +127,7 @@ const enableCapture = function enableCapture(baseFetchFunction, requestClass, do
116127
const requestClone = request.clone();
117128
let response;
118129
try {
119-
response = await baseFetchFunction(...args);
130+
response = await baseFetchFunction(requestClone);
120131

121132
if (thisSubsegmentCallback) {
122133
thisSubsegmentCallback(subsegment, requestClone, response);
@@ -159,8 +170,8 @@ const enableCapture = function enableCapture(baseFetchFunction, requestClass, do
159170
};
160171

161172
return overridenFetchAsync;
162-
};
173+
}
163174

164175
module.exports.captureFetchGlobal = captureFetchGlobal;
165176
module.exports.captureFetchModule = captureFetchModule;
166-
module.exports._fetchEnableCapture = enableCapture;
177+
module.exports.enableCapture = enableCapture;

Diff for: sdk_contrib/fetch/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
"repository": "https://github.com/aws/aws-xray-sdk-node/tree/master/sdk_contrib/fetch",
4141
"devDependencies": {
4242
"@types/node-fetch": "^2.6.4",
43-
"chai-as-promised": "^7.1.1",
43+
"chai-as-promised": "=7.1.1",
4444
"node-fetch": "^2.6.11",
4545
"ts-expect": "^1.3.0"
4646
},

Diff for: sdk_contrib/fetch/test/integration/fetch_p.test.js

+49-2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,31 @@
1+
2+
let listener;
3+
let server;
4+
let goodUrl;
5+
let receivedHeaders;
6+
7+
8+
before(() => {
9+
// Create an HTTP server to receive requests.
10+
const http = require('http');
11+
server = http.createServer((req, res) => {
12+
receivedHeaders = { ...req.headers };
13+
// Respond with something
14+
res.writeHead(200, { 'Content-Type': 'text/plain' });
15+
res.end('Example reply\n');
16+
});
17+
18+
listener = server.listen();
19+
const address = server.address();
20+
const host = address.family === 'IPv6' ? `[${address.address}]` : address.address;
21+
goodUrl = `http://${host}:${address.port}/test`;
22+
});
23+
24+
after(() => {
25+
// close http server
26+
listener.close();
27+
});
28+
129
describe('Integration tests', function () {
230
const chai = require('chai');
331
const sinonChai = require('sinon-chai');
@@ -16,12 +44,11 @@ describe('Integration tests', function () {
1644
const fetchModule = require('node-fetch');
1745
const Segment = AWSXray.Segment;
1846
const Subsegment = AWSXray.Subsegment;
19-
const goodUrl = 'https://example.org';
2047
const badUrl = 'http://localhost:1';
2148

2249
const hasGlobalFetch = globalThis.fetch !== undefined;
2350

24-
describe('captureFetchModule', function () {
51+
describe('captureFetchGlobal', function () {
2552

2653
let saveGlobalFetch;
2754
let mockSegment;
@@ -43,6 +70,7 @@ describe('Integration tests', function () {
4370
stubAddFetchRequestData = sandbox.stub(mockSubsegment, 'addFetchRequestData');
4471
stubAddErrorFlag = sandbox.stub(mockSubsegment, 'addErrorFlag');
4572
stubClose = sandbox.stub(mockSubsegment, 'close');
73+
receivedHeaders = {};
4674
});
4775

4876
afterEach(function () {
@@ -65,6 +93,24 @@ describe('Integration tests', function () {
6593
stubClose.should.have.been.calledOnce;
6694
});
6795

96+
it('adds header', async function () {
97+
const spyCallback = sandbox.spy();
98+
const fetch = captureFetchGlobal(true, spyCallback);
99+
const response = await fetch(goodUrl, { headers: {
100+
'foo': 'bar'
101+
}});
102+
response.status.should.equal(200);
103+
receivedHeaders.should.to.have.property('x-amzn-trace-id');
104+
receivedHeaders.should.to.have.property('foo', 'bar');
105+
(await response.text()).should.contain('Example');
106+
stubIsAutomaticMode.should.have.been.called;
107+
stubAddNewSubsegment.should.have.been.calledOnce;
108+
stubResolveSegment.should.have.been.calledOnce;
109+
stubAddFetchRequestData.should.have.been.calledOnce;
110+
stubAddErrorFlag.should.not.have.been.calledOnce;
111+
stubClose.should.have.been.calledOnce;
112+
});
113+
68114
it('sets error flag on failed fetch when global fetch exists', async function () {
69115
const spyCallback = sandbox.spy();
70116
const fetch = captureFetchGlobal(true, spyCallback);
@@ -116,6 +162,7 @@ describe('Integration tests', function () {
116162
const fetch = captureFetchModule(fetchModule, true, spyCallback);
117163
const response = await fetch(goodUrl);
118164
response.status.should.equal(200);
165+
receivedHeaders.should.to.have.property('x-amzn-trace-id');
119166
(await response.text()).should.contain('Example');
120167
stubIsAutomaticMode.should.have.been.called;
121168
stubAddNewSubsegment.should.have.been.calledOnce;

Diff for: sdk_contrib/fetch/test/unit/fetch_p.test.js

-11
Original file line numberDiff line numberDiff line change
@@ -339,17 +339,6 @@ describe('Unit tests', function () {
339339
response.should.equal(stubValidResponse);
340340
});
341341

342-
it('resolves to response through proxy when fetch options are supplied', async function() {
343-
const activeFetch = captureFetch(true);
344-
const proxyStub = sinon.stub();
345-
const request = new FetchRequest('https://www.foo.com/test');
346-
const response = await activeFetch(request, {
347-
dispatcher: proxyStub
348-
});
349-
stubFetch.should.have.been.calledOnceWith(request, {dispatcher: proxyStub});
350-
response.should.equal(stubValidResponse);
351-
});
352-
353342
it('calls subsegmentCallback with error upon fetch throwing', async function () {
354343
const spyCallback = sandbox.spy();
355344
const activeFetch = captureFetch(true, spyCallback);

0 commit comments

Comments
 (0)