[BUG] npm >= 9.6.7 does not pass signals to the running script #6547
Comments
zdm
added
Bug
|
This is major issue, that may lead to the data loss. |
|
I created repo to reproduce this bug. https://github.com/zdm/npm-issue-547 Please, see README.. Could you please fix it? This is very important for us. |
|
Guys, |
|
I guess this is caused by: |
|
Yes. |
|
Is it possible to fix this? I am personally currently loaded on 200% and absolutely have no time to study and dig npm sources. |
|
@wraithgar |
|
This is complete shit. |
|
I just encountered this in npm v9.6.7 after upgrading my docker container to This was working in npm v8.19.4 that was shipped with |
|
I've run into the same issue in #6684 -- and it's more than signals are not passed to the final script: npm itself doesn't terminate when it receives a SIGTERM. |
|
@zdm I noticed you closed this bug -- is it fixed? |
|
Why was this closed? |
|
Could we reopen this issue, and merge npm/run-script#188 |
## Description This PR reverts the changes made in commit `545f3be94d412941537ad0011717933d48cb58cf`, which inadvertently broke signal forwarding to child processes (PR #142 ). Contrary to the assumptions by @nlf , `SIGTERM` and similar signals are not being correctly propagated to child processes. Instead, they are only received by npm, resulting in incomplete signal handling. The removal of signal forwarding in #142 means that child processes do not receive necessary signals for appropriate cleanup and termination. This issue is evident in workflows involving `npm start` used as a Docker command for local execution. For instance, using CTRL + C does not properly terminate the application and results in a forced kill after a 10-second delay. This behavior could lead to more significant problems in production environments, (if `npm` is used to start the app) such as data loss due to improper database connection closures. ## Minimal Reproduction Steps Create a package.json with the following content: ```json { "name": "npm", "scripts": { "start": "node ./main-test.js" } } ``` Create a main-test.js file: ```typescript const interval = setInterval(() => console.log('alive!'), 3000); async function onSignal(signal) { console.log(`${signal} received, cleaning up...`); clearInterval(interval); console.log('Cleaning up done'); } process.on('SIGINT', onSignal); process.on('SIGTERM', onSignal); ``` Execute `npm start`. The script should output `alive!` every 3 seconds. Attempt to terminate it using `kill -SIGTERM [PID]`. It should log `Cleaning up done` and shut down gracefully, which it does in older versions of `npm` (e.g., `v8.19.4`) but fails in newer versions (e.g., `v9.6.7`). ## Impact Reverting this change will restore the expected behavior for signal handling in `npm` # References - npm/cli#6547 - npm/cli#6684 - #142
This PR reverts the changes made in commit `545f3be94d412941537ad0011717933d48cb58cf`, which inadvertently broke signal forwarding to child processes (PR #142 ). Contrary to the assumptions by @nlf , `SIGTERM` and similar signals are not being correctly propagated to child processes. Instead, they are only received by npm, resulting in incomplete signal handling. The removal of signal forwarding in #142 means that child processes do not receive necessary signals for appropriate cleanup and termination. This issue is evident in workflows involving `npm start` used as a Docker command for local execution. For instance, using CTRL + C does not properly terminate the application and results in a forced kill after a 10-second delay. This behavior could lead to more significant problems in production environments, (if `npm` is used to start the app) such as data loss due to improper database connection closures. Create a package.json with the following content: ```json { "name": "npm", "scripts": { "start": "node ./main-test.js" } } ``` Create a main-test.js file: ```typescript const interval = setInterval(() => console.log('alive!'), 3000); async function onSignal(signal) { console.log(`${signal} received, cleaning up...`); clearInterval(interval); console.log('Cleaning up done'); } process.on('SIGINT', onSignal); process.on('SIGTERM', onSignal); ``` Execute `npm start`. The script should output `alive!` every 3 seconds. Attempt to terminate it using `kill -SIGTERM [PID]`. It should log `Cleaning up done` and shut down gracefully, which it does in older versions of `npm` (e.g., `v8.19.4`) but fails in newer versions (e.g., `v9.6.7`). Reverting this change will restore the expected behavior for signal handling in `npm` - npm/cli#6547 - npm/cli#6684 - #142
Is there an existing issue for this?
This issue exists in the latest npm version
Current Behavior
I have docker sontainer with nodejs application.
Application started using
npm run prodcommand.It starts script
main.js.This script waiting for
SIGTERMand exits.With npm <= 9.6.6 - when I send TERM signal, to the container it works as expected.
With npm >= 9.6.7 - main script just not receive signal.
Tested with all nom versions from 9.6.5 - 9.7.1.
Bug was introduced in v9.6.7.
Expected Behavior
Please, see above.
Steps To Reproduce
Please, see above.
Environment
; copy and paste output from `npm config ls` hereThe text was updated successfully, but these errors were encountered: