Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 6, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Allow reloading unhandledRejection plugin
BethanyBerkowitz committed Aug 28, 2023

Verified

This commit was signed with the committer’s verified signature.
erikmd Erik Martin-Dorel
commit 194ac3ef821ed41688105fa02d80e8fc5f7cebfc
66 changes: 45 additions & 21 deletions packages/js/src/server/integrations/unhandled_rejection_monitor.ts
Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
@@ -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()
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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)
})
})
})
Original file line number Diff line number Diff line change
@@ -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)
})
})
})