haraka / Haraka

A fast, highly extensible, and event driven SMTP server
https://haraka.github.io
MIT License
5.02k stars 662 forks source link

Async waterfall throws array error #992

Closed spathon closed 9 years ago

spathon commented 9 years ago

When using async.waterfall in exports.hook_queue we get Error: First argument to waterfall must be an array of functions

exports.hook_queue = function (next, conn) {
  var x = [
    function(callback) {
        callback(null, 'one', 'two');
    },
    function(arg1, arg2, callback) {
      // arg1 now equals 'one' and arg2 now equals 'two'
        callback(null, 'three');
    },
    function(arg1, callback) {
        // arg1 now equals 'three'
        callback(null, 'done');
    }
  ];

  console.log(x.constructor !== Array); // returns false

  async.waterfall(x, function (err, result) {
      console.log(err);
  });
};

For some reason inside async.waterfall tasks.constructor !== Array returns true

if (tasks.constructor !== Array) {
  var err = new Error('First argument to waterfall must be an array of functions');
  return callback(err);
}

screen shot 2015-05-29 at 14 32 10

async.waterfall works in a regular express app on the same server and we have tried with async (0.9.0 and 0.9.2) and node (0.10.38 and 0.12.4)

Is haraka somehow causing this?

smfreegard commented 9 years ago

It could be; Haraka plugins run in a vm sandbox and that might be causing the problem there.

I think someone else ran into this yesterday; if you upgrade the version of async that Haraka uses to 1.0.0, then it should work as I've just checked the async code and it now uses the more sensible Array.isArray(...) instead of trying to check the constructor.

spathon commented 9 years ago

Thanks for the fast reply it helped us.

So Haraka is basically merging my node_modules folder which means that I always have to use the same versions of the packages Haraka is using?

Are you planning to update async to version 1 and release a new version of Haraka soon?

smfreegard commented 9 years ago

So Haraka is basically merging my node_modules folder which means that I always have to use the same versions of the packages Haraka is using?

Huh? No - each node module has it's own node_modules directory, so it should be totally independent of your app.

Are you planning to update async to version 1 and release a new version of Haraka soon?

We don't have any plans for a release at the moment. But when we do, then I'll make sure that async is @1.0.0

spathon commented 9 years ago

Apparently not because we had async ~0.9.0 specified in our package json and the async in our node_modules folder is 0.9.2 but when calling async in our plugin it's using async 0.2.9 from Haraka (latest on npm 2.6.1). The array issue in async was fixed in version 0.3.0. Do you know why Haraka is using it's own async then?

smfreegard commented 9 years ago

It should depend on where your plugin is installed e.g. if you run haraka -i /etc/haraka, then your local node_modules folder for the plugins would be /etc/haraka/node_modules and any plugins in /etc/haraka/plugins would first use /etc/haraka/node_modules before using the versions supplied with Haraka.

See https://github.com/baudehlo/Haraka/blob/master/haraka.js#L8-L18

spathon commented 9 years ago

For some reason it don't it just loads the modules that haraka don't use screen shot 2015-05-29 at 20 32 43 and process.env.HARAKA is returning /var/www/haraka where the installation is

Writing the full path from the plugin works var async = require('../../node_modules/async'); but var async = require('async'); uses harakas

msimerson commented 9 years ago

The latest Haraka in baudehlo/master specifies async 0.9.0, which I recently updated. The latest release of Haraka still has 0.2.9. If you installed Haraka via npm, you'll still get the old version of async.

spathon commented 9 years ago

Yeah I noticed that but I guess this might be an issue for others in the future if any one tries to use a newer module than Haraka is using. Right now I will just use var async = require('../../node_modules/async'); every time where I need async in my plugin

msimerson commented 9 years ago

this might be an issue for others in the future

Since async is updated in master, It won't be an issue after we release the next version of Haraka.