fix(js): move unhandled rejection monitor to a class, fix filter for domainUncaughtExceptionClear #1166
Conversation
BethanyBerkowitz
changed the title
| }) | ||
| }) | ||
|
|
||
| // Seems like it was meant to work this way but was never implemented? |
There was a problem hiding this comment.
This is easy to add to make it match how uncaughtException works (eg return right away if __isReporting is true... but there may be a reason it doesn't do that... leaning toward minimal change with this PR.
There was a problem hiding this comment.
🤔 I don't remember. So, now it was changed to return is __isReporting is true?
There was a problem hiding this comment.
I decided not to change how this was working in this PR.
__isReporting is only used when enableUnhandledRejection is false... this seems like it was probably an oversight but it may have been for a good reason... it's difficult to know when there were no tests or comments written.
Happy to re-examine, but I was hoping to keep this PR to just a change in the format of the unhandled rejection code and adding the tests rather than changing the logic.
There was a problem hiding this comment.
Here's an issue to review this! #1167
BethanyBerkowitz
changed the title
Status
READY
Description
Closes #1165
Makes the format of the unhandled rejection listener match that of the uncaught exception listener and adds unit tests.
I also noticed that the filter for domainUncaughtExceptionClear needed unit tests and was missing the
returnkeyword.Todos
Steps to Test or Reproduce
Outline the steps to test or reproduce the PR here.