Adds asynchronous support

[jakobo]

This is an attempt to add asynchronous support in a backward-compatible manner.

Fixes #1

API Changes

All changes are additive as new features. The original exitHook continues to work with its original API. This code change does check that you passed a function into exitHook, though this was already the assumed behavior.

new asyncExitHook exported from the package with the following signature:

function asyncExitHook(onExit: () => (void | Promise<void>), options: Options): () => void;

new gracefulExit exported from the package with the following signature:

function gracefulExit(signal?: number): void;

type Options

interface Options {
	/**
	The amount of time in ms that the `onExit` function is expected to take.
	*/
	minimumWait: number;
}

When the exit handler is run, all synchronous handlers are executed first, regardless of the order added. Attempting to exit synchronously with asynchronous exit handlers registered will create a console.error explaining that asynchronous shutdown should use gracefulExit instead of process.exit.

Internals

• multiple queues We now maintain an internal set of callbacks and asyncCallbacks
• refactor The bulk of the original exitHook was moved to an internal addHook function, allowing both async and non async versions to use the same code path
• additional events We listen to beforeExit in addition to SIGINT and SIGTERM and PM2 message, giving us an additional chance to attempt cleanup
• maximum wait During execution of any registered async options, the longest minimumWait value will be used. After this time elapses, the shutdown operation will continue

Original Post

API Changes

exitHook The default signature has an additional optional parameter maxWait, which is required if your hook is asynchronous.

exitHook(onExit); // still supported

exitHook(async () => {
  // async stuff
}, 100);

In this example, the exitHook handler will wait a maximum of 100ms for the registered hook to complete before forcibly terminating the process. A duration is required for all asynchronous hooks to ensure that the node process cannot be extended indefinitely.

Internals

• multiple queues A queue for synchronous and asynchronous callbacks exists. This allows us to quickly determine if the user is in a synchronous exit (via process.exit()) and if they have asynchronous callbacks registered. If both conditions are true, we notify the user asynchronous callbacks will not be run and offer a remediation.
• synchronous first To reduce complexity, all synchronous callbacks are ran first
• beforeExit We listen for beforeExit events which can be asynchronous

[sindresorhus]

which is required if your hook is asynchronous.

This is not mentioned in the docs.

[sindresorhus]

exitHook.exit() is missing from the readme.

[sindresorhus]

The downsides with async must be more clearly documented.

[sindresorhus]

I think we should provide a reasonable default timeout.

[jakobo]

Doc Changes

required param in docs, exit() missing, async drawbacks must be more clear

Given this feedback, I think it makes more sense to keep the exitHook API the way it is and make the async option an explicit opt-in via exitHook.async({onExit: function, maxWait: number}). It also means the docs can be changed to have a dedicated async section that clearly explains the drawbacks of using the asynchronous exit hooks. In particular the non-top level use case is really important to capture.

Code Changes

• type docs
• async api will be fixed with the change to exitHook.async and will use an options object
• lint rules will be followed. This //ignore was mostly because I attempted to preserve the exitHook api instead of exitHook.async(options). This will be resolved with the alternate API
• default timeout. I feel 1000ms is reasonable. It's long enough for most simple "close this connection" tasks even if there's a need to flush a log or write a file.

---

I'll pull all these changes together and incrementally update the PR. Will put it into draft until all the changes are landed. Thank you for the detailed feedback.

[sindresorhus]

I think it should be a named export:

import exitHook, {asyncExitHook} from 'exit-hook';

[jakobo]

All great feedback, I'll return to draft and reopen once all the feedback is addressed.

Planned:

• Change async hook to a named export asyncExitHook instead of attached to exitHook
• asyncExitHook change parameter order from hook, options to options, hook to make clear that you must set options for your hook given the caveats (pending discussion on parameter order)
• change minWait to minimumWait
• Move exitHook.exit to a named export gracefulExit
• Update docs to reflect API changes
• Add inline example for asyncExitHook to readme

[jakobo]

I've made the suggested changes, save for the parameter ordering on asyncHook. I could be convinced otherwise, but I've seen both options, callback and callback, options used in the wild.

I feel the API aligns closely with p-retry, where first argument to p-retry is the callback returning something Promise-like, followed by the retry options. This is an API very similar to what we're introducing with asyncHook.

I've also made sure that our minimumWait is now a required option.

[sindresorhus]

I also noticed we don't do any typeof checks for the onExit, do you feel they're necessary to add?

Yeah, it would be useful to have such a check.

[jakobo]

Alright, type tests added as part of this change for both exitHook and asyncExitHook.

[sindresorhus]

Thanks :)