node-cleanup icon indicating copy to clipboard operation
node-cleanup copied to clipboard

async/await?

Open noyearzero opened this issue 5 years ago • 12 comments

Is it possible to use an await inside a cleanup callback?

noyearzero avatar May 30 '19 18:05 noyearzero

The way I got this to work was have a regular function on nodeCleanup, and have it call an async function (which eventually runs process.kill(process.pid, signal); after awaiting). Basically do something like this:

nodeCleanup((exitCode, signal) => {
        shutDownSequence(exitCode, signal); // This is your async function.
        nodeCleanup.uninstall(); // Unregister the nodeCleanup handler.
        return false;
    });

Seshpenguin avatar Jul 05 '19 17:07 Seshpenguin

The way that is typed, shutDownSequence would not be called asynchronously. You would have to call it with await (and also have defined the function with 'async')

nodeCleanup((exitCode, signal) => { await shutDownSequence(exitCode, signal); // This is your async function. nodeCleanup.uninstall(); // Unregister the nodeCleanup handler. return false; });

noyearzero avatar Jul 15 '19 13:07 noyearzero

@noyearzero It doesn't have to be called with await. Once it's called (We just ignore the promise returned by shutDownSequence), the function attached to nodeCleanup continues executing: disabling nodeCleanup (by running uninstall and returning false) so it won't prematurely kill the process.

At the same time the async function shutDownSequence runs and eventually will call process.kill.

async function shutDownSequence(exitCode, signal) {
    await someOtherThing();
    await moreThings();
    process.kill(process.pid, signal); // this line kills the process.
} 

Seshpenguin avatar Jul 15 '19 13:07 Seshpenguin

So what are you saying? await is just an optional thing javascript programmers type out but don't have to because they like typing things that aren't necessary?

noyearzero avatar Jul 15 '19 15:07 noyearzero

@noyearzero ?

node-cleanup doesn't support promises, so you can't directly pass an async function. I doesn't know to wait for the promise to resolve. In an ideal world we can do this:

// this won't work
nodeCleanup( async (exitCode, signal) => {
    await someOtherThing();
    await moreThings();
});

But that won't work, since it would require updating node-cleanup to handle the promise returned by the async function.

The workaround I proposed is based on the existing documentation, but updated for async/await. Recall that the example with callbacks from the readme is:

nodeCleanup(function (exitCode, signal) {
    if (signal) {
        unsavedData.save(function done() {
            // calling process.exit() won't inform parent process of signal
            process.kill(process.pid, signal);
        });
        nodeCleanup.uninstall(); // don't call cleanup handler again
        return false;
    }
});

Because node-cleanup doesn't know how to wait for asynchronous tasks to complete, we tell it not to do the final step of killing the process (by calling nodeCleanup.uninstall(); and return false). Then, when the async code finishes (in this example, calling the callback), we run the final step ourselves (process.kill(process.pid, signal);).

Now, how do we take this example and use async/await? What we have to do is create a separate async function to do our cleanup first. Let's take this function as an example:

async function shutDownSequence(exitCode, signal) {
    await someOtherThing();
    await moreThings();
    process.kill(process.pid, signal); // finally, this line kills the process.
} 

This is the code where we run our cleanup before killing the process. We can't pass this directly to node-cleanup, because as we figured out before, it's not designed to handle promises (which is what an async/await function returns). That's okay though, since we can just call this asynchronous function from the regular node-cleanup function:

nodeCleanup((exitCode, signal) => {
        shutDownSequence(exitCode, signal); // This is your async function.
        nodeCleanup.uninstall(); // Unregister the nodeCleanup handler.
        return false;
    });

This is what the flow looks like:

  • shutDownSequence(exitCode, signal) is called and starts "Running in the background".
  • nodeCleanup.uninstall() is called (this is to prevent an infinite loop when shutDownSequence kills the process). This happens right after, it doesn't wait for shutDownSequence to finish!
  • Then, return false. This tells node-cleanup to not actually shutdown the process, since we are still running our async tasks. At this point, node-cleanup has done everything it needs to do (since the function has returned).
  • Eventually, shutDownSequence completes whatever it needs to do, and calls process.kill(process.pid, signal); to finally exit node.

Notice that we actually want to call shutDownSequence without await. Why?

  1. The function passed to node-cleanup isn't (and can't be) an async function, so it wouldn't work anyway.
  2. Because we need the two lines after our call to run before shutDownSequence finishes, to make sure that shutDownSequence can properly kill the process without being interrupted by node-cleanup catching the kill signal.

We can safely ignore the promise returned shutDownSequence because it will never resolve anyway, since we run process.kill(process.pid, signal); to kill the process.

Seshpenguin avatar Jul 15 '19 15:07 Seshpenguin

Thanks for that explanation. So I guess the answer to my original question is "sort of"... await directly in the cleanup callback doesn't work, but called functions can still be async and their internal awaits will still work. It seems strange that the first callback can't be defined as async and use awaits like normal, but as long as I know.

Something along the way lead me to believe that if you don't call a function with await, then it won't actually execute the function as an asynchronous one. I think from now on I will always define the callbacks in two functions like your example whether or not I would need async/await at that time. Then if I ever need to add it, I don't have to rewrite the code.

noyearzero avatar Jul 15 '19 17:07 noyearzero

I'm glad I could help!

Just as an aside, async/await came with ES2017, and this module predates that.

Seshpenguin avatar Jul 15 '19 17:07 Seshpenguin

Just as an aside, async/await came with ES2017, and this module predates that.

I've been using nodeCleanup for historical reasons, but there are more modern and more popular modules nowadays that support async.

dandv avatar Oct 16 '19 22:10 dandv

@dandv Not sure what async-exit-hook does differently, to me it seems like it doesn't support async exits at all, not properly, and all in all they are just doing what @Seshpenguin is suggesting you do with node-cleanup :/

Correct me if I'm wrong, please!

fjeddy avatar Nov 25 '19 00:11 fjeddy

Is there some reason you couldn't use Promise/then() syntax instead of async/await?

// Perform asynchronous cleanup upon receiving a signal
nodeCleanup(function (exitCode, signal) {
  if (signal) {
    stopServer().then(() => {
      // calling process.exit() won't inform parent process of signal
      process.kill(process.pid, signal)
    })
    nodeCleanup.uninstall() // don't call cleanup handler again
    return false
  }
})

async function stopServer() {
  // Do async shutdown details here
}

robross0606 avatar Mar 10 '21 22:03 robross0606

Can someone explain, when (or why) would signal be null in nodeCleanup(function (exitCode, signal) {}), and how should async cleanup be handled in that case? @types/node-cleanup defines signal as string | null

alex996 avatar Mar 20 '21 20:03 alex996

Is there some reason you couldn't use Promise/then() syntax instead of async/await?

Instinctively, this is how I did it, not realizing nodeCleanup wasn't async lol. I'm using it to close a MongoDB connection.

I read that keeping the MongoDB connection open was best practice but I needed a way to close it when the process terminated. I ran a check to see if DB connection was open before trying to close it, then checked again afterwards to confirm this. Works as expected. Simple and necessary. That makes a great module in my opinion.

Cleanup(function (exitCode, signal) {
    if (signal) {
        if (DBclient.isConnected()) {
            console.log("\nDB Connection Open");
            DBclient.close()
            .then(() => {
                if (!DBclient.isConnected()) {
                    console.log("DB Connection Closed");
                    console.log(`closing Signal: ${signal}`);
                    process.kill(process.pid, signal);
                }
            });
        }
        Cleanup.uninstall();
        return false;
    }
});

My Output is this (when my DB connection was previously opened)

^C DB Connection Open DB Connection Closed Closing Signal: SIGINT

BuildersRejected avatar May 06 '21 01:05 BuildersRejected