Skip to content

Commit

Permalink
fix: use stdin=inhert on vanilla run
Browse files Browse the repository at this point in the history 
* fix: non-utf8 stdin processing
Fixes #890
Ref FormidableLabs/webpack-dashboard#16

The library blessed inside of the webpack-dashboard does some funky stuff with std streams and the expectation of the encoding. I'm not 100% sure of this change, I've got to see all the tests, but this tweak moves to inherit the child's stdin and doesn't purposely encode the stdin stream as utf8 (which, I don't recall exactly why I did this in the first place…though I'm fairly sure it was another issue somewhere else in nodemon).
* fix: use stdin:inherit when we're not forked
* fix: don't use inherit when required
* style: lint
* style: I am going to 🐎 kick the lint in the 🌰
  • Loading branch information
@remy
remy authored Aug 18, 2016
1 parent ee110ad commit 58a236e
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions lib/monitor/run.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ function run(options) {
var stdio = ['pipe', 'pipe', 'pipe'];

if (config.options.stdout) {
stdio = ['pipe', process.stdout, process.stderr];
stdio = [process.send || config.required ? 'pipe' : 'inherit',
process.stdout,
process.stderr,];
}

var sh = 'sh';
Expand Down Expand Up @@ -227,7 +229,8 @@ function run(options) {
// connect stdin to the child process (options.stdin is on by default)
if (options.stdin) {
process.stdin.resume();
process.stdin.setEncoding('utf8');
// FIXME decide whether or not we need to decide the encoding
// process.stdin.setEncoding('utf8');
process.stdin.pipe(child.stdin);

bus.once('exit', function () {
Expand Down

4 comments on commit 58a236e

@EngineeredTruth
Copy link

@EngineeredTruth EngineeredTruth commented on 58a236e Aug 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I run 'nodemon' my node server doesn't automatically restart when I make changes and save. Was this intentional?

@dylansmith
Copy link

@dylansmith dylansmith commented on 58a236e Aug 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @remy, this update caused a bit of havok with our setup. We use docker-compose for development, and each of our services is defined something like this:

my-service:
  image: some-image
  command: bash -c "./wait.sh && npm run dev"
  depends_on:
    - database
  • wait.sh is a bash script that waits for a required database container to be running
  • the npm run dev script does this: NODE_ENV=local nodemon index.js

Since 1.10.1, starting the services results in the following error (changing nodemon to node works as expected):

database (x.x.x.x:5432) open
[nodemon] 1.10.1
[nodemon] to restart at any time, enter `rs`
[nodemon] watching: *.*
[nodemon] starting `node ./src/server.js`
_stream_readable.js:512
    dest.end();
        ^
TypeError: Cannot read property 'end' of null
    at Socket.onend (_stream_readable.js:512:9)
    at Socket.g (events.js:286:16)
    at emitNone (events.js:91:20)
    at Socket.emit (events.js:185:7)
    at endReadableNT (_stream_readable.js:975:12)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)

I've locked down our nodemon versions to 1.10.0 for now.

@nkelly75
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dylansmith - I see something very similar. Running in dev mode using nodemon kicked off via docker-compose.

@remy
Copy link
Owner Author

@remy remy commented on 58a236e Aug 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now reverted in [email protected]

Please sign in to comment.