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
Show file tree
Hide file tree
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
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
Expand Up @@ -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
Expand Up @@ -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
Expand Up @@ -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
Expand All @@ -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')
Expand All @@ -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,
Expand All @@ -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)
})
Expand All @@ -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
Expand Up @@ -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
Expand Down Expand Up @@ -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')

Expand All @@ -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)
})
})
})