chore(js): allow unhandledRejection plugin to be loaded more than once (

#1172)

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

@@ -5,38 +5,62 @@ import { Types } from '@honeybadger-io/core'
 export default class UnhandledRejectionMonitor {
   protected __isReporting: boolean
   protected __client: typeof Client
+  protected __listener: (reason: unknown, _promise: Promise<unknown>) => void
 
-  constructor(client: typeof Client) {
+  constructor() {
     this.__isReporting = false
+    this.__listener = this.makeListener()
+  }
+
+  setClient(client: typeof Client) {
     this.__client = client
   }
 
+  makeListener() {
+    const honeybadgerUnhandledRejectionListener = (reason: unknown, _promise: Promise<unknown>) => {
+      if (!this.__client || !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)
+          }
+        }
+      })
+    }
+    return honeybadgerUnhandledRejectionListener
+  }
+
+  maybeAddListener() {
+    const listeners = process.listeners('unhandledRejection')
+    if (!listeners.includes(this.__listener)) {
+      process.on('unhandledRejection', this.__listener)
+    }
+  }
+
+  maybeRemoveListener() {
+    const listeners = process.listeners('unhandledRejection')
+    if (listeners.includes(this.__listener)) {
+      process.removeListener('unhandledRejection', this.__listener)
+    }
+  }
+
   /**
    * 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)
-        }
-      }
-    })
+    const otherListeners = process.listeners('unhandledRejection')
+      .filter(listener => listener !== this.__listener)
+    return otherListeners.length > 0
   }
 }

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

@@ -2,16 +2,17 @@ import { Types } from '@honeybadger-io/core'
 import Client from '../../server'
 import UnhandledRejectionMonitor from './unhandled_rejection_monitor'
 
+const unhandledRejectionMonitor = new UnhandledRejectionMonitor()
+
 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)
-      })
+      unhandledRejectionMonitor.setClient(client)
+      if (client.config.enableUnhandledRejection) {
+        unhandledRejectionMonitor.maybeAddListener()
+      } else {
+        unhandledRejectionMonitor.maybeRemoveListener()
+      } 
     }
   }
 }

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

@@ -3,6 +3,9 @@ import * as util from '../../../../src/server/util'
 import Singleton from '../../../../src/server'
 import UnhandledRejectionMonitor from '../../../../src/server/integrations/unhandled_rejection_monitor'
 
+function getListenerCount() {
+  return process.listeners('unhandledRejection').length
+}
 
 describe('UnhandledRejectionMonitor', () => {
   // Using any rather than the real type so we can test and spy on private methods
@@ -18,7 +21,8 @@ describe('UnhandledRejectionMonitor', () => {
       { apiKey: 'testKey', afterUncaught: jest.fn(), logger: nullLogger() }, 
       new TestTransport()
     ) as unknown as typeof Singleton
-    unhandledRejectionMonitor = new UnhandledRejectionMonitor(client)
+    unhandledRejectionMonitor = new UnhandledRejectionMonitor()
+    unhandledRejectionMonitor.setClient(client)
     // Have to mock fatallyLogAndExit or we will crash the test
     fatallyLogAndExitSpy = jest
       .spyOn(util, 'fatallyLogAndExit')
@@ -35,16 +39,48 @@ describe('UnhandledRejectionMonitor', () => {
   describe('constructor', () => {
     it('set up variables and client', () => {
       expect(unhandledRejectionMonitor.__isReporting).toBe(false)
-      expect(unhandledRejectionMonitor.__client.config.apiKey).toBe('testKey')
+      expect(unhandledRejectionMonitor.__listener).toStrictEqual(expect.any(Function))
+      expect(unhandledRejectionMonitor.__listener.name).toBe('honeybadgerUnhandledRejectionListener')
     })
   })
 
-  describe('handleUnhandledRejection', () => {   
+  describe('maybeAddListener', () => {
+    it('adds our listener a maximum of one time', () => {
+      expect(getListenerCount()).toBe(0)
+      // Adds our listener
+      unhandledRejectionMonitor.maybeAddListener()
+      expect(getListenerCount()).toBe(1)
+      // Doesn't add a duplicate
+      unhandledRejectionMonitor.maybeAddListener()
+      expect(getListenerCount()).toBe(1)
+    })
+  })
+
+  describe('maybeRemoveListener', () => {
+    it('removes our listener if it is present', () => {
+      unhandledRejectionMonitor.maybeAddListener()
+      process.on('unhandledRejection', (err) => { console.log(err) })
+      expect(getListenerCount()).toBe(2)
+
+      unhandledRejectionMonitor.maybeRemoveListener()
+      expect(getListenerCount()).toBe(1)
+    })
+
+    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', () => {   
     const promise = new Promise(() => true)
     const reason = 'Promise rejection reason'
 
     it('calls notify and fatallyLogAndExit', (done) => {   
-      unhandledRejectionMonitor.handleUnhandledRejection(reason, promise)
+      unhandledRejectionMonitor.__listener(reason, promise)
       expect(notifySpy).toHaveBeenCalledTimes(1)
       expect(notifySpy).toHaveBeenCalledWith(
         reason, 
@@ -59,7 +95,7 @@ describe('UnhandledRejectionMonitor', () => {
 
     it('exits if enableUnhandledRejection is false and there are no other listeners', () => {
       client.configure({ enableUnhandledRejection: false })
-      unhandledRejectionMonitor.handleUnhandledRejection(reason, promise)
+      unhandledRejectionMonitor.__listener(reason, promise)
       expect(notifySpy).not.toHaveBeenCalled()
       expect(fatallyLogAndExitSpy).toHaveBeenCalledWith(reason)
     })
@@ -68,9 +104,24 @@ describe('UnhandledRejectionMonitor', () => {
       process.on('unhandledRejection', () => true)
       process.on('unhandledRejection', () => true)
       client.configure({ enableUnhandledRejection: false })
-      unhandledRejectionMonitor.handleUnhandledRejection(reason, promise)
+      unhandledRejectionMonitor.__listener(reason, promise)
       expect(notifySpy).not.toHaveBeenCalled()
       expect(fatallyLogAndExitSpy).not.toHaveBeenCalled()
     })
   })
+
+  describe('hasOtherUnhandledRejectionListeners', () => {
+    it('returns true if there are user-added listeners', () => {
+      unhandledRejectionMonitor.maybeAddListener()
+      process.on('unhandledRejection', function userAddedListener() {
+        return
+      })
+      expect(unhandledRejectionMonitor.hasOtherUnhandledRejectionListeners()).toBe(true)
+    })
+
+    it('returns false if there is only our expected listener', () => {
+      unhandledRejectionMonitor.maybeAddListener()
+      expect(unhandledRejectionMonitor.hasOtherUnhandledRejectionListeners()).toBe(false)
+    })
+  })
 })

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

@@ -3,6 +3,14 @@ import { TestTransport, TestClient, nullLogger } from '../../helpers'
 import * as util from '../../../../src/server/util'
 import Singleton from '../../../../src/server'
 
+function getListeners() {
+  return process.listeners('unhandledRejection')
+}
+
+function getListenerCount() {
+  return getListeners().length
+}
+
 describe('Uncaught Exception Plugin', () => {
   let client: typeof Singleton
   let notifySpy: jest.SpyInstance
@@ -34,9 +42,9 @@ describe('Uncaught Exception Plugin', () => {
   describe('load', () => {
     const load = plugin().load
 
-    it('attaches a listener for unhandledRejection', () => {
+    it('attaches a listener for unhandledRejection if enableUnhandledRejection is true', () => {
       load(client)
-      const listeners = process.listeners('unhandledRejection')
+      const listeners = getListeners()
       expect(listeners.length).toBe(1)
       expect(listeners[0].name).toBe('honeybadgerUnhandledRejectionListener') 
 
@@ -45,11 +53,23 @@ describe('Uncaught Exception Plugin', () => {
       expect(notifySpy).toHaveBeenCalledTimes(1)
     })
 
-    it('returns if enableUncaught is not true', () => {
+    it('does not add a listener if enableUnhandledRejection is false', () => {
+      client.configure({ enableUnhandledRejection: false })
+      load(client)
+      expect(getListenerCount()).toBe(0)
+    })
+
+    it('adds or removes listener if needed when reloaded', () => {
+      load(client)
+      expect(getListenerCount()).toBe(1)
+
       client.configure({ enableUnhandledRejection: false })
       load(client)
-      const listeners = process.listeners('unhandledRejection')
-      expect(listeners.length).toBe(0)
+      expect(getListenerCount()).toBe(0)
+
+      client.configure({ enableUnhandledRejection: true })
+      load(client)
+      expect(getListenerCount()).toBe(1)
     })
   })  
 })