strongloop / node-foreman

A Node.js Version of Foreman
http://strongloop.github.io/node-foreman/
Other
1.27k stars 119 forks source link

Unknown signal: null error with Node 8 #143

Closed ilkkao closed 6 years ago

ilkkao commented 7 years ago

I started to see this error when I upgraded to Node 8. Steps to repro on Mac:

  1. brew install redis

  2. Procfile: redis: redis-server

  3. nf (and then CONTROL-C)

08:04:13 redis.1   |                  _._
08:04:13 redis.1   |             _.-``__ ''-._
08:04:13 redis.1   |        _.-``    `.  `_.  ''-._           Redis 3.2.9 (00000000/0) 64 bit
08:04:13 redis.1   |    .-`` .-```.  ```\/    _.,_ ''-._
08:04:13 redis.1   |   (    '      ,       .-`  | `,    )     Running in standalone mode
08:04:13 redis.1   |   |`-._`-...-` __...-.``-._|'` _.-'|     Port: 6379
08:04:13 redis.1   |   |    `-._   `._    /     _.-'    |     PID: 26996
08:04:13 redis.1   |    `-._    `-._  `-./  _.-'    _.-'
08:04:13 redis.1   |   |`-._`-._    `-.__.-'    _.-'_.-'|
08:04:13 redis.1   |   |    `-._`-._        _.-'_.-'    |           http://redis.io
08:04:13 redis.1   |    `-._    `-._`-.__.-'_.-'    _.-'
08:04:13 redis.1   |   |`-._`-._    `-.__.-'    _.-'_.-'|
08:04:13 redis.1   |   |    `-._`-._        _.-'_.-'    |
08:04:13 redis.1   |    `-._    `-._`-.__.-'_.-'    _.-'
08:04:13 redis.1   |        `-._    `-.__.-'    _.-'
08:04:13 redis.1   |            `-._        _.-'
08:04:13 redis.1   |                `-.__.-'
08:04:13 redis.1   |  26996:M 31 May 08:04:13.186 # Server started, Redis version 3.2.9
08:04:13 redis.1   |  26996:M 31 May 08:04:13.899 * DB loaded from disk: 0.713 seconds
08:04:13 redis.1   |  26996:M 31 May 08:04:13.899 * The server is now ready to accept connections on port 6379
^C[WARN] Interrupted by User
[DONE] Killing all processes with signal  SIGINT
08:04:16 redis.1   |  26996:signal-handler (1496210656) Received SIGINT scheduling shutdown...

08:04:16 redis.1   |  26996:M 31 May 08:04:16.425 # User requested shutdown...
08:04:16 redis.1   |  26996:M 31 May 08:04:16.425 * Saving the final RDB snapshot before exiting.
 ✘  ~/git/mas   master ●  08:04:17 redis.1   |  26996:M 31 May 08:04:17.010 * DB saved on disk
08:04:17 redis.1   |  26996:M 31 May 08:04:17.010 # Redis is now ready to exit, bye bye...
internal/util.js:183
  throw new errors.Error('ERR_UNKNOWN_SIGNAL', signal);
  ^

Error [ERR_UNKNOWN_SIGNAL]: Unknown signal: null
    at convertToValidSignal (internal/util.js:183:9)
    at ChildProcess.kill (internal/child_process.js:373:5)
    at EventEmitter.<anonymous> (/Users/ilkkao/git/mas/server/node_modules/foreman/lib/proc.js:54:11)
    at emitOne (events.js:115:13)
    at EventEmitter.emit (events.js:210:7)
    at ChildProcess.<anonymous> (/Users/ilkkao/git/mas/server/node_modules/foreman/lib/proc.js:50:13)
    at emitTwo (events.js:125:13)
    at ChildProcess.emit (events.js:213:7)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:197:12)
ilkkao commented 7 years ago

Actually no redis needed. Procfile can be something: echo "foo"

sdalezman commented 7 years ago

@ilkkao i had the same issue when upgrading to node 8 yesterday. i haven't had a chance to really dig into the issue and what exactly changed in node 8 (there were some updates to child process kill signals), but the issue seems to be that the event emitter in proc.js is receiving a null value for signal. the event emitter is called from a child process exit event. exit events receive two arguments (code, signal). only of them is guaranteed to be not null, but if you look at the code in proc.js:

  child.on('exit', function(code, signal) {
    emitter.emit('killall', signal);
  });

it's passing signal which in this case is null and is an invalid event signal. I think node8 had some cleanups around event signal sanitization that caused this error to show for the first time. if i have some time will take a deeper look this weekend

mitchellporter commented 7 years ago

@sdalezman Have you looked into this yet or made any progress?

meowgorithm commented 7 years ago

+1. Happening for me on Node v8.1.4 on MacOS 10.12.5.

spochart commented 7 years ago

+1

sdalezman commented 7 years ago

@mitchellporter i haven't had a chance to dig into why it started happening in node 8. but it seems to be that throughout proc.js the exit event listeners aren't accepting the right number of arguments and are passing null values to the emitter.

to be honest, i probably don't have time to open a pr this week to fix it but will try to this weekend.

sdalezman commented 7 years ago

@slnode can you please confirm to move forward with a pr patch that updates proc.js and proxy.js which both have event listeners for the exit event. Both pass signal as an argument when it's possibly a null value. perhaps, i'm missing something around the implementation, but i don't see anything in the code that prevents a program from exiting with a null signal. since the node documentation guarantees one of code or signal to be a non null it's a pretty simple fix.

https://nodejs.org/api/child_process.html#child_process_event_exit

jpap commented 7 years ago

+1

davidmfoley commented 7 years ago

I had this problem and made a quick hack -- this has no tests and probably has other problems that don't matter for my use case, but might be useful if you are having the same problem: https://github.com/lightboard/node-foreman/commit/a3982bc495f693aefdb5ade6dda6c5d88bb0effe

mnemanja commented 7 years ago

I'm getting the same (similar) error. How to solve it? Any progress?

> nf start

[OKAY] Loaded ENV .env File as KEY=VALUE Format
23:08:29 electron.1   |  starting electron
23:08:30 react.1      |  > rentacar-manager@0.1.0 start F:\www\rentacar-manager-v2
23:08:30 react.1      |  > react-scripts start
23:08:31 react.1      |  Something is already running on port 5000.
[DONE] Killing all processes with signal  null
internal/util.js:183
  throw new errors.Error('ERR_UNKNOWN_SIGNAL', signal);
  ^

Error [ERR_UNKNOWN_SIGNAL]: Unknown signal: null
    at convertToValidSignal (internal/util.js:183:9)
    at ChildProcess.kill (internal/child_process.js:373:5)
    at EventEmitter.<anonymous> (F:\www\rentacar-manager-v2\node_modules\foreman\lib\proc.js:54:11)
    at emitOne (events.js:120:20)
    at EventEmitter.emit (events.js:210:7)
    at ChildProcess.<anonymous> (F:\www\rentacar-manager-v2\node_modules\foreman\lib\proc.js:50:13)
    at emitTwo (events.js:125:13)
    at ChildProcess.emit (events.js:213:7)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:197:12)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! rentacar-manager@0.1.0 dev: `nf start`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the rentacar-manager@0.1.0 dev script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\neman\AppData\Roaming\npm-cache\_logs\2017-10-08T21_08_31_779Z-debug.log
barbalex commented 7 years ago

Using foreman for the first time I spent quite some time debugging my code until I finally found this issue stating that it is not my code but rather foreman and node 8.

Seing that this issue is five months old I will have to find a way not to use foreman.

barbalex commented 7 years ago

To make the issue more clear: My code contains lots of async functions. And they will not work in node < v8.

davidmfoley commented 7 years ago

@barbalex @mnemanja see my quick fix (ie hack) for foreman on node8 above

Hackbyrd commented 6 years ago

This issue still has not been addressed. Can someone please address this issue?

barbalex commented 6 years ago

The last commit was in August 2016. Seems like this code is not getting a lot of love any more :-(

barbalex commented 6 years ago

@davidmfoley I finally tried your fix. And it works great! Please go get a beer.

vancouverwill commented 6 years ago

https://github.com/heroku/node-foreman also works

js2me commented 6 years ago

I have same problem on all Windows versions, but on the Linux it works. But if I use tasks separately then this tasks works

[WARN] No ENV file found
01:02:23 server.1           |  "PORT" ?? ????? ???????? ??? ?????
01:02:23 server.1           |  ????????, ??????? ?????? ??? ?????? ????.
[DONE] Killing all processes with signal  null
internal/util.js:192
  throw new errors.TypeError('ERR_UNKNOWN_SIGNAL', signal);
  ^

TypeError [ERR_UNKNOWN_SIGNAL]: Unknown signal: null
    at convertToValidSignal (internal/util.js:192:9)
    at ChildProcess.kill (internal/child_process.js:386:5)
    at EventEmitter.<anonymous> (D:\mera-projects\titan-frontend\node_modules\foreman\lib\proc.js:54:11)
    at EventEmitter.emit (events.js:185:15)
    at ChildProcess.<anonymous> (D:\mera-projects\titan-frontend\node_modules\foreman\lib\proc.js:50:13)
    at ChildProcess.emit (events.js:180:13)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:209:12)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! titan-frontend@2.20.1 start-dev: `nf start`
npm ERR! Exit status 1
npm ERR!
bajtos commented 6 years ago

Should be fixed in foreman@3.0.0

joelleibow commented 6 years ago

I'm still running into this issue on Node 8.11.1 and using foreman@3.0.0 😢