feat(model): add throwOnValidationError option for opting into getting MongooseBulkWriteError if all valid operations succeed in bulkWrite() and insertMany()
#13410
Conversation
…g MongooseBulkWriteError if all valid operations succeed in `bulkWrite()` and `insertMany()` Fix #13256
| @@ -2962,6 +2963,7 @@ Model.startSession = function() { | |||
| * @param {Boolean} [options.lean=false] if `true`, skips hydrating and validating the documents. This option is useful if you need the extra performance, but Mongoose won't validate the documents before inserting. | |||
| * @param {Number} [options.limit=null] this limits the number of documents being processed (validation/casting) by mongoose in parallel, this does **NOT** send the documents in batches to MongoDB. Use this option if you're processing a large number of documents and your app is running out of memory. | |||
| * @param {String|Object|Array} [options.populate=null] populates the result documents. This option is a no-op if `rawResult` is set. | |||
| * @param {Boolean} [options.throwOnValidationError=false] If true and `ordered: false`, throw an error if one of the operations failed validation, but all valid operations completed successfully. | |||
There was a problem hiding this comment.
I'd argue that this should default to true, since this is the intuitive behavior anyway, we can consider this as a bug rather than a missing feature. People should opt-out if they want backward-compatible behavior for some reason, then on v8.0 we can remove the option altogether.
What do you think?
There was a problem hiding this comment.
The problem is that we have tests that assert on the counterintuitive behavior, and for stability's sake I like to avoid changing existing tests outside of major releases.
There was a problem hiding this comment.
In that case I think it's good to merge as is, and remove the option in v8.0.
What do you think? Or would you rather to keep it as an option in v8, and maybe just change the default?
Fix #13256
Expands on #13258 .
Summary
#13256 is a bit of a strange edge case: currently, if you use
bulkWrite()orinsertMany()withordered: false, and there are validation errors but all valid ops succeed, you don't get an error. That's because we return the raw MongoDB server result, decorated with some extra Mongoose properties. This is very inconsistent, because you would expect to get an error if one of the operations failed validation.This PR adds a new
MongooseBulkWriteErrorclass that consolidates some of the patterns we like for reporting bulk write errors: namely, list ofvalidationErrorsand aresultsarray that helps you tell what happened with each individual operation or document you passed in.Examples