Skip to content

Commit 307f314

Browse files
authored
fix: captureError would fail on an Error attribute that is an invalid date (#2870)
Fixes: #2030
1 parent 6939c03 commit 307f314

File tree

3 files changed

+50
-3
lines changed

3 files changed

+50
-3
lines changed

CHANGELOG.asciidoc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ Notes:
5353
[float]
5454
===== Bug fixes
5555
56+
- Capturing an error would fail if the Error instance had an attribute that
57+
was an invalid date. ({issues}2030[#2030])
58+
5659
- Fix the span for an instrumented S3 ListBuckets API call to not be invalid
5760
for APM server intake. ({pull}2866[#2866])
5861

lib/errors.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,13 @@ function attributesFromErr (err) {
9191
continue
9292
case 'object':
9393
// Ignore all objects except Dates.
94-
if (typeof val.toISOString !== 'function') {
94+
if (typeof val.toISOString !== 'function' || typeof val.getTime !== 'function') {
9595
continue
96+
} else if (Number.isNaN(val.getTime())) {
97+
val = 'Invalid Date' // calling toISOString() on invalid dates throws
98+
} else {
99+
val = val.toISOString()
96100
}
97-
val = val.toISOString()
98101
}
99102
attrs[key] = val
100103
n++
@@ -280,5 +283,6 @@ module.exports = {
280283
createAPMError,
281284

282285
// Exported for testing.
286+
attributesFromErr,
283287
_moduleNameFromFrames
284288
}

test/errors.test.js

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const path = require('path')
1313
const tape = require('tape')
1414

1515
const logging = require('../lib/logging')
16-
const { createAPMError, generateErrorId, _moduleNameFromFrames } = require('../lib/errors')
16+
const { createAPMError, generateErrorId, attributesFromErr, _moduleNameFromFrames } = require('../lib/errors')
1717
const { dottedLookup } = require('./_utils')
1818

1919
const log = logging.createLogger('off')
@@ -331,3 +331,43 @@ tape.test('#_moduleNameFromFrames()', function (suite) {
331331

332332
suite.end()
333333
})
334+
335+
tape.test('#attributesFromErr()', function (suite) {
336+
var cases = [
337+
// 'err' is an Error instance, or a function that returns one.
338+
{
339+
name: 'no attrs',
340+
err: new Error('boom'),
341+
expectedAttrs: undefined
342+
},
343+
{
344+
name: 'string attr',
345+
err: () => {
346+
const err = new Error('boom')
347+
err.aStr = 'hello'
348+
return err
349+
},
350+
expectedAttrs: { aStr: 'hello' }
351+
},
352+
{
353+
name: 'Invalid Date attr',
354+
err: () => {
355+
const err = new Error('boom')
356+
err.aDate = new Date('invalid')
357+
return err
358+
},
359+
expectedAttrs: { aDate: 'Invalid Date' }
360+
}
361+
]
362+
363+
cases.forEach(function (opts) {
364+
suite.test(opts.name, function (t) {
365+
const err = typeof (opts.err) === 'function' ? opts.err() : opts.err
366+
const attrs = attributesFromErr(err)
367+
t.deepEqual(attrs, opts.expectedAttrs, 'got expected attrs')
368+
t.end()
369+
})
370+
})
371+
372+
suite.end()
373+
})

0 commit comments

Comments
 (0)