Skip to content

Commit 0759a0e

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 dbf2501 commit 0759a0e

File tree

3 files changed

+45
-24
lines changed

3 files changed

+45
-24
lines changed

lib/bulk/common.js

+3-10
Original file line numberDiff line numberDiff line change
@@ -1206,15 +1206,6 @@ class BulkOperationBase {
12061206
*/
12071207
handleWriteError(callback, writeResult) {
12081208
if (this.s.bulkResult.writeErrors.length > 0) {
1209-
if (this.s.bulkResult.writeErrors.length === 1) {
1210-
handleCallback(
1211-
callback,
1212-
new BulkWriteError(toError(this.s.bulkResult.writeErrors[0]), writeResult),
1213-
null
1214-
);
1215-
return true;
1216-
}
1217-
12181209
const msg = this.s.bulkResult.writeErrors[0].errmsg
12191210
? this.s.bulkResult.writeErrors[0].errmsg
12201211
: 'write operation failed';
@@ -1232,7 +1223,9 @@ class BulkOperationBase {
12321223
null
12331224
);
12341225
return true;
1235-
} else if (writeResult.getWriteConcernError()) {
1226+
}
1227+
1228+
if (writeResult.getWriteConcernError()) {
12361229
handleCallback(
12371230
callback,
12381231
new BulkWriteError(toError(writeResult.getWriteConcernError()), writeResult),

lib/error.js

+12-2
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,12 @@ class MongoError extends Error {
4141
super(message);
4242
} else {
4343
super(message.message || message.errmsg || message.$err || 'n/a');
44+
if (message.errorLabels) {
45+
this[kErrorLabels] = new Set(message.errorLabels);
46+
}
47+
4448
for (let name in message) {
45-
if (name === 'errorLabels') {
46-
this[kErrorLabels] = new Set(message[name]);
49+
if (name === 'errorLabels' || name === 'errmsg') {
4750
continue;
4851
}
4952

@@ -57,6 +60,13 @@ class MongoError extends Error {
5760
this.name = 'MongoError';
5861
}
5962

63+
/**
64+
* Legacy name for server error responses
65+
*/
66+
get errmsg() {
67+
return this.message;
68+
}
69+
6070
/**
6171
* Creates a new MongoError object
6272
*

test/functional/bulk.test.js

+30-12
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)