Skip to content

Commit

Permalink
fix: prevent invalid error stack traces (#617)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhousley authored Jul 17, 2023
1 parent 14d4294 commit 3d9f2c0
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 12 deletions.
1 change: 0 additions & 1 deletion src/features/jserrors/constants.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { FEATURE_NAMES } from '../../loaders/features/features'

export const FEATURE_NAME = FEATURE_NAMES.jserrors
export const NR_ERR_PROP = 'nr@seenError'
43 changes: 34 additions & 9 deletions src/features/jserrors/instrument/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ export class Instrument extends InstrumentBase {

globalScope.addEventListener('error', (errorEvent) => {
if (!this.abortHandler) return

/**
* If the spa feature is loaded, errors may already have been captured in the `fn-err` listener above.
* This ensures those errors are not captured twice.
*/
if (this.#seenErrors.has(errorEvent.error)) {
this.#seenErrors.delete(errorEvent.error)
return
Expand All @@ -66,16 +71,31 @@ export class Instrument extends InstrumentBase {
this.abortHandler = undefined // weakly allow this abort op to run only once
}

/**
* Any value can be used with the `throw` keyword. This function ensures that the value is
* either a proper Error instance or attempts to convert it to an UncaughtError instance.
* @param {any} error The value thrown
* @returns {Error|UncaughtError} The converted error instance
*/
#castError (error) {
if (error instanceof Error) {
return error
}

if (typeof error.message !== 'undefined') {
return new UncaughtError(error.message, error.filename, error.lineno, error.colno)
/**
* The thrown value may contain a message property. If it does, try to treat the thrown
* value as an Error-like object.
*/
if (typeof error?.message !== 'undefined') {
return new UncaughtError(
error.message,
error.filename || error.sourceURL,
error.lineno || error.line,
error.colno || error.col
)
}

return new UncaughtError(error)
return new UncaughtError(typeof error === 'string' ? error : stringify(error))
}

/**
Expand All @@ -85,6 +105,7 @@ export class Instrument extends InstrumentBase {
*/
#castPromiseRejectionEvent (promiseRejectionEvent) {
let prefix = 'Unhandled Promise Rejection: '

if (promiseRejectionEvent?.reason instanceof Error) {
try {
promiseRejectionEvent.reason.message = prefix + promiseRejectionEvent.reason.message
Expand All @@ -93,12 +114,12 @@ export class Instrument extends InstrumentBase {
return promiseRejectionEvent.reason
}
}
if (typeof promiseRejectionEvent.reason === 'undefined') return new Error(prefix)
try {
return new Error(prefix + stringify(promiseRejectionEvent.reason))
} catch (e) {
return new Error(promiseRejectionEvent.reason)
}

if (typeof promiseRejectionEvent.reason === 'undefined') return new UncaughtError(prefix)

const error = this.#castError(promiseRejectionEvent.reason)
error.message = prefix + error.message
return error
}

/**
Expand All @@ -111,6 +132,10 @@ export class Instrument extends InstrumentBase {
return errorEvent.error
}

/**
* Older browsers do not contain the `error` property on the ErrorEvent instance.
* https://caniuse.com/mdn-api_errorevent_error
*/
return new UncaughtError(errorEvent.message, errorEvent.filename, errorEvent.lineno, errorEvent.colno)
}
}
2 changes: 1 addition & 1 deletion tests/functional/err/unhandled-rejection-readable.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ testDriver.test('unhandledPromiseRejections are caught and are readable', suppor
assertErrorAttributes(t, request.query)
const actualErrors = getErrorsFromResponse(request, browser)
const expectedErrorMessages = [
{ message: 'Unhandled Promise Rejection: "Test"', tested: false, meta: 'string' },
{ message: 'Unhandled Promise Rejection: Test', tested: false, meta: 'string' },
{ message: 'Unhandled Promise Rejection: 1', tested: false, meta: 'number' },
{ message: 'Unhandled Promise Rejection: {"a":1,"b":{"a":1}}', tested: false, meta: 'nested obj' },
{ message: 'Unhandled Promise Rejection: [1,2,3]', tested: false, meta: 'array' },
Expand Down
1 change: 0 additions & 1 deletion tests/functional/spa/errors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ testDriver.test('string error in custom tracer', function (t, browser, router) {
var nodeId = interactionTree.children[0].nodeId

var error = errors[0]
console.log(JSON.stringify(error))
t.equal(error.params.message, 'some error')
t.equal(error.params.browserInteractionId, interactionId, 'should have the correct interaction id')
t.equal(error.params.parentNodeId, nodeId, 'has the correct node id')
Expand Down

0 comments on commit 3d9f2c0

Please sign in to comment.