adding better mongoose conflict error handling

[ekryski]

Summary

This adds better support for mongodb duplicate key errors. It uses regular expression to parse out the offending key and value and creates a better error message out of it.

Other Information

Related to this Automattic/mongoose#2129

[corymsmith]

One thing to note is that this may break other peoples error handling code if they are doing similar parsing currently in their own API / client. I think this is the right way to go though!

[ekryski]

@corymsmith Yeah, you are right. This likely will need to be a major version release in case people were relying on the error message.

[corymsmith]

[ekryski]

published as v5.0.0

[idibidiart]

@ekryski

Good call.

But can you please clarify to me the difference between this being a good idea and checking for undefined being passed to $push potentially not being a good idea? I would take @daffl view without any hesitation but I'm trying to understand what the role of something like Feathers-Mongoose is. Is it supposed to take care of ALL input that would cause the ODM/ORM to throw an error or ONLY input that is part of the supported query language as @daffl suggested? A little confused as to the seemingly arbitrary distinction.

Ref:
#167 (comment)

[corymsmith]

@idibidiart This isn't doing any validation on inputs, this is merely formatting the error returned from MongoDB

[ekryski]

Ya. It's a little grey for sure. I started implementing my own error hook to do that change but figured it was better to have that in the core module rather than have everyone need to repeat this in every app that uses mongoose (this should also probably happen in feathers-mongodb because it's an issue with mongo).

It also was more of a security concern as pointed out in #166. Honestly, I would have liked to see this fixed in mongodb or mongoose but since that is very unlikely to happen and that it was leaking DB info, I decided to fix this. It also makes it much easier to customize the error message further in an error hook because now you have the actual offending key/value.

[idibidiart]

Oh I see @corymsmith I misunderstood the context.

I updated the other issue with my guess as to the rationale in that other case... trying to understand the development philosophy by picking on a random sampling of issues may not be the best approach but it's pertinent to my learning process ... Thanks for all the great work btw guys

cc: @ekryski