Fatal error when graphicsmagick isn't installed

[Munter]

stream.js:81
      throw er; // Unhandled stream error in pipe.
            ^
Error: The gm stream ended without emitting any data
    at Socket.module.exports.gmOperations (/root/global/falconsocial-global-page/node_modules/assetgraph-builder/node_modules/express-processimage/lib/getFiltersAndTargetContentTypeFromQueryString.js:64:59)
    at Socket.EventEmitter.emit (events.js:85:17)
    at Pipe.onread (net.js:416:51)

I think we need to implement some error handling in buildProduction to give some better feedback to the user. It's non intuitive that they need to install graphicsmagick if they don't know that gm is the executable of that package.

The same thing might apply for inkscape

[papandreou]

Hmm, what if the graphicsmagick filter isn't the last in the chain? Eg. foo.png?resize=10,20&pngcrush?

[Munter]

The change I made should still run all other filters if this specific error is encountered. I've set it at a warning level, which we might want to bump up to error instead if it isn't a good idea that the build keeps running after it encounters a missing gm install.

The important part of that patch is to catch the non-installed gm binary and give the user a message on how to resolve the problem instead of a cryptic stack trace. I have no strong feelings on wether we should stop with an error or keep going.

[papandreou]

I understand the intention, and I agree that it's an improvement :). What I was referring to was that this error handler should subscribe to all items in filters, not just the last item:

                    filters[filters.length - 1]
                        .on('error', function (err) {
                            if (err.message === 'The gm stream ended without emitting any data') {
                                assetGraph.emit('warn', new Error('processImages: ' + imageAsset.url + '\nPlease install raphicsmagick to apply the following filters: ' + filtersAndTargetContentType.usedQueryStringFragments.join(', ')));
                            } else {
                                assetGraph.emit('error', err);
                            }
                            cb();
                        })

[Munter]

Hah, must have been a late night session :)

I'll take a look at fixing that later. Also I have a feeling I might have hooked into the wrong place. an error message about missing gm should probably be sent all the way from node-gm or express-processimage

[Munter]

This sees to be fixed. I must have forgotten to close the issue after making the change