Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(model): add throwOnValidationError option for opting into getting MongooseBulkWriteError if all valid operations succeed in bulkWrite() and insertMany() #14599

Merged
merged 1 commit into from
Jun 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions lib/error/bulkWriteError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*!
* Module dependencies.
*/

'use strict';

const MongooseError = require('./');


/**
* If `bulkWrite()` or `insertMany()` has validation errors, but
* all valid operations succeed, and 'throwOnValidationError' is true,
* Mongoose will throw this error.
*
* @api private
*/

class MongooseBulkWriteError extends MongooseError {
constructor(validationErrors, results, rawResult, operation) {
let preview = validationErrors.map(e => e.message).join(', ');
if (preview.length > 200) {
preview = preview.slice(0, 200) + '...';
}
super(`${operation} failed with ${validationErrors.length} Mongoose validation errors: ${preview}`);

this.validationErrors = validationErrors;
this.results = results;
this.rawResult = rawResult;
this.operation = operation;
}
}

Object.defineProperty(MongooseBulkWriteError.prototype, 'name', {
value: 'MongooseBulkWriteError'
});

/*!
* exports
*/

module.exports = MongooseBulkWriteError;
48 changes: 48 additions & 0 deletions lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const Document = require('./document');
const DocumentNotFoundError = require('./error/notFound');
const DivergentArrayError = require('./error/divergentArray');
const EventEmitter = require('events').EventEmitter;
const MongooseBulkWriteError = require('./error/bulkWriteError');
const MongooseBuffer = require('./types/buffer');
const MongooseError = require('./error/index');
const OverwriteModelError = require('./error/overwriteModel');
Expand Down Expand Up @@ -3375,6 +3376,7 @@ Model.startSession = function() {
* @param {Boolean} [options.lean=false] if `true`, skips hydrating the documents. This means Mongoose will **not** cast or validate any of the documents passed to `insertMany()`. This option is useful if you need the extra performance, but comes with data integrity risk. Consider using with [`castObject()`](#model_Model-castObject).
* @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.
* @param {Function} [callback] callback
* @return {Promise} resolving to the raw result from the MongoDB driver if `options.rawResult` was `true`, or the documents that passed validation, otherwise
* @api public
Expand Down Expand Up @@ -3419,6 +3421,7 @@ Model.$__insertMany = function(arr, options, callback) {
const limit = options.limit || 1000;
const rawResult = !!options.rawResult;
const ordered = typeof options.ordered === 'boolean' ? options.ordered : true;
const throwOnValidationError = typeof options.throwOnValidationError === 'boolean' ? options.throwOnValidationError : false;
const lean = !!options.lean;

if (!Array.isArray(arr)) {
Expand Down Expand Up @@ -3496,6 +3499,14 @@ Model.$__insertMany = function(arr, options, callback) {

// Quickly escape while there aren't any valid docAttributes
if (docAttributes.length === 0) {
if (throwOnValidationError) {
return callback(new MongooseBulkWriteError(
validationErrors,
results,
null,
'insertMany'
));
}
if (rawResult) {
const res = {
acknowledged: true,
Expand Down Expand Up @@ -3598,6 +3609,20 @@ Model.$__insertMany = function(arr, options, callback) {
}
}

if (ordered === false && throwOnValidationError && validationErrors.length > 0) {
for (let i = 0; i < results.length; ++i) {
if (results[i] === void 0) {
results[i] = docs[i];
}
}
return callback(new MongooseBulkWriteError(
validationErrors,
results,
res,
'insertMany'
));
}

if (rawResult) {
if (ordered === false) {
for (let i = 0; i < results.length; ++i) {
Expand Down Expand Up @@ -3728,6 +3753,7 @@ function _setIsNew(doc, val) {
* @param {Boolean} [options.skipValidation=false] Set to true to skip Mongoose schema validation on bulk write operations. Mongoose currently runs validation on `insertOne` and `replaceOne` operations by default.
* @param {Boolean} [options.bypassDocumentValidation=false] If true, disable [MongoDB server-side schema validation](https://www.mongodb.com/docs/manual/core/schema-validation/) for all writes in this bulk.
* @param {Boolean} [options.strict=null] Overwrites the [`strict` option](/docs/guide.html#strict) on schema. If false, allows filtering and writing fields not defined in the schema for all writes in this bulk.
* @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.
* @param {Function} [callback] callback `function(error, bulkWriteOpResult) {}`
* @return {Promise} resolves to a [`BulkWriteOpResult`](https://mongodb.github.io/node-mongodb-native/4.9/classes/BulkWriteResult.html) if the operation succeeds
* @api public
Expand Down Expand Up @@ -3777,6 +3803,7 @@ Model.bulkWrite = function(ops, options, callback) {
let remaining = validations.length;
let validOps = [];
let validationErrors = [];
const results = [];
if (remaining === 0) {
completeUnorderedValidation.call(this);
} else {
Expand All @@ -3786,6 +3813,7 @@ Model.bulkWrite = function(ops, options, callback) {
validOps.push(i);
} else {
validationErrors.push({ index: i, error: err });
results[i] = err;
}
if (--remaining <= 0) {
completeUnorderedValidation.call(this);
Expand All @@ -3799,13 +3827,25 @@ Model.bulkWrite = function(ops, options, callback) {
map(v => v.error);

function completeUnorderedValidation() {
const validOpIndexes = validOps;
validOps = validOps.sort().map(index => ops[index]);

if (validOps.length === 0) {
if ('throwOnValidationError' in options && options.throwOnValidationError && validationErrors.length > 0) {
return cb(new MongooseBulkWriteError(
validationErrors.map(err => err.error),
results,
getDefaultBulkwriteResult(),
'bulkWrite'
));
}
return cb(null, getDefaultBulkwriteResult());
}

this.$__collection.bulkWrite(validOps, options, (error, res) => {
for (let i = 0; i < validOpIndexes.length; ++i) {
results[validOpIndexes[i]] = null;
}
if (error) {
if (validationErrors.length > 0) {
error.mongoose = error.mongoose || {};
Expand All @@ -3816,6 +3856,14 @@ Model.bulkWrite = function(ops, options, callback) {
}

if (validationErrors.length > 0) {
if ('throwOnValidationError' in options && options.throwOnValidationError) {
return cb(new MongooseBulkWriteError(
validationErrors,
results,
res,
'bulkWrite'
));
}
res.mongoose = res.mongoose || {};
res.mongoose.validationErrors = validationErrors;
}
Expand Down
104 changes: 104 additions & 0 deletions test/model.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6111,6 +6111,71 @@ describe('Model', function() {
const { num } = await Test.findById(_id);
assert.equal(num, 99);
});

it('bulkWrite should throw an error if there were operations that failed validation, ' +
'but all operations that passed validation succeeded (gh-13256)', async function() {
const userSchema = new Schema({ age: { type: Number } });
const User = db.model('User', userSchema);

const createdUser = await User.create({ name: 'Test' });

const err = await User.bulkWrite([
{
updateOne: {
filter: { _id: createdUser._id },
update: { $set: { age: 'NaN' } },
upsert: true
}
},
{
updateOne: {
filter: { _id: createdUser._id },
update: { $set: { age: 13 } },
upsert: true
}
},
{
updateOne: {
filter: { _id: createdUser._id },
update: { $set: { age: 12 } },
upsert: true
}
}
], { ordered: false, throwOnValidationError: true })
.then(() => null)
.catch(err => err);

assert.ok(err);
assert.equal(err.name, 'MongooseBulkWriteError');
assert.equal(err.validationErrors[0].path, 'age');
assert.equal(err.results[0].path, 'age');
});

it('throwOnValidationError (gh-14572) (gh-13256)', async function() {
const schema = new Schema({
num: Number
});

const M = db.model('Test', schema);

const ops = [
{
insertOne: {
document: {
num: 'not a number'
}
}
}
];

const err = await M.bulkWrite(
ops,
{ ordered: false, throwOnValidationError: true }
).then(() => null, err => err);
assert.ok(err);
assert.equal(err.name, 'MongooseBulkWriteError');
assert.equal(err.validationErrors[0].errors['num'].name, 'CastError');
});
});

it('insertMany with Decimal (gh-5190)', async function() {
Expand Down Expand Up @@ -9028,6 +9093,45 @@ describe('Model', function() {
assert.equal(TestModel.staticFn(), 'Returned from staticFn');
});
});

it('insertMany should throw an error if there were operations that failed validation, ' +
'but all operations that passed validation succeeded (gh-13256)', async function() {
const userSchema = new Schema({
age: { type: Number }
});

const User = db.model('User', userSchema);

let err = await User.insertMany([
new User({ age: 12 }),
new User({ age: 12 }),
new User({ age: 'NaN' })
], { ordered: false, throwOnValidationError: true })
.then(() => null)
.catch(err => err);

assert.ok(err);
assert.equal(err.name, 'MongooseBulkWriteError');
assert.equal(err.validationErrors[0].errors['age'].name, 'CastError');
assert.ok(err.results[2] instanceof Error);
assert.equal(err.results[2].errors['age'].name, 'CastError');

let docs = await User.find();
assert.deepStrictEqual(docs.map(doc => doc.age), [12, 12]);

err = await User.insertMany([
new User({ age: 'NaN' })
], { ordered: false, throwOnValidationError: true })
.then(() => null)
.catch(err => err);

assert.ok(err);
assert.equal(err.name, 'MongooseBulkWriteError');
assert.equal(err.validationErrors[0].errors['age'].name, 'CastError');

docs = await User.find();
assert.deepStrictEqual(docs.map(doc => doc.age), [12, 12]);
});
});


Expand Down
2 changes: 2 additions & 0 deletions types/models.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ declare module 'mongoose' {
skipValidation?: boolean;
strict?: boolean;
timestamps?: boolean | 'throw';
throwOnValidationError?: boolean;
}

interface InsertManyOptions extends
Expand All @@ -28,6 +29,7 @@ declare module 'mongoose' {
rawResult?: boolean;
ordered?: boolean;
lean?: boolean;
throwOnValidationError?: boolean;
}

type InsertManyResult<T> = mongodb.InsertManyResult<T> & {
Expand Down
Loading