fix: prevent invalid error stack traces (#617)

Author: patrickhousley

src/features/jserrors/constants.js

@@ -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'

src/features/jserrors/instrument/index.js

@@ -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
@@ -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))
   }
 
   /**
@@ -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
@@ -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
   }
 
   /**
@@ -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)
   }
 }

tests/functional/err/unhandled-rejection-readable.test.js

@@ -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' },

tests/functional/spa/errors.test.js

@@ -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')