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

Allow custom handling for global errors/rejections #843

Merged
merged 3 commits into from Jul 25, 2018

Conversation

@dominykas
Copy link
Contributor

dominykas commented Jul 24, 2018

Closes #782

dominykas added 3 commits Jul 24, 2018
…verride global error handlers

This allows testing code which is explicitly meant to e.g. crash the process or otherwise result in global errors.
@@ -47,4 +47,70 @@ describe('Test CLI', () => {
});
});
});

it('passes rejection to flags.onUnhandledRejection handler', (flags) => {

This comment has been minimized.

Copy link
@dominykas

dominykas Jul 24, 2018

Author Contributor

I figured it's best to keep all these tests together so that // covered by test/cli_error/failure.js comment for disabled coverage remains true?

return;
}
catch (errInErrorhandling) {
err = errInErrorhandling;

This comment has been minimized.

Copy link
@dominykas

dominykas Jul 24, 2018

Author Contributor

I figure if the user has onUncaughtException declared, then they know what they expect as an err, so it's best to fail the test with whatever happens inside of it, rather than with the original error.

This comment has been minimized.

Copy link
@geek

geek Jul 25, 2018

Member

This sounds like a good plan to me

@@ -574,13 +574,28 @@ internals.protect = function (item, state) {
/* $lab:coverage:off$ */
const promiseRejectionHandler = function (err) {

if (flags.onUnhandledRejection) {
flags.onUnhandledRejection(err);

This comment has been minimized.

Copy link
@dominykas

dominykas Jul 24, 2018

Author Contributor

If this throws - it's handled by onUncaughtException - I think try/catch is not necessary here?

@geek
geek approved these changes Jul 24, 2018
@geek geek added the feature label Jul 24, 2018
@geek geek self-assigned this Jul 24, 2018
@geek geek added this to the 16.0.0 milestone Jul 25, 2018
@geek geek merged commit 4d3e504 into hapijs:master Jul 25, 2018
2 checks passed
2 checks passed
Node Security No known vulnerabilities found
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dominykas dominykas deleted the dominykas:allow-custom-error-handling branch Jul 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.