Skip to content

Commit d27bfd2

Browse files
authored
Fix error due to missing header in the response in resolveSampling method (#706)
* Fix error where header is missing in the response in resolveSampling method * update var naming
1 parent 72d523d commit d27bfd2

File tree

2 files changed

+81
-9
lines changed

2 files changed

+81
-9
lines changed

packages/core/lib/middleware/mw_utils.js

+6-4
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ var utils = {
106106
}
107107
}
108108

109-
if (amznTraceHeader.sampled === '?') {
110-
res.header[XRAY_HEADER] = 'Root=' + amznTraceHeader.root + ';Sampled=' + (isSampled ? '1' : '0');
109+
if (amznTraceHeader.sampled === '?' && res.header) {
110+
res.header(XRAY_HEADER, 'Root=' + amznTraceHeader.root + ';Sampled=' + (isSampled ? '1' : '0'));
111111
}
112112

113113
if (!isSampled) {
@@ -172,8 +172,10 @@ var utils = {
172172
var name = this.resolveName(req.headers.host);
173173
var segment = new Segment(name, amznTraceHeader.root, amznTraceHeader.parent);
174174

175-
var responseWithEmbeddedRequest = Object.assign({}, res, { req: req });
176-
this.resolveSampling(amznTraceHeader, segment, responseWithEmbeddedRequest);
175+
if (!res.req) {
176+
res.req = req;
177+
}
178+
this.resolveSampling(amznTraceHeader, segment, res);
177179

178180
segment.addIncomingRequestData(new IncomingRequestData(req));
179181

packages/core/test/unit/middleware/mw_utils.test.js

+75-5
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ describe('Middleware utils', function() {
1414
var envVarName = 'envDefaultName';
1515
var hostName = 'www.myhost.com';
1616
var traceId = '1-f9194208-2c7ad569f5d6ff149137be86';
17+
var parentId = '74051af127d2bcba';
1718

1819
function reloadMWUtils() {
1920
var path = '../../../lib/logger';
@@ -45,8 +46,6 @@ describe('Middleware utils', function() {
4546
});
4647

4748
describe('#processHeaders', function() {
48-
var parentId = '74051af127d2bcba';
49-
5049
it('should return an empty array on an undefined request', function() {
5150
var headers = MWUtils.processHeaders();
5251

@@ -149,7 +148,7 @@ describe('Middleware utils', function() {
149148
});
150149

151150
describe('#resolveSampling', function() {
152-
var res, sandbox, segment, shouldSampleStub;
151+
var res, sandbox, segment, responseHeaders, shouldSampleStub;
153152

154153
beforeEach(function() {
155154
sandbox = sinon.createSandbox();
@@ -158,18 +157,22 @@ describe('Middleware utils', function() {
158157
shouldSampleStub = sandbox.stub(MWUtils.sampler, 'shouldSample').returns(true);
159158

160159
segment = {};
160+
responseHeaders = {};
161161
res = {
162162
req: {
163163
headers: { host: 'moop.hello.com' },
164164
url: '/evergreen',
165165
method: 'GET',
166166
},
167-
header: {}
167+
header: (headerKey, headerValue) => {
168+
responseHeaders[headerKey] = headerValue;
169+
}
168170
};
169171
});
170172

171173
afterEach(function() {
172174
sandbox.restore();
175+
responseHeaders = {};
173176
});
174177

175178
it('should not mark segment as not traced if the sampled header is set to "1"', function() {
@@ -209,7 +212,7 @@ describe('Middleware utils', function() {
209212
MWUtils.resolveSampling(headers, segment, res);
210213

211214
var expected = new RegExp('^Root=' + traceId + ';Sampled=1$');
212-
assert.match(res.header[XRAY_HEADER], expected);
215+
assert.match(responseHeaders[XRAY_HEADER], expected);
213216
});
214217

215218
it('should mark segment as not traced if the sampling rules check returns false', function() {
@@ -220,6 +223,73 @@ describe('Middleware utils', function() {
220223

221224
assert.equal(segment.notTraced, true);
222225
});
226+
227+
it('should not throw error when res.header is undefined and Sampled=?', function() {
228+
var resWithoutHeader = {
229+
req: {
230+
headers: {},
231+
url: '/api/move/up',
232+
method: 'GET',
233+
}
234+
};
235+
shouldSampleStub.returns(false);
236+
var headers = { root: traceId, sampled: '?' };
237+
238+
assert.doesNotThrow(
239+
() => {
240+
MWUtils.resolveSampling(headers, segment, resWithoutHeader);
241+
}
242+
);
243+
244+
assert.equal(segment.notTraced, true);
245+
});
246+
});
247+
248+
249+
describe('#traceRequestResponseCycle', function() {
250+
var sandbox, shouldSampleStub;
251+
252+
beforeEach(function() {
253+
sandbox = sinon.createSandbox();
254+
MWUtils.sampler = localSampler;
255+
MWUtils.setDefaultName(defaultName);
256+
257+
shouldSampleStub = sandbox.stub(MWUtils.sampler, 'shouldSample').returns(true);
258+
});
259+
260+
afterEach(function() {
261+
sandbox.restore();
262+
});
263+
264+
it('should not throw error when Sampled=?', function() {
265+
var req = {
266+
headers: {
267+
url: '/api/move/up',
268+
[XRAY_HEADER]: 'Root=' + traceId + ';Parent=' + parentId + ';Sampled=?'
269+
},
270+
host: hostName,
271+
method: 'GET',
272+
};
273+
var segment;
274+
var responseHeaders = {};
275+
276+
var res = {
277+
req: req,
278+
on: (name, callback) => {},
279+
header: (headerKey, headerValue) => {
280+
responseHeaders[headerKey] = headerValue;
281+
}
282+
};
283+
shouldSampleStub.returns(false);
284+
285+
assert.doesNotThrow(
286+
() => {
287+
segment = MWUtils.traceRequestResponseCycle(req, res);
288+
}
289+
);
290+
assert.equal(segment.notTraced, true);
291+
assert.equal(responseHeaders[XRAY_HEADER], 'Root=' + traceId + ';Sampled=0');
292+
});
223293
});
224294

225295
describe('#samplingWildcardMatch', function() {

0 commit comments

Comments
 (0)