Node v0.10.11 breaks Lo-Dash's build script, related to `compiler.stdin.end(source)` #5688

Closed
jdalton opened this Issue Jun 14, 2013 · 8 comments

Comments

Projects
None yet
4 participants

jdalton commented Jun 14, 2013

Node v0.10.11 breaks Lo-Dash's build script, related to compiler.stdin.end(source).

It passes an exception to the callback onClosureSimpleCompile callback, resulting in...

/Users/jdalton/Projects/lodash/build/minify.js:530
      throw exception;
            ^
Error: stdin:262: ERROR - Parse error. unterminated comment
  /**
  ^

stdin:266: ERROR - Parse error. missing } after function body
   * @param {Array} [array=[]] The arr
                                     ^

2 error(s), 0 warning(s)

    at ChildProcess.<anonymous> (/Users/jdalton/Projects/lodash/build/minify.js:427:27)
    at ChildProcess.EventEmitter.emit (events.js:98:17)
    at Process.ChildProcess._handle.onexit (child_process.js:789:12)

Before this, #5622 broke the Lo-Dash's build from Node v0.10.8-0.10.10 :(
Any idea what it is?

jdalton commented Jun 14, 2013

BTW I wrote the source I'm passing to compiler.stdin.end(...) to a file and tested it.
It's valid and complete so something is getting lost in between.

Also the build script works as expected in Node v0.10.7.

Owner

bnoordhuis commented Jun 14, 2013

Do you have a reduced test case I can try? I'd rather not go through several hundred lines of code.

jdalton commented Jun 14, 2013

Here is a reduced test case. It requires a jquery.js file and compiler.jar in the same directory.

#!/usr/bin/env node

var cp = require('child_process'),
    fs = require('fs'),
    path = require('path');

var closurePath = path.join(process.cwd(), 'compiler.jar'),
    compiler = cp.spawn('java', ['-jar', closurePath, '--warning_level=QUIET', '--compilation_level=SIMPLE_OPTIMIZATIONS']);

var inputPath = path.join(process.cwd(), 'jquery.js'),
    outputPath = path.join(process.cwd(), 'jquery.min.js');

var error = '';
compiler.stderr.on('data', function(data) {
  error += data;
});

var output = '';
compiler.stdout.on('data', function(data) {
  output += data;
});

compiler.on('exit', function(status) {
  if (status) {
    var exception = new Error(error);
    exception.status = status;
  }
  if (exception) {
    throw exception;
  }
  console.log('Success!');
  fs.writeFileSync(outputPath, output, 'utf-8');
});

var source = fs.readFileSync(inputPath);
compiler.stdin.end(source);

It should log Success! and write a jquery.min.js file.
What happens instead is something like:

/Users/jdalton/Projects/lodash/vendor/closure-compiler/minify.js:29
    throw exception;
          ^
Error: stdin:314: ERROR - Parse error. missing } after function body
            // Exte
                  ^

stdin:314: ERROR - Parse error. missing } in compound statement
            // Exte
                  ^

2 error(s), 0 warning(s)

    at ChildProcess.<anonymous> (/Users/jdalton/Projects/lodash/vendor/closure-compiler/minify.js:25:21)
    at ChildProcess.EventEmitter.emit (events.js:98:17)
    at Process.ChildProcess._handle.onexit (child_process.js:789:12)

Quite possibly related: I'm also seeing problems in my codebase specific to node 0.10.11 and related to input or output streams returned by child_process.spawn(). In my case, the results of the child_process read back from its stdout stream are truncated relative to the behavior of node 0.10.10. I'm not sure yet if this is because the data written to the stdin stream was disrupted, or if the stdout stream is signalling end of stream too early.

A clue may be this notation in the node 0.8.25 release:

  • child_process: fix handle delivery (Ben Noordhuis)

Is it possible that some child_process stream code intended only for the 0.8 branch leaked over to 0.10 and is incompatible with the significant streaming i/o changes in 0.10? Just a guess.

Here is the smallest test case I can find to reproduce this. This program should write a 100000 line file named foo.txt. It works correctly prior to v0.10.11 On my machine running 0.10.11 the output in foo.txt is truncated at line 12563.

 (function() {
   var cproc, error, fout, fs, p, x, _i;

   cproc = require('child_process');

   fs = require('fs');

   try {
      fout = fs.createWriteStream('foo.txt');
   } catch (_error) {
      error = _error;
      console.error("ERROR: Could not open file");
      process.exit(1);
   }

   p = cproc.spawn('sort', null, {});

   p.stdout.setEncoding('ascii');

   p.stderr.setEncoding('ascii');

   p.stdout.on('data', function(data) {
      return fout.write(data);
   });

   p.stderr.on('data', function(data) {
      return process.stderr.write(data);
   });

   p.stdout.on('end', function() {
      process.stderr.write("\nEND Recieved\n");
      return fout.end(function() {
        return process.stderr.write("\nOutput file closed.\n");
      });
   });

   p.on('exit', function(code) {
      return process.stderr.write('Process exited with code: ' + code);
   });

   for (x = _i = 1; _i <= 100000; x = ++_i) {
      p.stdin.write("Filler text...................................................................." + x + "\n");
   }

   p.stdin.end();

 }).call(this);

isaacs commented Jun 14, 2013

Hm.. This looks like e0519ac may be to blame.

Can you try applying this patch and see if it fixes the issue? https://gist.github.com/isaacs/5785747

@isaacs isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Jun 14, 2013

@isaacs isaacs net: Do not destroy socket mid-write
The fix in e0519ac is overly zealous,
and can destroy a socket while there are still outstanding writes in
progress.

Closes GH-5688
6966212

jdalton commented Jun 15, 2013

@isaacs I tested a build with this patch and it seems to fix the issue :D

@isaacs isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Jun 17, 2013

@isaacs isaacs net: Do not destroy socket mid-write
The fix in e0519ac is overly zealous,
and can destroy a socket while there are still outstanding writes in
progress.

Closes GH-5688
3c7945b
Owner

bnoordhuis commented Jun 17, 2013

Landed (and therefore fixed) in 3c7945b. Thanks for the bug report.

bnoordhuis closed this Jun 17, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment