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(NODE-3565): Error for insertMany with partially empty array is unhelpful #3221

Merged
merged 14 commits into from
May 5, 2022
3 changes: 3 additions & 0 deletions src/bulk/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1133,6 +1133,9 @@ export abstract class BulkOperationBase {

/** Specifies a raw operation to perform in the bulk write. */
raw(op: AnyBulkWriteOperation): this {
if (op == null || typeof op !== 'object') {
throw new MongoInvalidArgumentError('Operation must be an object with an operation key');
}
if ('insertOne' in op) {
const forceServerObjectId = shouldForceServerObjectId(this);
if (op.insertOne && op.insertOne.document == null) {
Expand Down
9 changes: 8 additions & 1 deletion src/operations/insert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,14 @@ export class InsertManyOperation extends AbstractOperation<InsertManyResult> {
);

bulkWriteOperation.execute(server, session, (err, res) => {
if (err || res == null) return callback(err);
if (err || res == null) {
if (err && err.message === 'Operation must be an object with an operation key') {
err = new MongoInvalidArgumentError(
'Collection.insertMany() cannot be called with an array that has null/undefined values'
);
}
return callback(err);
}
callback(undefined, {
acknowledged: writeConcern?.w !== 0 ?? true,
insertedCount: res.insertedCount,
Expand Down
85 changes: 84 additions & 1 deletion test/integration/crud/bulk.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@ const {
ignoreNsNotFound,
kampofo marked this conversation as resolved.
Show resolved Hide resolved
assert: test
} = require('../shared');
const { Long, MongoBatchReExecutionError, MongoDriverError } = require('../../../src');
const {
Long,
MongoBatchReExecutionError,
MongoDriverError,
MongoInvalidArgumentError
} = require('../../../src');
const crypto = require('crypto');
const chai = require('chai');

Expand All @@ -22,6 +27,84 @@ describe('Bulk', function () {
before(function () {
return setupDatabase(this.configuration);
});
describe('BulkOperationBase', () => {
describe('#raw()', function () {
let client;
beforeEach(async function () {
client = this.configuration.newClient();
await client.connect();
});
afterEach(async function () {
await client.close();
});
context('when called with an undefined operation', function () {
it('should throw a MongoInvalidArgument error ', async function () {
const bulkOp = client.db('test').collection('test').initializeUnorderedBulkOp();
expect(() => bulkOp.raw(undefined)).to.throw(MongoInvalidArgumentError);
expect(() => bulkOp.raw(true)).to.throw(MongoInvalidArgumentError);
expect(() => bulkOp.raw(3)).to.throw(MongoInvalidArgumentError);
});

it('should throw an error with the specifc message: "Operation must be an object with an operation key"', async function () {
const bulkOp = client.db('test').collection('test').initializeUnorderedBulkOp();
baileympearson marked this conversation as resolved.
Show resolved Hide resolved
expect(() => bulkOp.raw(undefined))
.to.throw(MongoInvalidArgumentError)
.to.match(/Operation must be an object with an operation key/);
});
});

context('when called with a valid operation', function () {
it('should not throw a MongoInvalidArgument error', async function () {
try {
client.db('test').collection('test').initializeUnorderedBulkOp().raw({ insertOne: {} });
} catch (error) {
expect(error).not.to.exist;
}
});
});
});
});

describe('Collection', function () {
describe('#insertMany()', function () {
let client;
beforeEach(async function () {
client = this.configuration.newClient();
await client.connect();
});
afterEach(async function () {
await client.close();
});
context('when passed an invalid docs argument', function () {
it('should throw a MongoInvalidArgument error', async function () {
try {
const docs = [];
docs[1] = { color: 'red' };
await client.db('test').collection('test').insertMany(docs);
expect.fail('Expected insertMany to throw error, failed to throw error');
} catch (error) {
expect(error).to.be.instanceOf(MongoInvalidArgumentError);
baileympearson marked this conversation as resolved.
Show resolved Hide resolved
expect(error.message).to.equal(
'Collection.insertMany() cannot be called with an array that has null/undefined values'
);
}
});
});
context('when passed a valid document list', function () {
it('insertMany should not throw a MongoInvalidArgument error when called with a valid operation', async function () {
try {
let result = await client
.db('test')
.collection('test')
.insertMany([{ color: 'blue' }]);
expect(result).to.exist;
} catch (error) {
expect(error).not.to.exist;
}
});
});
});
});

context('promise tests', () => {
it('Should correctly execute unordered bulk operation in promise form', function (done) {
Expand Down