Process always exits with exit code 0 #182

Closed
Grepsy opened this Issue Sep 9, 2011 · 8 comments

4 participants

@Grepsy

The process never ends with an exit code other then 0, it seems the code in the 'drain' event is never executed because node exits before the event is fired. Should we block and wait for the event?

quit: function(code){
    //Workaround for https://github.com/joyent/node/issues/1669
    var flushed = process.stdout.flush();
    if (!flushed) {
        fs.writeSync(1, 'executes' + "\n");
        process.once("drain", function () {
            fs.writeSync(1, 'never executes' + "\n");
            process.exit(code || 0);
        });
    } else {
        fs.writeSync(1, 'never executes' + "\n");
        process.exit(code || 0);
    }
},

This is caused by the change in issue #176.

@nzakas

Do you know how to make it wait?

@Grepsy

I'm currently doing the following, works for me, but doesn't strike me as a good solution, not sure what's a nodeish way to fix this.

    while (process.stdout.flush());
    process.exit(code || 0);

Also I'm not affected by the problem this flushing change aims to solve, so can't really test whether this breaks that change.

@nzakas

Hmm, I'm not able to reproduce. When I check the exit code, I'm seeing 1. Are you using an unusually large CSS file?

@Grepsy

No, but I am running on Windows using node 0.5.4.

@nzakas

On Windows? Using Cygwin?

@brentlintner

Hey all. I ran into the same issue in node-jshint, and I found the joyent issue workaround in the codebase (thanks btw).

I noticed the drain event does fire when you listen directly off process.stdout.on (vs process.on). It seems to fix things for me, see: jshint/node-jshint@2dba79f.

That fix also catches an exception that is thrown in node 0.5.x (no flush method), and instead exists normally (the stdout cut off bug seems fixed on 0.5.x). This was actually still happening in csslint for me, although I noticed this closed issue so I was not sure. Regardless, I saw this issue and thought I would just comment here first. Hope this helps.

I was using node 0.5.9 and 0.4.12 as a reference btw.

@nzakas

@brent - thanks! This is tremendously helpful. I'll make the change in CSS Lint.

@BrentonEarl
@nzakas nzakas pushed a commit that closed this issue Oct 14, 2011
Nicholas C. Zakas Added check for flush() method on Node.js before attempting to use it…
… (fixes #182)
8d1d9ca
@nzakas nzakas closed this in 8d1d9ca Oct 14, 2011
This was referenced Apr 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment