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

Conversation

BethanyBerkowitz
Copy link
Contributor

@BethanyBerkowitz BethanyBerkowitz commented Aug 28, 2023

Status

READY

Description

Part of #1163

This PR makes it safe to load the unhandledRejection plugin more than once.

If loaded with enableUnhandledRejection true, it will add a listener if one has not already been added
If loaded with enableUnhandledRejection false, it will remove the listener if it exists
However, the client itself still only loads the plugin once, so this PR should not cause any user-facing change in behavior.

Related PRs

#1170 does the same with unhandledException

Todos

  • Tests
  • Documentation

Steps to Test or Reproduce

Unit tests mostly cover it, however I also tested manually using the sails example app. Any node-based example app would be fine, you just need to trigger a promise rejection where the middleware will not pick it up. eg

Honeybadger.configure({
  apiKey: process.env.HONEYBADGER_API_KEY, 
  enableUncaught: false,
  enableUnhandledRejection: true,
})


function reject() {
  Promise.reject(new Error('error within a promise'))
}

setTimeout(reject, 500)

I tested with enableUnhandledRejection: true and false.

Copy link
Member

@joshuap joshuap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. If it's not urgent, it would be great if @subzero10 could also review when he's back from vacation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants