fix: make MongoBulkWriteError conform to CRUD spec (#2621)

`BulkWriteError` has been renamed to `MongoBulkWriteError`, and the class has new properties that make it compliant with spec tests. NODE-1989
mongodb · Nov 19, 2020 · 7aa3567 · 7aa3567
1 parent bb1e081
commit 7aa3567
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 51 deletions.
diff --git a/src/bulk/common.ts b/src/bulk/common.ts
@@ -551,7 +551,9 @@ function executeCommands(
   function resultHandler(err?: AnyError, result?: Document) {
     // Error is a driver related error not a bulk op error, return early
     if (err && 'message' in err && !(err instanceof MongoWriteConcernError)) {
-      return callback(new BulkWriteError(err, new BulkWriteResult(bulkOperation.s.bulkResult)));
+      return callback(
+        new MongoBulkWriteError(err, new BulkWriteResult(bulkOperation.s.bulkResult))
+      );
     }
 
     if (err instanceof MongoWriteConcernError) {
@@ -651,7 +653,10 @@ function handleMongoWriteConcernError(
   );
 
   callback(
-    new BulkWriteError(new MongoError(wrappedWriteConcernError), new BulkWriteResult(bulkResult))
+    new MongoBulkWriteError(
+      new MongoError(wrappedWriteConcernError),
+      new BulkWriteResult(bulkResult)
+    )
   );
 }
 
@@ -660,16 +665,39 @@ function handleMongoWriteConcernError(
  * @public
  * @category Error
  */
-export class BulkWriteError extends MongoError {
-  result?: BulkWriteResult;
+export class MongoBulkWriteError extends MongoError {
+  result: BulkWriteResult;
 
-  /** Creates a new BulkWriteError */
-  constructor(error?: AnyError, result?: BulkWriteResult) {
+  /** Number of documents inserted. */
+  insertedCount: number;
+  /** Number of documents matched for update. */
+  matchedCount: number;
+  /** Number of documents modified. */
+  modifiedCount: number;
+  /** Number of documents deleted. */
+  deletedCount: number;
+  /** Number of documents upserted. */
+  upsertedCount: number;
+  /** Inserted document generated Id's, hash key is the index of the originating operation */
+  insertedIds: { [key: number]: ObjectId };
+  /** Upserted document generated Id's, hash key is the index of the originating operation */
+  upsertedIds: { [key: number]: ObjectId };
+
+  /** Creates a new MongoBulkWriteError */
+  constructor(error: AnyError, result: BulkWriteResult) {
     super(error as Error);
     Object.assign(this, error);
 
-    this.name = 'BulkWriteError';
+    this.name = 'MongoBulkWriteError';
     this.result = result;
+
+    this.insertedCount = result.insertedCount;
+    this.matchedCount = result.matchedCount;
+    this.modifiedCount = result.modifiedCount;
+    this.deletedCount = result.deletedCount;
+    this.upsertedCount = result.upsertedCount;
+    this.insertedIds = result.insertedIds;
+    this.upsertedIds = result.upsertedIds;
   }
 }
 
@@ -1214,7 +1242,7 @@ export abstract class BulkOperationBase {
         : 'write operation failed';
 
       callback(
-        new BulkWriteError(
+        new MongoBulkWriteError(
           new MongoError({
             message: msg,
             code: this.s.bulkResult.writeErrors[0].code,
@@ -1229,7 +1257,7 @@ export abstract class BulkOperationBase {
 
     const writeConcernError = writeResult.getWriteConcernError();
     if (writeConcernError) {
-      callback(new BulkWriteError(new MongoError(writeConcernError), writeResult));
+      callback(new MongoBulkWriteError(new MongoError(writeConcernError), writeResult));
       return true;
     }
   }

diff --git a/src/index.ts b/src/index.ts
@@ -57,11 +57,7 @@ export {
   MongoParseError,
   MongoWriteConcernError
 } from './error';
-export {
-  BulkWriteError as MongoBulkWriteError,
-  BulkWriteOptions,
-  AnyBulkWriteOperation
-} from './bulk/common';
+export { MongoBulkWriteError, BulkWriteOptions, AnyBulkWriteOperation } from './bulk/common';
 export {
   // Utils
   instrument,

diff --git a/test/functional/crud_spec.test.js b/test/functional/crud_spec.test.js
@@ -6,8 +6,6 @@ const chai = require('chai');
 const expect = chai.expect;
 chai.use(require('chai-subset'));
 
-const BulkWriteError = require('../../src/bulk/common').BulkWriteError;
-
 const TestRunnerContext = require('./spec-runner').TestRunnerContext;
 const gatherTestSuites = require('./spec-runner').gatherTestSuites;
 const generateTopologyTests = require('./spec-runner').generateTopologyTests;
@@ -111,36 +109,6 @@ describe('CRUD spec', function () {
     });
   });
 
-  function transformBulkWriteResult(result) {
-    const r = {};
-    r.insertedCount = result.nInserted;
-    r.matchedCount = result.nMatched;
-    r.modifiedCount = result.nModified || 0;
-    r.deletedCount = result.nRemoved;
-    r.upsertedCount = result.getUpsertedIds().length;
-    r.upsertedIds = {};
-    r.insertedIds = {};
-
-    // Update the n
-    r.n = r.insertedCount;
-
-    // Inserted documents
-    const inserted = result.getInsertedIds();
-    // Map inserted ids
-    for (let i = 0; i < inserted.length; i++) {
-      r.insertedIds[inserted[i].index] = inserted[i]._id;
-    }
-
-    // Upserted documents
-    const upserted = result.getUpsertedIds();
-    // Map upserted ids
-    for (let i = 0; i < upserted.length; i++) {
-      r.upsertedIds[upserted[i].index] = upserted[i]._id;
-    }
-
-    return r;
-  }
-
   function invert(promise) {
     return promise.then(
       () => {
@@ -152,11 +120,6 @@ describe('CRUD spec', function () {
 
   function assertWriteExpectations(collection, outcome) {
     return function (result) {
-      // TODO: when we fix our bulk write errors, get rid of this
-      if (result instanceof BulkWriteError) {
-        result = transformBulkWriteResult(result.result);
-      }
-
       Object.keys(outcome.result).forEach(resultName => {
         expect(result).to.have.property(resultName);
         if (resultName === 'upsertedId') {