feat: Ignore unhandled promise rejections that lack a valid reason (#1233)

metal-messiah · web-flow · commit 25a1fffb91fa · 2024-11-01T09:08:21.000-05:00
diff --git a/src/features/jserrors/shared/cast-error.js b/src/features/jserrors/shared/cast-error.js
@@ -31,9 +31,16 @@ export function castError (error) {
    * @returns {Error} An Error object with the message as the casted reason
    */
 export function castPromiseRejectionEvent (promiseRejectionEvent) {
-  let prefix = 'Unhandled Promise Rejection'
+  const prefix = 'Unhandled Promise Rejection'
 
-  if (canTrustError(promiseRejectionEvent?.reason)) {
+  /**
+   * If the casted return value is falsy like this, it will get dropped and not produce an error event for harvest.
+   * We drop promise rejections that could not form a valid error stack or message deriving from the .reason attribute
+   * -- such as a manually invoked rejection without an argument -- since they lack reproduction value and create confusion.
+   * */
+  if (!promiseRejectionEvent?.reason) return
+
+  if (canTrustError(promiseRejectionEvent.reason)) {
     try {
       promiseRejectionEvent.reason.message = prefix + ': ' + promiseRejectionEvent.reason.message
       return castError(promiseRejectionEvent.reason)
@@ -42,8 +49,6 @@ export function castPromiseRejectionEvent (promiseRejectionEvent) {
     }
   }
 
-  if (typeof promiseRejectionEvent.reason === 'undefined') return castError(prefix)
-
   const error = castError(promiseRejectionEvent.reason)
   error.message = prefix + ': ' + error?.message
   return error
diff --git a/tests/assets/unhandled-promise-rejection-falsy-reason.html b/tests/assets/unhandled-promise-rejection-falsy-reason.html
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<!--
+  Copyright 2020 New Relic Corporation.
+  PDX-License-Identifier: Apache-2.0
+-->
+<html>
+  <head>
+    <title>RUM Unit Test</title>
+    {init} {config} {loader}
+  </head>
+
+  <body>
+    <script>
+      "use strict";
+       // falsy reasons, these should get dropped
+      new Promise(function(res, rej) { rej() });
+      new Promise(function(res, rej) { rej('') });
+      new Promise(function(res, rej) { rej(null) });
+    </script>
+    this is a generic page that is instrumented by the JS agent
+  </body>
+</html>
diff --git a/tests/assets/unhandled-promise-rejection-readable.html b/tests/assets/unhandled-promise-rejection-readable.html
@@ -52,6 +52,14 @@
         }
         rej(new Foo());
       });
+      new Promise(function(res, rej) {
+        throw new Error("test");
+      });
+      new Promise(function(res, rej) {
+        (function () {
+          throw new Error("test");
+        })()
+      });
     </script>
     this is a generic page that is instrumented by the JS agent
   </body>
diff --git a/tests/specs/err/scripts-and-callbacks.e2e.js b/tests/specs/err/scripts-and-callbacks.e2e.js
@@ -159,13 +159,13 @@ describe('JSE Error detection in various callbacks', () => {
     { message: 'Unhandled Promise Rejection: [1,2,3]', meta: 'array' },
     { message: 'Unhandled Promise Rejection: test', meta: 'error with message' },
     { message: 'test', meta: 'error with no setter with message' },
-    { message: 'Unhandled Promise Rejection', meta: 'undefined' },
-    { message: 'Unhandled Promise Rejection: null', meta: 'null' },
     { message: 'Unhandled Promise Rejection: ', meta: 'error with no message' },
     { message: 'Unhandled Promise Rejection: {}', meta: 'map object' },
     { message: 'Unhandled Promise Rejection: {"abc":"Hello"}', meta: 'factory function' },
     { message: 'Unhandled Promise Rejection: ', meta: 'uncalled function' },
-    { message: 'Unhandled Promise Rejection: {"abc":"circular"}', meta: 'circular object' }
+    { message: 'Unhandled Promise Rejection: {"abc":"circular"}', meta: 'circular object' },
+    { message: 'Unhandled Promise Rejection: test', meta: 'thrown error' },
+    { message: 'Unhandled Promise Rejection: test', meta: 'thrown error in nested fn' }
   ]
   expectedErrorMessages.forEach(expected => {
     it('should report unhandledPromiseRejections that are readable - ' + expected.meta, async () => {
@@ -189,6 +189,17 @@ describe('JSE Error detection in various callbacks', () => {
     })
   })
 
+  it('should not report unhandled rejections with falsy reasons', async () => {
+    const pageUrl = await browser.testHandle.assetURL('unhandled-promise-rejection-falsy-reason.html')
+    const [errorsExpect] = await Promise.all([
+      errorsCapture.waitForResult({ timeout: 10000 }),
+      browser.url(pageUrl)
+        .then(() => browser.waitForAgentLoad())
+    ])
+
+    expect(errorsExpect).toEqual([])
+  })
+
   it('should report errors from XHR callbacks', async () => {
     const pageUrl = await browser.testHandle.assetURL('xhr-error.html')
     const [[{ request: { body: errorBody, query: errorQuery } }]] = await Promise.all([
diff --git a/tests/unit/features/jserrors/shared/cast-error.test.js b/tests/unit/features/jserrors/shared/cast-error.test.js
@@ -133,15 +133,13 @@ describe('cast-error', () => {
       })
     })
 
-    test('handles events with an undefined reason', () => {
-      const castedError = castPromiseRejectionEvent({ reason: undefined })
-      expect(castedError).toMatchObject({
-        name: 'UncaughtError',
-        message: 'Unhandled Promise Rejection',
-        sourceURL: undefined,
-        line: undefined,
-        column: undefined
-      })
+    test('terminates for events with falsy reasons', () => {
+      let castedError = castPromiseRejectionEvent({ reason: undefined })
+      expect(castedError).toBeUndefined()
+      castedError = castPromiseRejectionEvent({ reason: null })
+      expect(castedError).toBeUndefined()
+      castedError = castPromiseRejectionEvent({ reason: '' })
+      expect(castedError).toBeUndefined()
     })
   })
 })
