Cannot bind uncaughtException & unhandledRejection together

[vekexasia]

Hello,

it looks like you can bind to both. Is there any particoular reason?

Also, would you expose the exit function so that one can eventually calll it manually?

[Tapppi]

Sorry for the late response!

it looks like you can bind to both. Is there any particoular reason?

Yes, one might want to bind different handlers. Should make a generic exit handler though.. Any suggestions for names etc.?

Also, would you expose the exit function so that one can eventually calll it manually?

Interesting idea, I'll have to give this further thought, but possibly.

[rijnhard]

on the exit function, that would be a good idea, as then there's a safe way to exit the process. Possibly even override process.exit to be safe (but that should be optional)

[Tapppi]

on the exit function, that would be a good idea, as then there's a safe way to exit the process. Possibly even override process.exit to be safe (but that should be optional)

Currently figuring out all the implications of this. I'll try to carve out some more OSS time next weekend and get this in. Small change but node's process lifecycle is quite unfriendly to customisation, in part due to OS differences.

[rijnhard]

@Tapppi working with AsyncExitHook more essentially what I think it needs to do is simply on asyncExitHook.exit(0);

• fire exit handlers
• wait until all are complete (maybe have a timeout here.)
• then call process.exit(code);

[rijnhard]

@Tapppi any movement here?

[rijnhard]

Just saying, I ran across this again today

[rijnhard]

Workaround for now. dont use either function and do this instead.

import asyncExitHook from 'async-exit-hook';

let stopped = false;

/**
 * closes thingy before exiting
 * @param {function} cb callback for async exit hook
 */
function shutdown(cb = () => {}) {
    if (!stopped) {
        stopped = true;
        Logger.info('thingy is shutting down...');

        thingy1.stop((success) => {
            thingy2.shutdown().finally(() => cb());
        });
    }
}

asyncExitHook((cb) => {
    Logger.info('thingy is exiting');
    shutdown(cb);
});

asyncExitHook.hookEvent('uncaughtException', 1, (err) => {
    Logger.fatal('thingy is shutting down due to UncaughtException...');
    return false;
});

asyncExitHook.hookEvent('unhandledRejection', 2, (err) => {
    Logger.fatal('thingy is shutting down due to unhandledRejectionHandler...');
    return false;
});

[meotimdihia]

@Tapppi: this problem needs to write in the documentation. Or need to fix it.

[geogeim]

please fix this, i lost a few hours debugging

[larvanitis]

There are two problems with the current implementation:

1. Only the first hooked event is registered.

Both registration functions check for the single errHooks array's length.

https://github.com/Tapppi/async-exit-hook/blob/80e692c88e62a88cf5750460ff02c28298e5b09b/index.js#L151 https://github.com/Tapppi/async-exit-hook/blob/80e692c88e62a88cf5750460ff02c28298e5b09b/index.js#L160

2. All error hook handlers will be called, even if they weren't registered on that event (from 1).

Both errors trigger all errHooks.

https://github.com/Tapppi/async-exit-hook/blob/80e692c88e62a88cf5750460ff02c28298e5b09b/index.js#L69

Solution

Both can be fixed by using separate arrays - eg uncaughtExceptionHooks and unhandledRejectionHooks and checking for the actual error at https://github.com/Tapppi/async-exit-hook/blob/80e692c88e62a88cf5750460ff02c28298e5b09b/index.js#L67