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
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
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(
'Argument "docs" is a sparse array containing element(s) that are null or undefined'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'Argument "docs" is a sparse array containing element(s) that are null or undefined'
'Collection.insertMany() cannot be called with an array that has null/undefined values'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

);
}
return callback(err);
}
callback(undefined, {
acknowledged: writeConcern?.w !== 0 ?? true,
insertedCount: res.insertedCount,
Expand Down
87 changes: 86 additions & 1 deletion test/integration/crud/bulk.test.js
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,86 @@ 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
try {
bulkOp.raw(undefined);
expect.fail(
'Expected passing argument of "undefined" to .raw to throw error, failed to throw error'
);
} catch (error) {
expect(error.message).to.equal('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('Db.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('insertMany should throw a MongoInvalidArgument error when called with a invalid operation', async function () {
baileympearson marked this conversation as resolved.
Show resolved Hide resolved
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
}
});
});
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