Handling errors in stream mode

[rimassa]

Hello!

Suppose the following image operation:
var stream = gm('piano.png').crop(100, 100, 0, 0).stream('jpg')
How can I be notified of processing errors (such as if "piano.png" couldn't be opened)?

I've tried the alternative:
gm('piano.png').crop(100, 100, 0, 0).stream('jpg', function (err, stdout, stderr) {...})
Even in "piano.png" doesn't exists, the err parameter in the callback is null (that is expected).
If I attach a data event listener on stream stderr, I'm able to receive error messages - and detect processing errors.
But I've tried with a PNG file (http://www.iphonestheme.com/uploads/allimg/130206/1-1302061353440-L.png), and although the image was successfully processed, I still got some warnings coming from the stderr stream:

convert: iCCP: known incorrect sRGB profile `/Users/rgermano/iG/workspaces/iglr-app/test/resources/img-validator/piano_600x1636.png' @ warning/png.c/MagickPNGWarningHandler/1830.

Is there an approach using streams to catch processing errors only (not warnings)?

Thanks!
Ricardo

[jonathanong]

hmmm that would be a bug. errors should be buffered and returned as an Error instance.

[kevinoid]

Unless I am mistaken, fixing this issue will require a change to the API. The problem is that the only way stream() could return an Error resulting from the process exit code is if it waits to call the callback until the process exits, which would prevent the caller from reading the streams until after the process exits (possibly resulting in a deadlock or large memory bloat).

Assuming this is correct, would it be acceptable to either pass the ChildProcess as an additional argument to the stream callback or to add a method similar to stream() which returns the ChildProcess instead of the stream objects? Would you be willing to accept a PR which implemented either of these, or another solution?

[jbergstroem]

We've been hitting this as well using gm 1.16.0. Two variations:

Stack: Error: Command failed: convert: iCCP: extra compressed data `/tmp/Screen-Shot-2014-10-27-at-12.30.46-am.png' @ warning/png.c/MagickPNGWarningHandler/1831.

   at ChildProcess._spawn.proc.on.onExit (/foo/node_modules/gm/lib/command.js:243:17)
   at ChildProcess.emit (events.js:98:17)
   at maybeClose (child_process.js:756:16)
   at Process.ChildProcess._handle.onexit (child_process.js:823:5)

Stack: Error: Command failed: convert: iCCP: extra compressed data `/tmp/Steve-Jobs.png' @ warning/png.c/MagickPNGWarningHandler/1831.

   at ChildProcess._spawn.proc.on.onExit (/foo/node_modules/gm/lib/command.js:243:17)
   at ChildProcess.emit (events.js:98:17)
   at maybeClose (child_process.js:756:16)
   at Socket.<anonymous> (child_process.js:969:11)
   at Socket.emit (events.js:95:17)
   at Pipe.close (net.js:465:12)

[santiagoaguiar]

I hit the same issue.

Agree with @kevinoid , simplest fix is pass the ChildProcess to the callback when using unbuffered stream mode. Would you be willing to accept a PR that implement that solution?

When the stream is buffered, this is already handled by the _spawn function on command.js. But when the callback for unbuffered mode is called, proc is no longer accessible and we can't listen to those events.

Change would be non-breaking and can serve as a workaround, since this issue is otherwise very serious.

The stream() API method is a bit dangerous now as it stands, since it seems that on failures you might be left listening for events on the streams forever.

[paulmelnikow]

Is anyone still following this issue? I opened a patch at #647. Would appreciate extra eyes!