From 92717192bf8f871de0b5ea011f3643c100e665c0 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 24 Feb 2025 10:50:31 -0500 Subject: [PATCH] feat(model): make bulkWrite `results` include MongoDB bulk write errors as well as validation errors Fix #15265 --- lib/model.js | 29 ++++++++---------- test/model.test.js | 76 ++++++++++++++++++++++++++++++++++++++++++++++ types/models.d.ts | 2 +- 3 files changed, 89 insertions(+), 18 deletions(-) diff --git a/lib/model.js b/lib/model.js index e153a0450a..7536609e4f 100644 --- a/lib/model.js +++ b/lib/model.js @@ -3104,11 +3104,9 @@ Model.$__insertMany = function(arr, options, callback) { const res = { acknowledged: true, insertedCount: 0, - insertedIds: {}, - mongoose: { - validationErrors: validationErrors - } + insertedIds: {} }; + decorateBulkWriteResult(res, validationErrors, validationErrors); return callback(null, res); } callback(null, []); @@ -3161,10 +3159,7 @@ Model.$__insertMany = function(arr, options, callback) { // Decorate with mongoose validation errors in case of unordered, // because then still do `insertMany()` - res.mongoose = { - validationErrors: validationErrors, - results: results - }; + decorateBulkWriteResult(res, validationErrors, results); } return callback(null, res); } @@ -3198,10 +3193,7 @@ Model.$__insertMany = function(arr, options, callback) { if (error.writeErrors != null) { for (let i = 0; i < error.writeErrors.length; ++i) { const originalIndex = validDocIndexToOriginalIndex.get(error.writeErrors[i].index); - error.writeErrors[i] = { - ...error.writeErrors[i], - index: originalIndex - }; + error.writeErrors[i] = { ...error.writeErrors[i], index: originalIndex }; if (!ordered) { results[originalIndex] = error.writeErrors[i]; } @@ -3245,10 +3237,7 @@ Model.$__insertMany = function(arr, options, callback) { }); if (rawResult && ordered === false) { - error.mongoose = { - validationErrors: validationErrors, - results: results - }; + decorateBulkWriteResult(error, validationErrors, results); } callback(error, null); @@ -3486,8 +3475,14 @@ Model.bulkWrite = async function bulkWrite(ops, options) { then(res => ([res, null])). catch(error => ([null, error])); + const writeErrorsByIndex = {}; + if (error?.writeErrors) { + for (const writeError of error.writeErrors) { + writeErrorsByIndex[writeError.err.index] = writeError; + } + } for (let i = 0; i < validOpIndexes.length; ++i) { - results[validOpIndexes[i]] = null; + results[validOpIndexes[i]] = writeErrorsByIndex[i] ?? null; } if (error) { if (validationErrors.length > 0) { diff --git a/test/model.test.js b/test/model.test.js index ceef2073b6..d4aa35ec68 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -6,6 +6,7 @@ const sinon = require('sinon'); const start = require('./common'); +const CastError = require('../lib/error/cast'); const assert = require('assert'); const { once } = require('events'); const random = require('./util').random; @@ -4707,6 +4708,46 @@ describe('Model', function() { assert.equal(err.validationErrors[0].path, 'age'); assert.equal(err.results[0].path, 'age'); }); + + it('bulkWrite should return both write errors and validation errors in error.results (gh-15265)', async function() { + const userSchema = new Schema({ _id: Number, age: { type: Number } }); + const User = db.model('User', userSchema); + + const createdUser = await User.create({ _id: 1, name: 'Test' }); + + const err = await User.bulkWrite([ + { + updateOne: { + filter: { _id: createdUser._id }, + update: { $set: { age: 'NaN' } } + } + }, + { + insertOne: { + document: { _id: 3, age: 14 } + } + }, + { + insertOne: { + document: { _id: 1, age: 13 } + } + }, + { + insertOne: { + document: { _id: 1, age: 14 } + } + } + ], { ordered: false, throwOnValidationError: true }) + .then(() => null) + .catch(err => err); + + assert.ok(err); + assert.strictEqual(err.mongoose.results.length, 4); + assert.ok(err.mongoose.results[0] instanceof CastError); + assert.strictEqual(err.mongoose.results[1], null); + assert.equal(err.mongoose.results[2].constructor.name, 'WriteError'); + assert.equal(err.mongoose.results[3].constructor.name, 'WriteError'); + }); }); it('deleteOne with cast error (gh-5323)', async function() { @@ -7060,6 +7101,41 @@ describe('Model', function() { assert.deepStrictEqual(docs.map(doc => doc.age), [12, 12]); }); + it('insertMany should return both write errors and validation errors in error.results (gh-15265)', async function() { + const userSchema = new Schema({ _id: Number, age: { type: Number } }); + const User = db.model('User', userSchema); + await User.insertOne({ _id: 1, age: 12 }); + + const err = await User.insertMany([ + { _id: 1, age: 'NaN' }, + { _id: 3, age: 14 }, + { _id: 1, age: 13 }, + { _id: 1, age: 14 } + ], { ordered: false }).then(() => null).catch(err => err); + + assert.ok(err); + assert.strictEqual(err.results.length, 4); + assert.ok(err.results[0] instanceof ValidationError); + assert.ok(err.results[1] instanceof User); + assert.ok(err.results[2].err); + assert.ok(err.results[3].err); + }); + + it('insertMany should return both write errors and validation errors in error.results with rawResult (gh-15265)', async function() { + const userSchema = new Schema({ _id: Number, age: { type: Number } }); + const User = db.model('User', userSchema); + + const res = await User.insertMany([ + { _id: 1, age: 'NaN' }, + { _id: 3, age: 14 } + ], { ordered: false, rawResult: true }); + + assert.ok(res); + assert.strictEqual(res.mongoose.results.length, 2); + assert.ok(res.mongoose.results[0] instanceof ValidationError); + assert.ok(res.mongoose.results[1] instanceof User); + }); + it('returns writeResult on success', async() => { const userSchema = new Schema({ diff --git a/types/models.d.ts b/types/models.d.ts index 88cedbaa34..4ff5fe83ec 100644 --- a/types/models.d.ts +++ b/types/models.d.ts @@ -308,7 +308,7 @@ declare module 'mongoose' { bulkWrite( writes: Array>, options: MongooseBulkWriteOptions & { ordered: false } - ): Promise } }>; + ): Promise } }>; bulkWrite( writes: Array>, options?: MongooseBulkWriteOptions