fix(js): move unhandled rejection monitor to a class, fix filter for domainUncaughtExceptionClear

packages/js/src/server.ts

@@ -3,7 +3,7 @@ import domain from 'domain'
 import { Client, Util, Types } from '@honeybadger-io/core'
 import { getSourceFile } from './server/util'
 import uncaughtException from './server/integrations/uncaught_exception_plugin'
-import unhandledRejection from './server/integrations/unhandled_rejection'
+import unhandledRejection from './server/integrations/unhandled_rejection_plugin'
 import { errorHandler, requestHandler } from './server/middleware'
 import { lambdaHandler } from './server/aws_lambda'
 import { ServerTransport } from './server/transport'

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

@@ -31,7 +31,7 @@ export default class UncaughtExceptionMonitor {
     // Node sets up these listeners when we use domains
     // Since they're not set up by a user, they shouldn't affect whether we exit or not
     const domainListeners = allListeners.filter(listener => {
-      listener.name === 'domainUncaughtExceptionClear' 
+      return listener.name === 'domainUncaughtExceptionClear' 
     })
     return allListeners.length - domainListeners.length > 1
   }

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

@@ -0,0 +1,42 @@
+import Client from '../../server'
+import { fatallyLogAndExit } from '../util'
+import { Types } from '@honeybadger-io/core'
+
+export default class UnhandledRejectionMonitor {
+  protected __isReporting: boolean
+  protected __client: typeof Client
+
+  constructor(client: typeof Client) {
+    this.__isReporting = false
+    this.__client = client
+  }
+
+  /**
+   * If there are no other unhandledRejection listeners,
+   * we want to report the exception to Honeybadger and
+   * mimic the default behavior of NodeJs,
+   * which is to exit the process with code 1
+   */
+  hasOtherUnhandledRejectionListeners() {
+    return process.listeners('unhandledRejection').length > 1
+  }
+
+  handleUnhandledRejection(reason: unknown, _promise: Promise<unknown>) {
+    if (!this.__client.config.enableUnhandledRejection) {
+      if (!this.hasOtherUnhandledRejectionListeners() && !this.__isReporting) {
+        fatallyLogAndExit(reason as Error)
+      }
+      return
+    }
+
+    this.__isReporting = true;
+    this.__client.notify(reason as Types.Noticeable, { component: 'unhandledRejection' }, {
+      afterNotify: () => {
+        this.__isReporting = false;
+        if (!this.hasOtherUnhandledRejectionListeners()) {
+          fatallyLogAndExit(reason as Error)
+        }
+      }
+    })
+  }
+}

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

@@ -0,0 +1,17 @@
+import { Types } from '@honeybadger-io/core'
+import Client from '../../server'
+import UnhandledRejectionMonitor from './unhandled_rejection_monitor'
+
+export default function (): Types.Plugin {
+  return {
+    load: (client: typeof Client) => {
+      if (!client.config.enableUnhandledRejection) {
+        return
+      }
+      const unhandledRejectionMonitor = new UnhandledRejectionMonitor(client)
+      process.on('unhandledRejection', function honeybadgerUnhandledRejectionListener(reason, _promise) {
+        unhandledRejectionMonitor.handleUnhandledRejection(reason, _promise)
+      })
+    }
+  }
+}

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

