Skip to content

Commit

Permalink
fix: always include writeErrors on a BulkWriteError instance
Browse files Browse the repository at this point in the history
We special case situations where there is only one error returned
from a bulk operation, but that can be quite confusing for users of
the bulk API. Promoting the single error to the top-level error
message is reasonable, but in all cases we should use a consistent
shape, and return `writeErrors` as a field on the object

NODE-2625
  • Loading branch information
mbroadst committed May 27, 2020
1 parent 6cee96b commit 58b4f94
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 23 deletions.
15 changes: 4 additions & 11 deletions lib/bulk/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -1200,19 +1200,10 @@ class BulkOperationBase {
* @ignore
* @param {function} callback
* @param {BulkWriteResult} writeResult
* @param {class} self either OrderedBulkOperation or UnorderdBulkOperation
* @param {class} self either OrderedBulkOperation or UnorderedBulkOperation
*/
handleWriteError(callback, writeResult) {
if (this.s.bulkResult.writeErrors.length > 0) {
if (this.s.bulkResult.writeErrors.length === 1) {
handleCallback(
callback,
new BulkWriteError(toError(this.s.bulkResult.writeErrors[0]), writeResult),
null
);
return true;
}

const msg = this.s.bulkResult.writeErrors[0].errmsg
? this.s.bulkResult.writeErrors[0].errmsg
: 'write operation failed';
Expand All @@ -1230,7 +1221,9 @@ class BulkOperationBase {
null
);
return true;
} else if (writeResult.getWriteConcernError()) {
}

if (writeResult.getWriteConcernError()) {
handleCallback(
callback,
new BulkWriteError(toError(writeResult.getWriteConcernError()), writeResult),
Expand Down
11 changes: 11 additions & 0 deletions lib/core/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ class MongoError extends Error {
} else {
super(message.message || message.errmsg || message.$err || 'n/a');
for (var name in message) {
if (name === 'errmsg') {
continue;
}

this[name] = message[name];
}
}
Expand All @@ -29,6 +33,13 @@ class MongoError extends Error {
this.name = 'MongoError';
}

/**
* Legacy name for server error responses
*/
get errmsg() {
return this.message;
}

/**
* Creates a new MongoError object
*
Expand Down
42 changes: 30 additions & 12 deletions test/functional/bulk.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const test = require('./shared').assert,
expect = require('chai').expect;

const MongoError = require('../../index').MongoError;
const ignoreNsNotFound = require('./shared').ignoreNsNotFound;

describe('Bulk', function() {
before(function() {
Expand Down Expand Up @@ -1616,16 +1617,41 @@ describe('Bulk', function() {
.then(() => client.close());
});

it('should promote a single error to the top-level message, and preserve writeErrors', function() {
const client = this.configuration.newClient();
return client.connect().then(() => {
this.defer(() => client.close());

const coll = client.db().collection('single_bulk_write_error');
return coll
.drop()
.catch(ignoreNsNotFound)
.then(() => coll.insert(Array.from({ length: 4 }, (_, i) => ({ _id: i, a: i }))))
.then(() =>
coll.bulkWrite([{ insertOne: { _id: 5, a: 0 } }, { insertOne: { _id: 5, a: 0 } }])
)
.then(
() => {
throw new Error('expected a bulk error');
},
err => {
expect(err)
.property('message')
.to.match(/E11000/);
expect(err)
.to.have.property('writeErrors')
.with.length(1);
}
);
});
});

it('should preserve order of operation index in unordered bulkWrite', function() {
const client = this.configuration.newClient();
return client.connect().then(() => {
this.defer(() => client.close());

const coll = client.db().collection('bulk_write_ordering_test');
function ignoreNsNotFound(err) {
if (!err.message.match(/ns not found/)) throw err;
}

return coll
.drop()
.catch(ignoreNsNotFound)
Expand Down Expand Up @@ -1667,10 +1693,6 @@ describe('Bulk', function() {
this.defer(() => client.close());

const coll = client.db().collection('unordered_preserve_order');
function ignoreNsNotFound(err) {
if (!err.message.match(/ns not found/)) throw err;
}

return coll
.drop()
.catch(ignoreNsNotFound)
Expand Down Expand Up @@ -1704,10 +1726,6 @@ describe('Bulk', function() {
this.defer(() => client.close());

const coll = client.db().collection('bulk_op_ordering_test');
function ignoreNsNotFound(err) {
if (!err.message.match(/ns not found/)) throw err;
}

return coll
.drop()
.catch(ignoreNsNotFound)
Expand Down

0 comments on commit 58b4f94

Please sign in to comment.