.stream swallows errors when imagemagick not found

[bblack]

still digging into this, but it seems that on e.g.

gm(readStream)
.resize(w, h)
.stream('png', function(err, stdout){
  // no err; stdout is zero-length
});

no error is reported. it seems that the callback cb in lib/command.js is being called 3 times, and the first time is not passed the error. will add more specifics as i have them.

[bblack]

it looks like on streaming, the _spawn function explicitly calls the callback immediately. what's the best way to fix this? here are some options:

• wait for the process to exit before calling the callback. largely defeats the purpose of a streaming option.
• emit the error so it can be caught. basically allows the gives the caller the ability to do the above. e.g.

gm(ReadStream)
.resize(w, h)
.stream('png', function(err, stdout){
})
.on('error', function(){ ... });

• do more. since we're trying to be clever by emitting errors, and failing, try to be even cleverer by intercepting errors before allowing the stdout stream that's passed to the callback emit end. (probably a terrible idea).
• do less. remove the .stream method entirely, and instead just expose the proc object returned by child_process.spawn. probably makes the most sense, since we're already exposing the stdout and sterr streams directly. e.g.

gm(readStream)
.resize(w, h)
.proc('png'); // this is a proc, and i get all the errors and stdout and stderr

[paulmelnikow]

Would love to see this fixed. This comes up for users of the badges CLI as well as developers – see badges/shields#883.

These two seem like the workable options:

• emit the error so it can be caught. basically allows the gives the caller the ability to do the above. e.g.

If we did this, we could also add an .on('close') event.

• do less. remove the .stream method entirely, and instead just expose the proc object returned by child_process.spawn. probably makes the most sense, since we're already exposing the stdout and sterr streams directly. e.g.

This makes sense too.

Would it make sense to offer both?

@bblack Are you still using this? If I put a patch together would you like to give it a look?

[bblack]

Are you still using this?

Yes!

If I put a patch together would you like to give it a look?

That would be great, although I make no guarantee about when.

[paulmelnikow]

Hrm, just noticed this was previously reported at #256.

[paulmelnikow]

Exposing the proc object synchronously is difficult, because the preprocessors are first invoked in series. Though it's easy enough to expose it in the callback, as suggested in #256.

Would you like to close this and continue there?