@@ -55,7 +55,7 @@ describe('UncaughtExceptionMonitor', () => {
     const error = new Error('dang, broken again')  
 
     it('calls notify, afterUncaught, and fatallyLogAndExit', (done) => {
-      uncaughtExceptionMonitor.handleUncaughtException(error, client)
+      uncaughtExceptionMonitor.handleUncaughtException(error)
       expect(notifySpy).toHaveBeenCalledTimes(1)
       expect(notifySpy).toHaveBeenCalledWith(
         error, 
@@ -70,31 +70,56 @@ describe('UncaughtExceptionMonitor', () => {
 
     it('returns if it is already reporting', () => {
       uncaughtExceptionMonitor.__isReporting = true
-      uncaughtExceptionMonitor.handleUncaughtException(error, client)
+      uncaughtExceptionMonitor.handleUncaughtException(error)
       expect(notifySpy).not.toHaveBeenCalled()
       expect(fatallyLogAndExitSpy).not.toHaveBeenCalled()
     })
 
     it('returns if it was already called and there are other listeners', () => {
       process.on('uncaughtException', () => true)
       process.on('uncaughtException', () => true)
-      uncaughtExceptionMonitor.handleUncaughtException(error, client)
+      uncaughtExceptionMonitor.handleUncaughtException(error)
       expect(notifySpy).toHaveBeenCalledTimes(1)
 
       client.afterNotify(() => {
         expect(fatallyLogAndExitSpy).not.toHaveBeenCalled()
         expect(uncaughtExceptionMonitor.__handlerAlreadyCalled).toBe(true)
         // Doesn't notify a second time
-        uncaughtExceptionMonitor.handleUncaughtException(error, client)
+        uncaughtExceptionMonitor.handleUncaughtException(error)
         expect(notifySpy).toHaveBeenCalledTimes(1)
       })
     })
 
     it('exits if it was already called and there are no other listeners', () => {
       uncaughtExceptionMonitor.__handlerAlreadyCalled = true
-      uncaughtExceptionMonitor.handleUncaughtException(error, client)
+      uncaughtExceptionMonitor.handleUncaughtException(error)
       expect(notifySpy).not.toHaveBeenCalled()
       expect(fatallyLogAndExitSpy).toHaveBeenCalledWith(error)
     })
   })
+
+  describe('hasOtherUncaughtExceptionListeners', () => {
+    it('returns true if there are user-added listeners', () => {
+      process.on('uncaughtException', function honeybadgerUncaughtExceptionListener() {
+        return
+      })
+      process.on('uncaughtException', function domainUncaughtExceptionClear() {
+        return 
+      })
+      process.on('uncaughtException', function userAddedListener() {
+        return
+      })
+      expect(uncaughtExceptionMonitor.hasOtherUncaughtExceptionListeners()).toBe(true)
+    })
+
+    it('returns false if there are only our expected listeners', () => {
+      process.on('uncaughtException', function honeybadgerUncaughtExceptionListener() {
+        return
+      })
+      process.on('uncaughtException', function domainUncaughtExceptionClear() {
+        return 
+      })
+      expect(uncaughtExceptionMonitor.hasOtherUncaughtExceptionListeners()).toBe(false)
+    })
+  })
 })

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

@@ -0,0 +1,76 @@
+import { TestTransport, TestClient, nullLogger } from '../../helpers'
+import * as util from '../../../../src/server/util'
+import Singleton from '../../../../src/server'
+import UnhandledRejectionMonitor from '../../../../src/server/integrations/unhandled_rejection_monitor'
+
+
+describe('UnhandledRejectionMonitor', () => {
+  // Using any rather than the real type so we can test and spy on private methods
+  // eslint-disable-next-line  @typescript-eslint/no-explicit-any
+  let unhandledRejectionMonitor: any
+  let client: typeof Singleton
+  let fatallyLogAndExitSpy: jest.SpyInstance
+  let notifySpy: jest.SpyInstance
+
+  beforeEach(() => {
+    // We just need a really basic client, so ignoring type issues here
+    client = new TestClient(
+      { apiKey: 'testKey', afterUncaught: jest.fn(), logger: nullLogger() }, 
+      new TestTransport()
+    ) as unknown as typeof Singleton
+    unhandledRejectionMonitor = new UnhandledRejectionMonitor(client)
+    // Have to mock fatallyLogAndExit or we will crash the test
+    fatallyLogAndExitSpy = jest
+      .spyOn(util, 'fatallyLogAndExit')
+      .mockImplementation(() => true as never)
+    notifySpy = jest.spyOn(client, 'notify')
+  })
+
+  afterEach(() => {
+    jest.clearAllMocks()
+    process.removeAllListeners('unhandledRejection')
+    unhandledRejectionMonitor.__isReporting = false
+  })
+
+  describe('constructor', () => {
+    it('set up variables and client', () => {
+      expect(unhandledRejectionMonitor.__isReporting).toBe(false)
+      expect(unhandledRejectionMonitor.__client.config.apiKey).toBe('testKey')
+    })
+  })
+
+  describe('handleUnhandledRejection', () => {   
+    const promise = new Promise(() => true)
+    const reason = 'Promise rejection reason'
+
+    it('calls notify and fatallyLogAndExit', (done) => {   
+      unhandledRejectionMonitor.handleUnhandledRejection(reason, promise)
+      expect(notifySpy).toHaveBeenCalledTimes(1)
+      expect(notifySpy).toHaveBeenCalledWith(
+        reason, 
+        { component: 'unhandledRejection' },
+        { afterNotify: expect.any(Function) }
+      )
+      client.afterNotify(() => {
+        expect(fatallyLogAndExitSpy).toHaveBeenCalledWith(reason)
+        done()
+      }) 
+    })
+
+    it('exits if enableUnhandledRejection is false and there are no other listeners', () => {
+      client.configure({ enableUnhandledRejection: false })
+      unhandledRejectionMonitor.handleUnhandledRejection(reason, promise)
+      expect(notifySpy).not.toHaveBeenCalled()
+      expect(fatallyLogAndExitSpy).toHaveBeenCalledWith(reason)
+    })
+
+    it('returns if enableUnhandledRejection is false and there are other listeners', () => {
+      process.on('unhandledRejection', () => true)
+      process.on('unhandledRejection', () => true)
+      client.configure({ enableUnhandledRejection: false })
+      unhandledRejectionMonitor.handleUnhandledRejection(reason, promise)
+      expect(notifySpy).not.toHaveBeenCalled()
+      expect(fatallyLogAndExitSpy).not.toHaveBeenCalled()
+    })
+  })
+})

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

@@ -0,0 +1,55 @@
+import plugin from '../../../../src/server/integrations/unhandled_rejection_plugin'
+import { TestTransport, TestClient, nullLogger } from '../../helpers'
+import * as util from '../../../../src/server/util'
+import Singleton from '../../../../src/server'
+
+describe('Uncaught Exception Plugin', () => {
+  let client: typeof Singleton
+  let notifySpy: jest.SpyInstance
+
+  beforeEach(() => {
+    // We just need a really basic client, so ignoring type issues here
+    client = new TestClient(
+      { apiKey: 'testKey', afterUncaught: jest.fn(), logger: nullLogger() }, 
+      new TestTransport()
+    ) as unknown as typeof Singleton
+    // Have to mock fatallyLogAndExit or we will crash the test
+    jest
+      .spyOn(util, 'fatallyLogAndExit')
+      .mockImplementation(() => true as never)
+    notifySpy = jest.spyOn(client, 'notify')
+  })
+
+  afterEach(() => {
+    jest.clearAllMocks()
+    process.removeAllListeners('unhandledRejection')
+  })
+
+  it('is a function which returns an object with a load function', () => {
+    expect(plugin()).toStrictEqual({
+      load: expect.any(Function)
+    })
+  })
+
+  describe('load', () => {
+    const load = plugin().load
+
+    it('attaches a listener for unhandledRejection', () => {
+      load(client)
+      const listeners = process.listeners('unhandledRejection')
+      expect(listeners.length).toBe(1)
+      expect(listeners[0].name).toBe('honeybadgerUnhandledRejectionListener') 
+
+      const promise = new Promise(() => true)
+      process.emit('unhandledRejection', 'Stuff went wrong', promise)
+      expect(notifySpy).toHaveBeenCalledTimes(1)
+    })
+
+    it('returns if enableUncaught is not true', () => {
+      client.configure({ enableUnhandledRejection: false })
+      load(client)
+      const listeners = process.listeners('unhandledRejection')
+      expect(listeners.length).toBe(0)
+    })
+  })  
+})