remy / nodemon

Monitor for any changes in your node.js application and automatically restart the server - perfect for development
http://nodemon.io/
MIT License
26.21k stars 1.72k forks source link

Nodemon doesn't wait for async graceful exit handlers #2117

Closed simon-abbott closed 9 months ago

simon-abbott commented 1 year ago

Expected behaviour

The process should keep running in the foreground until the shutdown handler is finished.

Actual behaviour

The process continues in the foreground until the first await, then "soft exists" (causes the terminal prompt to be displayed), but doesn't kill the process.

https://github.com/remy/nodemon/assets/43803932/d1cc85cb-e2a7-4d21-a128-655f65d3b81c

Steps to reproduce

Run this file via nodemon, then ctrl+c it to shut it down:

/* file: hello.js */
const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms));

const interval = setInterval(() => console.log('tick'), 1000);

async function shutdown() {
  console.log('shutting down');
  await sleep(1000);
  clearInterval(interval);
  console.log('shutdown complete');
  process.exit(0);
}

process.on('SIGHUP', () => shutdown());
process.on('SIGINT', () => shutdown());
process.on('SIGTERM', () => shutdown());
github-actions[bot] commented 1 year ago

This issue has been automatically marked as idle and stale because it hasn't had any recent activity. It will be automtically closed if no further activity occurs. If you think this is wrong, or the problem still persists, just pop a reply in the comments and @remy will (try!) to follow up. Thank you for contributing <3

simon-abbott commented 1 year ago

This is definitely not stale, it's a real pain when running our webserver in dev mode, and it keeps messing with my terminal.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as idle and stale because it hasn't had any recent activity. It will be automtically closed if no further activity occurs. If you think this is wrong, or the problem still persists, just pop a reply in the comments and @remy will (try!) to follow up. Thank you for contributing <3

simon-abbott commented 1 year ago

Still not fixed, please keep open.

remy commented 1 year ago

I've just been giving this a test, and without npx - I get the desired result.

i.e. globally installed, or called directly.

The problem, I'm guessing, is that the signal is caught by npx (or npm when running with npm run X) and then passed to nodemon then passed to the sub process. nodemon intentionally sets timers to check if the process is still running (or needs to timeout), but npx isn't doing that and exits back to the terminal.

I'm not sure what to do here since it's outside of nodemon that the exit to shell is occurring, unless you have any suggestions? (I'd say this is a "not a bug", but it's definitely annoying, I can see that).

simon-abbott commented 1 year ago

Huh. That's interesting. I get the same result. Running nodemon directly works (though it sends SIGINT 3 times rather than 1 for some reason, but that's a separate issue). However running the test file directly through node or npx also works as expected. It's only when running nodemon through npx / npm run that it fails, so it still feels like it's more likely a nodemon bug to me.

Since npx / npm run work properly on the test file that suggests to me that they know how to be patient with scripts not exiting properly. Perhaps there's some logic bug with use of child_process? That seems the most likely culprit to me. Aside from that I also have nothing to go on without digging in and debugging the actual shutdown process, which I really do not have time for soon, unfortunately.

And before someone suggests "just run nodemon directly then", all the places we call nodemon are in npm scripts in our package.json. Refactoring that is not really an option for us.

sujeet-agrahari commented 1 year ago

Having the same issue!

❯ pnpm start

> node-fastify-architecture@1.0.0 start /Users/sujeet/github/node-fastify-architecture
> NODE_ENV=development nodemon --require=dotenv/config  src/server.js -L

[nodemon] 3.0.1
[nodemon] to restart at any time, enter `rs`
[nodemon] watching path(s): *.*
[nodemon] watching extensions: js,mjs,cjs,json
[nodemon] starting `node --require=dotenv/config src/server.js`
[11:19:13.312] INFO (54382): Database connected!
[11:19:13.345] INFO (54382): 
 └── /
    ├── fastify-overview-ui/
    │   └── * (HEAD, GET)
    ├── json-overview-ui (GET, HEAD)
    ├── api/v1/auth/login (POST)
    └── health (GET, HEAD)

[11:19:13.346] INFO (54382): Server listening at http://127.0.0.1:3000
^C ELIFECYCLE  Command failed.

 ❯ [11:19:16.065] INFO (54382): Got SIGINT! Shutting down gracefully      ✘ INT  4s  system
[11:19:16.075] INFO (54382): Database connection closed!

It just hangs in there and the process never gets closed!

Workaround: Don't use nodemon, use node --watch instead if you are using node v18

github-actions[bot] commented 1 year ago

This issue has been automatically marked as idle and stale because it hasn't had any recent activity. It will be automtically closed if no further activity occurs. If you think this is wrong, or the problem still persists, just pop a reply in the comments and @remy will (try!) to follow up. Thank you for contributing <3

simon-abbott commented 1 year ago

Please don't close <3

github-actions[bot] commented 11 months ago

This issue has been automatically marked as idle and stale because it hasn't had any recent activity. It will be automtically closed if no further activity occurs. If you think this is wrong, or the problem still persists, just pop a reply in the comments and @remy will (try!) to follow up. Thank you for contributing <3

simon-abbott commented 11 months ago

Don't close yet please!

github-actions[bot] commented 11 months ago

This issue has been automatically marked as idle and stale because it hasn't had any recent activity. It will be automtically closed if no further activity occurs. If you think this is wrong, or the problem still persists, just pop a reply in the comments and @remy will (try!) to follow up. Thank you for contributing <3

simon-abbott commented 11 months ago

Still an issue, please don't close!

github-actions[bot] commented 10 months ago

This issue has been automatically marked as idle and stale because it hasn't had any recent activity. It will be automtically closed if no further activity occurs. If you think this is wrong, or the problem still persists, just pop a reply in the comments and @remy will (try!) to follow up. Thank you for contributing <3

simon-abbott commented 10 months ago

Don't close please.

remy commented 10 months ago

So the problem is down to the use of npx - as described in an earlier comment. Nodemon can only (currently) offer documented work around - unless someone has a code fix?

I can add it to the faq...

github-actions[bot] commented 10 months ago

This issue has been automatically marked as idle and stale because it hasn't had any recent activity. It will be automtically closed if no further activity occurs. If you think this is wrong, or the problem still persists, just pop a reply in the comments and @remy will (try!) to follow up. Thank you for contributing <3

github-actions[bot] commented 9 months ago

Automatically closing this issue due to lack of activity