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

suggestion: on.error handler improvements #6

Open
binarykitchen opened this issue Feb 19, 2013 · 0 comments
Open

suggestion: on.error handler improvements #6

binarykitchen opened this issue Feb 19, 2013 · 0 comments

Comments

@binarykitchen
Copy link

hello there

i think the recommended function dispose() is not called on an error. let me explain, you have this line in your code:

reqDomain.on('error', next);

this is fine, you're delegating error handling to the middleware. but there is no guarantee that the middleware is going to render the error properly nor it's going dispose of the error there. an error might be thrown there again for any reasons.

i recommend to improve this code line further to make it more stable:

reqDomain.on('error', function(err) {
    try {
        next(err);
    } catch(exc) {
        console.error('Sorry, your middleware could not handle the error %s', err);
        console.error('Reason: %s', exc);
        // tried our best. clean up anything remaining.
        reqDomain.dispose();
    }
});

this is also recommend for another reason: to avoid memory leaks within nested domains.

my code suggestion is part of the official documentation:
http://nodejs.org/docs/latest/api/domain.html

cheers
michael

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

No branches or pull requests

1 participant