TritonDataCenter / node-workflow

Task orchestration, creation and running using NodeJS
MIT License
456 stars 66 forks source link

node-workflow does not include the Error JavaScript builtin constructor in tasks' sandboxes #156

Closed misterdjules closed 8 years ago

misterdjules commented 8 years ago

Any workflow task that creates an instance of Error currently uses a different Error constructor than the one used by modules that are injected in the sandbox.

For instance, if a task uses the module Foo which exports the following assertIsError function:

modules.exports = {
  assertIsError: function assertIsError(someError) { assert(someError instanceof Error); }
}

then a task running the following code:

var err = new Error('some error');
Foo.assertIsError(err);

would assert.

For a real-world use case in Triton, this caused tasks run by sdc-workflow to not be able to call the callback of a vasync parallel/forEachParallel async workflow with an error object. Indeed, the Error instance created from the task would be passed to verror's MultiError constructor, which would in turn check if that instance is an instanceof Error. It would determine it's not, since the workflow task and the vasync (and thus verror) module would use a different Error constructor. Finally, that would make verror use extsprintf.sprintf on an object rather than a string, which would make it assert.

In that specific case, upgrading sdc-workflow's node-workflow (or wf) dependency to the tip of joyent/node-workflow#master fixes the symptom, because that version upgrades verror to a version that doesn't use instanceof Error in the same way, but the original problem is still there, and still causes some problems e.g when formatting the error message.

misterdjules commented 8 years ago

Fixed by cac32eec0fa906adeba683f7f9436a9247740f11.