testdouble / scripty

Because no one should be shell-scripting inside a JSON file.
MIT License
964 stars 23 forks source link

Signals like SIGHUP aren't propogated #44

Open Schoonology opened 7 years ago

Schoonology commented 7 years ago

We're using scripty for our npm start script, under supervisord, in all environments. The supervisor is having difficulty shutting down the server, as it appears only scripty receives the signal (for one reason or another, we're not sure if it's configured for SIGHUP or SIGTERM at the moment).

I can think of two reasonable solutions:

  1. Propagate signals through scripty to the managed process tree.
  2. Add to the documentation a recommendation against using scripty for npm start, under supervisors (e.g. supervisord, upstart), or both.
searls commented 7 years ago

I think we should forward any signals received upward

Schoonology commented 7 years ago

Okay. I have a local, minimal, ad hoc test case, and can at least look at a fix, but I'm not sure I have the time this week to work on a regression test.

jasonkarns commented 7 years ago

This may just be a problem with npm. I remember running into precisely this issue at client with a plain (non-scripty) npm script. Though that was back in npm 1.x days. No idea if it's been resolved with npm 2 or 3.

jasonkarns commented 5 years ago

@Schoonology still an issue, I presume? any chance to hack up a test case?

korniychuk commented 3 years ago

I've decided to give up using scripty due to issues with SIGNALs and wrote.sh script paths directly to package.json. Eventually reducing the number of third-party dependencies is always good.

"start:prod": "scripty" -> "start:prod": "./npm-scripts/start/prod.sh"

One more notice: don't forget to prepend your final node calls with exec in .sh script. Otherwise, SIGNALs won't be transferred from SH to Node because if a child process started in blocking mode SH can't listen to signals.

#!/bin/sh
... some condiguring code ...
exec node ./app.js