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

Forward signals to child process #107

Merged
merged 1 commit into from
May 7, 2024
Merged

Conversation

MrJohz
Copy link
Contributor

@MrJohz MrJohz commented Apr 26, 2024

I ran into an issue where I would Ctrl-C a running process, and dotenv-cli would forward the resulting SIGINT signal to the child process, but would also immediately terminate, leaving the child process in a kind of zombie state.

You can see this in action using the following test file:

import { once } from "node:events";

// simulate a long-running process, e.g. a server
// that should only quit when SIGINT is received
const handle = setInterval(() => {}, 5000);

console.log("Running, press Ctrl+C to exit.");

await once(process, "SIGINT");
console.log("Received SIGINT, will exit in 5 seconds...");

setTimeout(() => {
  console.log("Exiting.");
  clearInterval(handle);
}, 5000);

This should produce the following output (pressing Ctrl+C at the appropriate time):

$ node test.mjs
Running, press Ctrl+C to exit.
^CReceived SIGINT, will exit in 5 seconds...
Exiting.
$

Note that the new shell prompt appears once the script has fully exited.

Running npx dotenv -- node test.mjs currently will produce the following output (again, pressing Ctrl+C at the appropriate time).

$ node test.js
Running, press Ctrl+C to exit.
^CReceived SIGINT, will exit in 5 seconds...
$ Exiting.

Note that the prompt appears once the parent dotenv program has finished, but the child test.mjs continues running after this point.

This PR explicitly forwards a number of signals to the child process. This ensures that the signals are not handled in dotenv-cli itself, but always in the child process, which prevents the above case where the parent process terminates prematurely.

I have added a bunch of signals that I think should be relevant in various cross-platform situations, but I'm not wedded to this particular set of signals here.

@entropitor entropitor merged commit 5845b5a into entropitor:master May 7, 2024
@entropitor
Copy link
Owner

https://github.com/entropitor/dotenv-cli/releases/tag/v7.4.3

@JoseGoncalves
Copy link

I've also hit this issue and using version 7.4.3 solved it. Nevertheless, I needed to load it directly from GitHub as it's not published on npm. Can that version be published on npm?

@entropitor
Copy link
Owner

Woops, small oversight. Done!

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

Successfully merging this pull request may close these issues.

3 participants