Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make MongoBulkWriteError conform to CRUD spec #2621

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 37 additions & 9 deletions src/bulk/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -651,7 +653,10 @@ function handleMongoWriteConcernError(
);

callback(
new BulkWriteError(new MongoError(wrappedWriteConcernError), new BulkWriteResult(bulkResult))
new MongoBulkWriteError(
new MongoError(wrappedWriteConcernError),
new BulkWriteResult(bulkResult)
)
);
}

Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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,
Expand All @@ -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;
}
}
Expand Down
6 changes: 1 addition & 5 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
37 changes: 0 additions & 37 deletions test/functional/crud_spec.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
() => {
Expand All @@ -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') {
Expand Down