Calling a callback with an error that is an array is unexpected

[simonexmachina]

I've just spent several hours debugging an issue which just provided an error message of undefined. I've ultimately tracked it down to an error originating from ncp calling the callback with an err that is an array. Unfortunately this error is then propagated to far away, to code which expects errors to be Error objects.

May I suggest that a better way to handle this would be an Error object with an array of paths in it?

[simonexmachina]

Strange, I received an email response to this, but it's not showing here.

Hm, under Node 0.11 I get an array with one Error ([Error]) and under Node 0.10 I get only Error. Possible bug? It happens when I try to create a file under another one (e.g. LICENSE/file) when license is a markdown file from github repo.

Yeah that's super strange. It should definitely always return either one or the other. My recollection was that the code would always return an array of errors, but I don't have it in front of me now.

[grabbou]

Yep, because I've noticed that it's not related to that library. fs-extra does the same (might be Node related problem).

[fresheneesz]

I've been seeing effects of this for months, and i had no idea what was going on. Since I was ignoring errors anyway, I didn't spend the time to look into it, but from time to time I added some extra info to my script to help me look into it whenever it popped up again.

I finally figured out this was coming from ncp. It is super terrible to have your callback receive errors in two different ways depending on your configuratin. Its also terrible (but less super) to call the callback with an error array (as stated by the OP). This is not the node way, and this module shouldn't be doing it. There isn't even any reason to do it that way: if you have stoponerr off, the callback will be called multiple times anyway - why not just call it with each error individually?

Also, if you pass one in, there is absolutely no reason to call the error stream with an array of errors.

Please fix the interface so that only single errors are passed to the callback and error stream. Or if you're too lazy, please make it super clear in the docs that this is how things work.

[grabbou]

+1 @fresheneesz . Actually, on Heroku, I am getting single objects and on Mac, I receive an array. My simple fix is to basically do err = err[0] || err to make it super clear, but actually, it's quite hard to write unit tests for my method using that library as depending on configuration, I receive different error messages for exactly the same thing!

[AvianFlu]

Three years ago, that array thing seemed like a great idea. It's definitely not worth the trouble, though, so I'm going to drop it.

I plan to review a bunch of PRs today, with an eye on releasing a 1.0 version. I'll include this change in that.

[grabbou]

Thanks @AvianFlu, I am developing a library where your package is quite important dependency. I'd love to see a fixed version, have you got an idea why I may receive different errors depending on OS I am? I suppose it's due to different OS implementations of filesystem operations.

If you need any help - you can count me in (especially in terms of bugs)

[fresheneesz]

Thanks Avian!

[davidgatti]

This one is bit old :D but I'm using version 2.0.0, and I still see an array, is it now on purpose or you guys forgot to change this?