fix(js): improved, unambiguous error message (#1428)

Author: subzero10

.github/workflows/node.js.yml

@@ -99,7 +99,7 @@ jobs:
         npm run test:integration
 
     - name: Upload Playwright Report
-      uses: actions/upload-artifact@v3
+      uses: actions/upload-artifact@v4
       if: failure()
       with:
         name: playwright-report

packages/js/src/server/integrations/uncaught_exception_monitor.ts

@@ -10,7 +10,7 @@ export default class UncaughtExceptionMonitor {
 
   constructor() {
     this.__isReporting = false
-    this.__handlerAlreadyCalled = false 
+    this.__handlerAlreadyCalled = false
     this.__listener = this.makeListener()
     this.removeAwsLambdaListener()
   }
@@ -22,23 +22,23 @@ export default class UncaughtExceptionMonitor {
   makeListener() {
     const honeybadgerUncaughtExceptionListener = (uncaughtError: Error) => {
       if (this.__isReporting || !this.__client) { return }
-    
+
       // report only the first error - prevent reporting recursive errors
       if (this.__handlerAlreadyCalled) {
         if (!this.hasOtherUncaughtExceptionListeners()) {
-          fatallyLogAndExit(uncaughtError)
+          fatallyLogAndExit(uncaughtError, 'uncaught exception')
         }
         return
       }
-    
+
       this.__isReporting = true
       this.__client.notify(uncaughtError, {
         afterNotify: (_err, _notice) => {
           this.__isReporting = false
           this.__handlerAlreadyCalled = true
           this.__client.config.afterUncaught(uncaughtError)
           if (!this.hasOtherUncaughtExceptionListeners()) {
-            fatallyLogAndExit(uncaughtError)
+            fatallyLogAndExit(uncaughtError, 'uncaught exception')
           }
         }
       })
@@ -71,16 +71,16 @@ export default class UncaughtExceptionMonitor {
    * we want to report the exception to Honeybadger and
    * mimic the default behavior of NodeJs,
    * which is to exit the process with code 1
-   * 
+   *
    * Node sets up domainUncaughtExceptionClear when we use domains.
    * Since they're not set up by a user, they shouldn't affect whether we exit or not
    */
   hasOtherUncaughtExceptionListeners(): boolean {
     const allListeners = process.listeners('uncaughtException')
     const otherListeners = allListeners.filter(listener => (
-      listener.name !== 'domainUncaughtExceptionClear' 
+      listener.name !== 'domainUncaughtExceptionClear'
       && listener !== this.__listener
     ))
     return otherListeners.length > 0
   }
-}
+}

packages/js/src/server/integrations/unhandled_rejection_monitor.ts

@@ -23,7 +23,7 @@ export default class UnhandledRejectionMonitor {
         afterNotify: () => {
           this.__isReporting = false;
           if (!this.hasOtherUnhandledRejectionListeners()) {
-            fatallyLogAndExit(reason as Error)
+            fatallyLogAndExit(reason as Error, 'unhandled rejection')
           }
         }
       })

packages/js/src/server/util.ts

@@ -6,8 +6,8 @@ import { promisify } from 'util'
 
 const readFile = promisify(fs.readFile)
 
-export function fatallyLogAndExit(err: Error): never {
-  console.error('[Honeybadger] Exiting process due to uncaught exception')
+export function fatallyLogAndExit(err: Error, source: string): never {
+  console.error(`[Honeybadger] Exiting process due to ${source}`)
   console.error(err.stack || err)
   process.exit(1)
 }

packages/js/test/unit/server/integrations/uncaught_exception_monitor.server.test.ts

@@ -19,7 +19,7 @@ describe('UncaughtExceptionMonitor', () => {
   beforeEach(() => {
     // We just need a really basic client, so ignoring type issues here
     client = new TestClient(
-      { apiKey: 'testKey', afterUncaught: jest.fn(), logger: nullLogger() }, 
+      { apiKey: 'testKey', afterUncaught: jest.fn(), logger: nullLogger() },
       new TestTransport()
     ) as unknown as typeof Singleton
     uncaughtExceptionMonitor = new UncaughtExceptionMonitor()
@@ -83,27 +83,27 @@ describe('UncaughtExceptionMonitor', () => {
     it('does nothing if our listener is not present', () => {
       process.on('uncaughtException', (err) => { console.log(err) })
       expect(getListenerCount()).toBe(1)
-      
+
       uncaughtExceptionMonitor.maybeRemoveListener()
       expect(getListenerCount()).toBe(1)
     })
   })
 
-  describe('__listener', () => {   
-    const error = new Error('dang, broken again')  
+  describe('__listener', () => {
+    const error = new Error('dang, broken again')
 
     it('calls notify, afterUncaught, and fatallyLogAndExit', (done) => {
       uncaughtExceptionMonitor.__listener(error)
       expect(notifySpy).toHaveBeenCalledTimes(1)
       expect(notifySpy).toHaveBeenCalledWith(
-        error, 
+        error,
         { afterNotify: expect.any(Function) }
       )
       client.afterNotify(() => {
         expect(client.config.afterUncaught).toHaveBeenCalledWith(error)
-        expect(fatallyLogAndExitSpy).toHaveBeenCalledWith(error)
+        expect(fatallyLogAndExitSpy).toHaveBeenCalledWith(error, 'uncaught exception')
         done()
-      }) 
+      })
     })
 
     it('returns if it is already reporting', () => {
@@ -132,15 +132,15 @@ describe('UncaughtExceptionMonitor', () => {
       uncaughtExceptionMonitor.__handlerAlreadyCalled = true
       uncaughtExceptionMonitor.__listener(error)
       expect(notifySpy).not.toHaveBeenCalled()
-      expect(fatallyLogAndExitSpy).toHaveBeenCalledWith(error)
+      expect(fatallyLogAndExitSpy).toHaveBeenCalledWith(error, 'uncaught exception')
     })
   })
 
   describe('hasOtherUncaughtExceptionListeners', () => {
     it('returns true if there are user-added listeners', () => {
       uncaughtExceptionMonitor.maybeAddListener()
       process.on('uncaughtException', function domainUncaughtExceptionClear() {
-        return 
+        return
       })
       process.on('uncaughtException', function userAddedListener() {
         return
@@ -151,7 +151,7 @@ describe('UncaughtExceptionMonitor', () => {
     it('returns false if there are only our expected listeners', () => {
       uncaughtExceptionMonitor.maybeAddListener()
       process.on('uncaughtException', function domainUncaughtExceptionClear() {
-        return 
+        return
       })
       expect(uncaughtExceptionMonitor.hasOtherUncaughtExceptionListeners()).toBe(false)
     })

packages/js/test/unit/server/integrations/unhandled_rejection_monitor.server.test.ts

@@ -18,7 +18,7 @@ describe('UnhandledRejectionMonitor', () => {
   beforeEach(() => {
     // We just need a really basic client, so ignoring type issues here
     client = new TestClient(
-      { apiKey: 'testKey', afterUncaught: jest.fn(), logger: nullLogger() }, 
+      { apiKey: 'testKey', afterUncaught: jest.fn(), logger: nullLogger() },
       new TestTransport()
     ) as unknown as typeof Singleton
     unhandledRejectionMonitor = new UnhandledRejectionMonitor()
@@ -69,28 +69,28 @@ describe('UnhandledRejectionMonitor', () => {
     it('does nothing if our listener is not present', () => {
       process.on('unhandledRejection', (err) => { console.log(err) })
       expect(getListenerCount()).toBe(1)
-      
+
       unhandledRejectionMonitor.maybeRemoveListener()
       expect(getListenerCount()).toBe(1)
     })
   })
 
-  describe('__listener', () => {   
+  describe('__listener', () => {
     const promise = new Promise(() => true)
     const reason = 'Promise rejection reason'
 
-    it('calls notify and fatallyLogAndExit', (done) => {   
+    it('calls notify and fatallyLogAndExit', (done) => {
       unhandledRejectionMonitor.__listener(reason, promise)
       expect(notifySpy).toHaveBeenCalledTimes(1)
       expect(notifySpy).toHaveBeenCalledWith(
-        reason, 
+        reason,
         { component: 'unhandledRejection' },
         { afterNotify: expect.any(Function) }
       )
       client.afterNotify(() => {
-        expect(fatallyLogAndExitSpy).toHaveBeenCalledWith(reason)
+        expect(fatallyLogAndExitSpy).toHaveBeenCalledWith(reason, 'unhandled rejection')
         done()
-      }) 
+      })
     })
   })