Skip to content

Commit 58b4f94

Browse files
committed
fix: always include writeErrors on a BulkWriteError instance
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
1 parent 6cee96b commit 58b4f94

File tree

3 files changed

+45
-23
lines changed

3 files changed

+45
-23
lines changed

lib/bulk/common.js

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,19 +1200,10 @@ class BulkOperationBase {
12001200
* @ignore
12011201
* @param {function} callback
12021202
* @param {BulkWriteResult} writeResult
1203-
* @param {class} self either OrderedBulkOperation or UnorderdBulkOperation
1203+
* @param {class} self either OrderedBulkOperation or UnorderedBulkOperation
12041204
*/
12051205
handleWriteError(callback, writeResult) {
12061206
if (this.s.bulkResult.writeErrors.length > 0) {
1207-
if (this.s.bulkResult.writeErrors.length === 1) {
1208-
handleCallback(
1209-
callback,
1210-
new BulkWriteError(toError(this.s.bulkResult.writeErrors[0]), writeResult),
1211-
null
1212-
);
1213-
return true;
1214-
}
1215-
12161207
const msg = this.s.bulkResult.writeErrors[0].errmsg
12171208
? this.s.bulkResult.writeErrors[0].errmsg
12181209
: 'write operation failed';
@@ -1230,7 +1221,9 @@ class BulkOperationBase {
12301221
null
12311222
);
12321223
return true;
1233-
} else if (writeResult.getWriteConcernError()) {
1224+
}
1225+
1226+
if (writeResult.getWriteConcernError()) {
12341227
handleCallback(
12351228
callback,
12361229
new BulkWriteError(toError(writeResult.getWriteConcernError()), writeResult),

lib/core/error.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ class MongoError extends Error {
1919
} else {
2020
super(message.message || message.errmsg || message.$err || 'n/a');
2121
for (var name in message) {
22+
if (name === 'errmsg') {
23+
continue;
24+
}
25+
2226
this[name] = message[name];
2327
}
2428
}
@@ -29,6 +33,13 @@ class MongoError extends Error {
2933
this.name = 'MongoError';
3034
}
3135

36+
/**
37+
* Legacy name for server error responses
38+
*/
39+
get errmsg() {
40+
return this.message;
41+
}
42+
3243
/**
3344
* Creates a new MongoError object
3445
*

test/functional/bulk.test.js

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const test = require('./shared').assert,
44
expect = require('chai').expect;
55

66
const MongoError = require('../../index').MongoError;
7+
const ignoreNsNotFound = require('./shared').ignoreNsNotFound;
78

89
describe('Bulk', function() {
910
before(function() {
@@ -1616,16 +1617,41 @@ describe('Bulk', function() {
16161617
.then(() => client.close());
16171618
});
16181619

1620+
it('should promote a single error to the top-level message, and preserve writeErrors', function() {
1621+
const client = this.configuration.newClient();
1622+
return client.connect().then(() => {
1623+
this.defer(() => client.close());
1624+
1625+
const coll = client.db().collection('single_bulk_write_error');
1626+
return coll
1627+
.drop()
1628+
.catch(ignoreNsNotFound)
1629+
.then(() => coll.insert(Array.from({ length: 4 }, (_, i) => ({ _id: i, a: i }))))
1630+
.then(() =>
1631+
coll.bulkWrite([{ insertOne: { _id: 5, a: 0 } }, { insertOne: { _id: 5, a: 0 } }])
1632+
)
1633+
.then(
1634+
() => {
1635+
throw new Error('expected a bulk error');
1636+
},
1637+
err => {
1638+
expect(err)
1639+
.property('message')
1640+
.to.match(/E11000/);
1641+
expect(err)
1642+
.to.have.property('writeErrors')
1643+
.with.length(1);
1644+
}
1645+
);
1646+
});
1647+
});
1648+
16191649
it('should preserve order of operation index in unordered bulkWrite', function() {
16201650
const client = this.configuration.newClient();
16211651
return client.connect().then(() => {
16221652
this.defer(() => client.close());
16231653

16241654
const coll = client.db().collection('bulk_write_ordering_test');
1625-
function ignoreNsNotFound(err) {
1626-
if (!err.message.match(/ns not found/)) throw err;
1627-
}
1628-
16291655
return coll
16301656
.drop()
16311657
.catch(ignoreNsNotFound)
@@ -1667,10 +1693,6 @@ describe('Bulk', function() {
16671693
this.defer(() => client.close());
16681694

16691695
const coll = client.db().collection('unordered_preserve_order');
1670-
function ignoreNsNotFound(err) {
1671-
if (!err.message.match(/ns not found/)) throw err;
1672-
}
1673-
16741696
return coll
16751697
.drop()
16761698
.catch(ignoreNsNotFound)
@@ -1704,10 +1726,6 @@ describe('Bulk', function() {
17041726
this.defer(() => client.close());
17051727

17061728
const coll = client.db().collection('bulk_op_ordering_test');
1707-
function ignoreNsNotFound(err) {
1708-
if (!err.message.match(/ns not found/)) throw err;
1709-
}
1710-
17111729
return coll
17121730
.drop()
17131731
.catch(ignoreNsNotFound)

0 commit comments

Comments
 (0)