From cfc43c375f9ebb585c35418a065f839a057753e3 Mon Sep 17 00:00:00 2001 From: Sam Pal Date: Tue, 4 Jun 2019 16:38:23 -0400 Subject: [PATCH 1/4] fix(Bulk): change BulkWriteError message to first item from writeErrors Fixes NODE-1965 --- lib/bulk/common.js | 2 +- test/functional/bulk_tests.js | 63 +++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/lib/bulk/common.js b/lib/bulk/common.js index 884fdf26a22..80e2cc8d7af 100644 --- a/lib/bulk/common.js +++ b/lib/bulk/common.js @@ -1119,7 +1119,7 @@ class BulkOperationBase { callback, new BulkWriteError( toError({ - message: 'write operation failed', + message: this.s.bulkResult.writeErrors[0].errmsg, code: this.s.bulkResult.writeErrors[0].code, writeErrors: this.s.bulkResult.writeErrors }), diff --git a/test/functional/bulk_tests.js b/test/functional/bulk_tests.js index 2362f141c83..e9d2f03cdd8 100644 --- a/test/functional/bulk_tests.js +++ b/test/functional/bulk_tests.js @@ -937,6 +937,69 @@ describe('Bulk', function() { } }); + it( + 'should provide descriptive error message for Unordered Batch with duplicate key errors on updates', + { + metadata: { requires: { topology: ['single', 'replicaset', 'ssl', 'heap', 'wiredtiger'] } }, + + test: function(done) { + const configuration = this.configuration; + var client = configuration.newClient(configuration.writeConcernMax(), { + poolSize: 1 + }); + + client.connect(function(err, client) { + var db = client.db(configuration.db); + var col = db.collection('err_batch_write_unordered_ops_legacy_6'); + + // Write concern + var writeConcern = configuration.writeConcernMax(); + writeConcern.unique = true; + writeConcern.sparse = false; + + // Add unique index on b field causing all updates to fail + col.ensureIndex({ b: 1 }, writeConcern, function(err) { + expect(err).to.not.exist; + + // Initialize the unordered Batch + var batch = col.initializeUnorderedBulkOp(); + + // Add some operations to be executed in order + batch.insert({ a: 1 }); + batch.find({ a: 1 }).update({ $set: { b: 1 } }); + batch.insert({ b: 1 }); + batch.insert({ b: 1 }); + batch.insert({ b: 1 }); + batch.insert({ b: 1 }); + + // Execute the operations + batch.execute(configuration.writeConcernMax(), function(err, result) { + expect(err).to.exist; + expect(result).to.not.exist; + + // Test basic settings + result = err.result; + expect(result.nInserted).to.equal(2); + expect(result.hasWriteErrors()).to.equal(true); + expect( + result.getWriteErrorCount() === 4 || result.getWriteErrorCount() === 3 + ).to.equal(true); + + // Individual error checking + var error = result.getWriteErrorAt(0); + expect(error.code === 11000 || error.code === 11001).to.equal(true); + expect(error.errmsg).to.exist; + expect(err.message).to.equal(error.errmsg); + + client.close(); + done(); + }); + }); + }); + } + } + ); + it( 'should Correctly Execute Unordered Batch of with upserts causing duplicate key errors on updates', { From daf11519946c7edfcf03a32c3cf056aaec2d833c Mon Sep 17 00:00:00 2001 From: Sam Pal Date: Thu, 6 Jun 2019 10:47:42 -0400 Subject: [PATCH 2/4] incorporate feedback --- test/functional/bulk_tests.js | 99 ++++++++++++++++------------------- 1 file changed, 45 insertions(+), 54 deletions(-) diff --git a/test/functional/bulk_tests.js b/test/functional/bulk_tests.js index e9d2f03cdd8..be323a856f0 100644 --- a/test/functional/bulk_tests.js +++ b/test/functional/bulk_tests.js @@ -937,68 +937,59 @@ describe('Bulk', function() { } }); - it( - 'should provide descriptive error message for Unordered Batch with duplicate key errors on updates', - { - metadata: { requires: { topology: ['single', 'replicaset', 'ssl', 'heap', 'wiredtiger'] } }, - - test: function(done) { - const configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { - poolSize: 1 - }); - - client.connect(function(err, client) { - var db = client.db(configuration.db); - var col = db.collection('err_batch_write_unordered_ops_legacy_6'); - - // Write concern - var writeConcern = configuration.writeConcernMax(); - writeConcern.unique = true; - writeConcern.sparse = false; + it('should provide descriptive error message for unordered batch with duplicate key errors on inserts', function(done) { + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { + poolSize: 1 + }); - // Add unique index on b field causing all updates to fail - col.ensureIndex({ b: 1 }, writeConcern, function(err) { - expect(err).to.not.exist; + client.connect(function(err, client) { + const db = client.db(configuration.db); + const col = db.collection('err_batch_write_unordered_ops_legacy_6'); + + // Add unique index on a field causing all inserts to fail + col.createIndexes( + [ + { + name: 'err_batch_write_unordered_ops_legacy_6', + key: { a: 1 }, + unique: true + } + ], + function(err) { + expect(err).to.not.exist; - // Initialize the unordered Batch - var batch = col.initializeUnorderedBulkOp(); + // Initialize the unordered Batch + const batch = col.initializeUnorderedBulkOp(); - // Add some operations to be executed in order - batch.insert({ a: 1 }); - batch.find({ a: 1 }).update({ $set: { b: 1 } }); - batch.insert({ b: 1 }); - batch.insert({ b: 1 }); - batch.insert({ b: 1 }); - batch.insert({ b: 1 }); + // Add some operations to be executed in order + batch.insert({ a: 1 }); + batch.insert({ a: 1 }); - // Execute the operations - batch.execute(configuration.writeConcernMax(), function(err, result) { - expect(err).to.exist; - expect(result).to.not.exist; + // Execute the operations + batch.execute(configuration.writeConcernMax(), function(err, result) { + expect(err).to.exist; + expect(result).to.not.exist; - // Test basic settings - result = err.result; - expect(result.nInserted).to.equal(2); - expect(result.hasWriteErrors()).to.equal(true); - expect( - result.getWriteErrorCount() === 4 || result.getWriteErrorCount() === 3 - ).to.equal(true); + // Test basic settings + result = err.result; + expect(result.nInserted).to.equal(1); + expect(result.hasWriteErrors()).to.equal(true); + expect(result.getWriteErrorCount() === 1).to.equal(true); - // Individual error checking - var error = result.getWriteErrorAt(0); - expect(error.code === 11000 || error.code === 11001).to.equal(true); - expect(error.errmsg).to.exist; - expect(err.message).to.equal(error.errmsg); + // Individual error checking + const error = result.getWriteErrorAt(0); + expect(error.code === 11000).to.equal(true); + expect(error.errmsg).to.exist; + expect(err.message).to.equal(error.errmsg); - client.close(); - done(); - }); + client.close(); + done(); }); - }); - } - } - ); + } + ); + }); + }); it( 'should Correctly Execute Unordered Batch of with upserts causing duplicate key errors on updates', From 1d0ba10fdf554a9b9355311c27465eef8c5e068e Mon Sep 17 00:00:00 2001 From: Sam Pal Date: Thu, 6 Jun 2019 11:19:11 -0400 Subject: [PATCH 3/4] incorporate more feedback --- lib/bulk/common.js | 6 +++++- test/functional/bulk_tests.js | 5 ++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/bulk/common.js b/lib/bulk/common.js index 80e2cc8d7af..8634ce0171e 100644 --- a/lib/bulk/common.js +++ b/lib/bulk/common.js @@ -1115,11 +1115,15 @@ class BulkOperationBase { return true; } + const msg = this.s.bulkResult.writeErrors[0].errmsg + ? this.s.bulkResult.writeErrors[0].errmsg + : 'write operation failed'; + handleCallback( callback, new BulkWriteError( toError({ - message: this.s.bulkResult.writeErrors[0].errmsg, + message: msg, code: this.s.bulkResult.writeErrors[0].code, writeErrors: this.s.bulkResult.writeErrors }), diff --git a/test/functional/bulk_tests.js b/test/functional/bulk_tests.js index be323a856f0..d2e394c4832 100644 --- a/test/functional/bulk_tests.js +++ b/test/functional/bulk_tests.js @@ -943,7 +943,7 @@ describe('Bulk', function() { poolSize: 1 }); - client.connect(function(err, client) { + client.connect((err, client) => { const db = client.db(configuration.db); const col = db.collection('err_batch_write_unordered_ops_legacy_6'); @@ -983,8 +983,7 @@ describe('Bulk', function() { expect(error.errmsg).to.exist; expect(err.message).to.equal(error.errmsg); - client.close(); - done(); + client.close(done); }); } ); From e6030c3d2c2c25da369c2b38ac29e90f8fc383f4 Mon Sep 17 00:00:00 2001 From: Sam Pal Date: Thu, 6 Jun 2019 11:32:12 -0400 Subject: [PATCH 4/4] update callback notation --- test/functional/bulk_tests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/bulk_tests.js b/test/functional/bulk_tests.js index d2e394c4832..31d6c280f03 100644 --- a/test/functional/bulk_tests.js +++ b/test/functional/bulk_tests.js @@ -956,7 +956,7 @@ describe('Bulk', function() { unique: true } ], - function(err) { + err => { expect(err).to.not.exist; // Initialize the unordered Batch @@ -967,7 +967,7 @@ describe('Bulk', function() { batch.insert({ a: 1 }); // Execute the operations - batch.execute(configuration.writeConcernMax(), function(err, result) { + batch.execute(configuration.writeConcernMax(), (err, result) => { expect(err).to.exist; expect(result).to.not.exist;