unshift error when using .execute() command

[alanosman]

Hello,

I recently was trying to update our modules for easyimage in our nodeJS app to the latest and have hit a wall on trying to figure out where the problem lies. We currently are using npm module version 2.1.0 in our application, which has been converting images quite well. I was upgrading (as we do) to 3.0.1 to stay up with the latest and ran into issues when shifting from the deprecated exec function changing to execute.

Around version 2.2.1, we start to see this warning when using the .exec() function.
but when switch to execute() function, it also fails.

When I change the code to execute we see an unshift error. The ends up being caught with our error handler on the returned promise that the execute function returns. I've experimented by changing the parameters that we are passing in, thinking that something changed there, but it doesn't seem to make a difference.

Can someone give me a clue as to what I might be doing wrong or is this a bug? I've pasted error, environment and code below.

node 7.10.1

2018-02-08T02:35:12.109Z Unhandled rejected promise: TypeError: Cannot read property 'unshift' of undefined at Object.execute  (/mnt/d/elastc-app/node_modules/easyimage/lib/execute.js:47:17) 
  at generateThumbnail  (/mnt/d/elastc-app/app/common/thumbnail/index.js:84:49) 
  at Object.generateLane  (/mnt/d/elastc-app/app/common/thumbnail/index.js:78:48)
  at Object.generate_thumbnails (/mnt/d/elastc-app/app/actions/api/tasks.js:4423:60)
  at /mnt/d/elastc-app/app/actions/api/tasks.js:3980:50
  at _fulfilled (/mnt/d/elastc-app/node_modules/q/q.js:854:54)
  at self.promiseDispatch.done (/mnt/d/elastc-app/node_modules/q/q.js:883:30)
  at Promise.promise.promiseDispatch (/mnt/d/elastc-app/node_modules/q/q.js:816:13)
  at /mnt/d/elastc-app/node_modules/q/q.js:624:44 at runSingle (/mnt/d/elastc-app/node_modules/q/
q.js:137:13)
  at flush (/mnt/d/elastc-app/node_modules/q/q.js:125:13)
  at _combinedTickCallback (internal/process/next_tick.js:73:7)
  at process._tickDomainCallback (internal/process/next_tick.js:128:9)

I am running Ubuntu on WSL (Windows)

LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 14.04.5 LTS
Release:        14.04
Codename:       trusty

I have this version of ImageMagick installed

Version: ImageMagick 7.0.7-13 Q16 x86_64 2017-12-01 http://www.imagemagick.org
Copyright: © 1999-2017 ImageMagick Studio LLC
License: http://www.imagemagick.org/script/license.php
Features: Cipher DPC HDRI OpenMP
Delegates (built-in): fontconfig freetype jbig jng jpeg lzma png tiff zlib

const Q         = require('q');
const appRoot   = '../..';
const easyimage = require('easyimage');
const path      = require('path');
const logger    = require(`${appRoot}/logger`);

const imageMagickParams = {
    detail       : '-quality 93 -thumbnail \"234\\>\" -unsharp 1.5x1+0.7+0.02 -gravity Center -extent 234 -crop x182+0+0',
    lane         : '-quality 93 -thumbnail \"231\\>\" -unsharp 1.5x1+0.7+0.02 -gravity Center -extent 231 -crop x182+0+0',
    organization : '-quality 93 -thumbnail \"227\\>\" -unsharp 1.5x1+0.7+0.02 -gravity North -extent 227 -crop x227+0+0',
    user         : '-quality 93 -thumbnail \"100\\>\" -unsharp 1.5x1+0.7+0.02 -gravity North -crop x100+0+0'
};

const imageMagickCommand = (type, sourceFilePath) => {
    const imParams            = imageMagickParams[type];
    const destinationFilePath = generateDestinationFilePath(type, sourceFilePath);

    const imCommand =
        `convert \"${sourceFilePath}\" ${imParams} \"${destinationFilePath}\"`;

    return imCommand;
};

const generateDestinationFilePath = (type, sourceFilePath) => {
    let destinationFilePath;

    if (type === 'lane' || type === 'detail') {
        var thumbFileSuffix = '_' + type;
        destinationFilePath = sourceFilePath + thumbFileSuffix + path.extname(sourceFilePath);
    } else {
        destinationFilePath = sourceFilePath;
    }

    return destinationFilePath;
};

const generateOrganization = srcFilePath    => generateThumbnail(srcFilePath,    'organization');
const generateProfile      = sourceFilePath => generateThumbnail(sourceFilePath, 'user');
const generateDetail       = sourceFilePath => generateThumbnail(sourceFilePath, 'detail');
const generateLane         = sourceFilePath => generateThumbnail(sourceFilePath, 'lane');

