Cannot read property 'unshift' of undefined
Opened this issue · 1 comments
alexb-uk commented
Error trace
TypeError: Cannot read property 'unshift' of undefined
at Subsegment.addError (/usr/src/app/node_modules/aws-xray-sdk-core/lib/segments/attributes/subsegment.js:210:25)
at /usr/src/app/node_modules/blah/blah/src/index.ts:305:18
at clsBind (/usr/src/app/node_modules/cls-hooked/context.js:172:17)
at runMicrotasks (<anonymous>)
at processTicksAndRejections (internal/process/task_queues.js:93:5)
Steps to reproduce
I believe the scenario is the following:
- Call addError() with an error
. triggers lines 16 and 25 below - Call addError() again with the same error object
. triggers IF on line 8 which sets this.cause without exceptions: [] - Call addError() again but with a different error object
. triggers line 30 and the unshift error.
Code snippet
Subsegment.prototype.addError = function addError(err, remote) {
if (err == null || typeof err !== 'object' && typeof (err) !== 'string') {
throw new Error('Failed to add error:' + err + ' to subsegment "' + this.name +
'". Not an object or string literal.');
}
this.addFaultFlag();
if (this.segment && this.segment.exception) {
if (err === this.segment.exception.ex) {
this.fault = true;
this.cause = { id: this.segment.exception.cause };
return;
}
delete this.segment.exception;
}
if (this.segment) {
this.segment.exception = {
ex: err,
cause: this.id
};
}
else {
//error, cannot propagate exception if not added to segment
}
if (this.cause === undefined) {
this.cause = {
working_directory: process.cwd(),
exceptions: []
};
}
this.cause.exceptions.unshift(new CapturedException(err, remote));
};
I believe this scenario is an anti-pattern in our code but I'm wondering why line 10 in the code snippet above does not prime the array like is done further down?
i.e. this.cause = { id: this.segment.exception.cause, exceptions: [] };
Am happy to submit a PR if that won't cause any other knock on issues? This would make the code more defensive without having to put a condition check on line 30.
e.g. this.cause.exceptions && this.cause.exceptions.unshift(new CapturedException(err, remote));
Thanks
willarmiros commented
Hi @alexb-uk,
Thanks for raising this, it does seem like a missed check on our end! We would be very open to such a PR.