Unclear errors #14

Closed
paulmillr opened this Issue Mar 30, 2012 · 2 comments

Projects

None yet

4 participants

@paulmillr

ncp 'a', 'b', (error) => console.log 'Error is', error
# => Error is []

a doesn't exist in this case.

@jprichardson

Agreed. Passing an array of errors is poor Node.js convention. Anyway you would modify it to have the callback fire with the first encountered error?

@spion
Contributor
spion commented Mar 4, 2013

The error is on line 198 in ncp.js in the onError handler

if (options.stopOnError) {
  return callback(err);
}
else if (!errs && options.errs) {
  errs = fs.createWriteStream(options.errs);
}
else if (!errs) {
  errs = [];
}
else if (options.errs) { // this is the problem
  if (typeof errs.write === 'undefined') {
    errs.push(err);
  }
  else { 
    errs.write(err.stack + '\n\n');
  }
}
return cb();

This logic doesn't work properly and the options used there are undocumented. However, its clear that

else if (options.errs) { 

is incorrect - it means that if an errs option is not provided, no errors will ever be pushed into the errs array.

To fix this, line 198 should be simply removed, along with its closing brace. Its not necessary to check for anything there - the errs array (or stream) is already defined and errors can be pushed (or written) to it.

@spion spion added a commit to doxout/ncp that referenced this issue Mar 4, 2013
@spion spion fixes #14 (unclear errors) 553da06
@AvianFlu AvianFlu closed this in acaa15b Mar 12, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment