Cannot read property 'unshift' of undefined
- Dominant language
- JavaScript
- Stars
- 280
- Forks
- 157
- PR merge metrics
- No merged PRs in 30d
Description
### Error trace
```javascript
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 ()
at processTicksAndRejections (internal/process/task_queues.js:93:5)
```
### Steps to reproduce
I believe the scenario is the following:
1. Call addError() with an error
. triggers lines 16 and 25 below
2. Call addError() again with the same error object
. triggers IF on line 8 which sets this.cause without exceptions: []
3. Call addError() again but with a different error object
. triggers line 30 and the unshift error.
### Code snippet
```javascript
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
Contributor guide
Assessment
This issue has not been assessed yet.