const generateThumbnail = (sourceFilePath, type) => {
    const deferredGenerateThumbnail = Q.defer();
    const imCommand                 = imageMagickCommand(type, sourceFilePath);
    const destinationFilePath       = generateDestinationFilePath(type,sourceFilePath);
    const easyImagePromise          = easyimage.execute(imCommand);

    easyImagePromise
        .then(() => {
            const easyImageInfoPromise = easyimage.info(destinationFilePath);

            easyImageInfoPromise
                .then(thumbnailFile => {
                    let im_filename   = thumbnailFile.name;
                    let im_name_parts = thumbnailFile.name.split(' ');

                    if (im_name_parts.length > 1) {
                        if (im_name_parts[0] !== 'Undefined')
                            logger.error('Unexpected behavior from imagemagick.info()');
                        // Do this in either case
                        im_filename = im_name_parts[1];
                    }
                    deferredGenerateThumbnail.resolve(im_filename);
                },
                err => {
                    logger.error('Error getting info for: ' + destinationFilePath + ' Error: ' + err);
                    deferredGenerateThumbnail.reject(err);
                }
            );
        },
        err => {
            logger.error('Error converting ' + sourceFilePath + ' Error: ' + err);
            deferredGenerateThumbnail.reject(err);
        });

    return deferredGenerateThumbnail.promise;
};

module.exports = {
    generateOrganization,
    generateDetail,
    generateLane,
    generateProfile
};

[mrkmg]

You will need to refactor to use the updated method of execute. https://hacksparrow.github.io/node-easyimage/globals.html#execute

It takes in the command, in your case "convert", then an array of options.

For example:

easyimage.execute("convert", ["-option1", "-option2", "etc"]);

You are still using the old "exec" style, which is to just pass a string. Version 2 deprecated that style of exec, and Version 3 completely removed it.

[alanosman]

Hi @mrkmg - thanks for the quick response.

Does this mean that all our parameters need to be individually quoted or can we pass them in together?

i.e. '-quality 93 -thumbnail \"234\\>\" -unsharp 1.5x1+0.7+0.02 -gravity Center -extent 234 -crop x182+0+0'

thanks

[alanosman]

Hi @mrkmg, ok I finally got it working. I had to provide each option and it's value in quotes separated by commas. That is not intuitive and not described in any examples or obvious from your comment. I think it would help to have a clear example so the next coder doesn't fall into the same pit of guessing like I did.

This small test script worked for me. I hope it helps the next person.

'use strict';

const Q             = require('q');
const easyimage     = require('easyimage');
const testImageIn   = 'test-image-in2.jpg';
const testImageOut  = 'test-image-out2.jpg';
const deferredImage = Q.defer();

const easyImagePromise = easyimage.execute(
    'convert',
    [ testImageIn,
      '-quality', '93',
      '-thumbnail', '231>',
      '-unsharp', '1.5x1+0.7+0.02',
      '-gravity', 'North',
      '-extent', '231',
      '-crop', 'x182+0+0',
      testImageOut ]);

easyImagePromise
    .then(() => {
    },
    err => {
        console.log('Error converting ' + testImageIn + ' Error: ' + err);
    });

return deferredImage.promise;

@mrkmg What I don't really understand is why you guys implemented it this way or why even deprecate the old way in the first place. The old way worked just fine. It seems to me sending options should be supported better, perhaps more like an element array. This would have been more intuitive:

const easyImagePromise = easyimage.executue(
{
   action: convert,
   src: testImageIn,
   dst: testImageOut,
   quality, 93,
   thumbnail, 231>,
   unsharp: 1.5x1+0.7+0.02,
   gravity: North,
   extent: 231,
   crop: x182+0+0,
});

or if mapping the options to pass into the command line was too much of a hassle, put all the options into a string, like options: '-quality 93, thumnbail 234, etc.

[alanosman]

On other point of feedback. I think the unshift error is completely misleading. Perhaps better error message - like 'invalid option parameters provided` would be more helpful to the wayward coder. Thanks again.

[mrkmg]

I completely agree on the documentation. That has always been my weak-point. I will strive to make it better.

As for why I deprecated the old method. it was no more than just child_process.spawn. Like, literally it just called that. In order to get compatibility with IM6 and IM7 on systems where the convert and identify command where behind the magick command, I needed a way to segregate the command from the arguments, and string parsing of cli options seemed silly in this case.

Another reason for switching from a single string of options to an array of options was to deal with escaping of the options. It can be quite difficult in some cases to get the escaping right when dealing with a complex set of options. This makes it so you do not have to worry so much of escaping.

I really dig the idea of passing in params of options in an object form, and actually gm seems to be similar in idea to that (except its procedural instead of parametric).