Skip to content

Commit

Permalink
refactor(NODE-3291)!: Standardize error representation in the driver (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
dariakp authored and ljhaywar committed Nov 9, 2021
1 parent bb1fd0a commit b8d59d9
Show file tree
Hide file tree
Showing 78 changed files with 588 additions and 472 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ Below are some conventions that aren't enforced by any of our tooling but we non
As a product of using TS we should be using es6 syntax features whenever possible.
- **Errors**
- Error messages should be sentence case, and have no periods at the end.
- Use built-in error types where possible (not just `Error`, but `TypeError`/`RangeError`), also endeavor to create new Mongo-specific error types (e.g. `MongoNetworkError`)
- Use driver-specific error types where possible (not just `Error`, but classes that extend `MongoError`, e.g. `MongoNetworkError`)

## Pull Request Process

Expand Down
53 changes: 30 additions & 23 deletions src/bulk/common.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { PromiseProvider } from '../promise_provider';
import { Long, ObjectId, Document, BSONSerializeOptions, resolveBSONOptions } from '../bson';
import { MongoError, MongoWriteConcernError, AnyError, MONGODB_ERROR_CODES } from '../error';
import {
MongoWriteConcernError,
AnyError,
MONGODB_ERROR_CODES,
MongoServerError,
MongoDriverError
} from '../error';
import {
applyRetryableWrites,
executeLegacyOperation,
Expand Down Expand Up @@ -303,7 +309,7 @@ export class BulkWriteResult {
}

return new WriteConcernError(
new MongoError({ errmsg: errmsg, code: MONGODB_ERROR_CODES.WriteConcernFailed })
new MongoServerError({ errmsg: errmsg, code: MONGODB_ERROR_CODES.WriteConcernFailed })
);
}
}
Expand All @@ -327,9 +333,9 @@ export class BulkWriteResult {
* @category Error
*/
export class WriteConcernError {
err: MongoError;
err: MongoServerError;

constructor(err: MongoError) {
constructor(err: MongoServerError) {
this.err = err;
}

Expand Down Expand Up @@ -649,16 +655,17 @@ function handleMongoWriteConcernError(
) {
mergeBatchResults(batch, bulkResult, undefined, err.result);

// TODO: Remove multiple levels of wrapping (NODE-3337)
const wrappedWriteConcernError = new WriteConcernError(
new MongoError({
new MongoServerError({
errmsg: err.result?.writeConcernError.errmsg,
code: err.result?.writeConcernError.result
})
);

callback(
new MongoBulkWriteError(
new MongoError(wrappedWriteConcernError),
new MongoServerError(wrappedWriteConcernError),
new BulkWriteResult(bulkResult)
)
);
Expand All @@ -669,7 +676,7 @@ function handleMongoWriteConcernError(
* @public
* @category Error
*/
export class MongoBulkWriteError extends MongoError {
export class MongoBulkWriteError extends MongoServerError {
result: BulkWriteResult;

/** Creates a new MongoBulkWriteError */
Expand Down Expand Up @@ -743,7 +750,7 @@ export class FindOperators {
/** Add a single update operation to the bulk operation */
updateOne(updateDocument: Document): BulkOperationBase {
if (!hasAtomicOperators(updateDocument)) {
throw new TypeError('Update document requires atomic operators');
throw new MongoDriverError('Update document requires atomic operators');
}

const currentOp = buildCurrentOp(this.bulkOperation);
Expand All @@ -756,7 +763,7 @@ export class FindOperators {
/** Add a replace one operation to the bulk operation */
replaceOne(replacement: Document): BulkOperationBase {
if (hasAtomicOperators(replacement)) {
throw new TypeError('Replacement document must not use atomic operators');
throw new MongoDriverError('Replacement document must not use atomic operators');
}

const currentOp = buildCurrentOp(this.bulkOperation);
Expand Down Expand Up @@ -1039,7 +1046,7 @@ export abstract class BulkOperationBase {
*/
find(selector: Document): FindOperators {
if (!selector) {
throw TypeError('Bulk find operation must specify a selector');
throw new MongoDriverError('Bulk find operation must specify a selector');
}

// Save a current selector
Expand Down Expand Up @@ -1073,51 +1080,51 @@ export abstract class BulkOperationBase {
if ('replaceOne' in op || 'updateOne' in op || 'updateMany' in op) {
if ('replaceOne' in op) {
if ('q' in op.replaceOne) {
throw new TypeError('Raw operations are not allowed');
throw new MongoDriverError('Raw operations are not allowed');
}
const updateStatement = makeUpdateStatement(
op.replaceOne.filter,
op.replaceOne.replacement,
{ ...op.replaceOne, multi: false }
);
if (hasAtomicOperators(updateStatement.u)) {
throw new TypeError('Replacement document must not use atomic operators');
throw new MongoDriverError('Replacement document must not use atomic operators');
}
return this.addToOperationsList(BatchType.UPDATE, updateStatement);
}

if ('updateOne' in op) {
if ('q' in op.updateOne) {
throw new TypeError('Raw operations are not allowed');
throw new MongoDriverError('Raw operations are not allowed');
}
const updateStatement = makeUpdateStatement(op.updateOne.filter, op.updateOne.update, {
...op.updateOne,
multi: false
});
if (!hasAtomicOperators(updateStatement.u)) {
throw new TypeError('Update document requires atomic operators');
throw new MongoDriverError('Update document requires atomic operators');
}
return this.addToOperationsList(BatchType.UPDATE, updateStatement);
}

if ('updateMany' in op) {
if ('q' in op.updateMany) {
throw new TypeError('Raw operations are not allowed');
throw new MongoDriverError('Raw operations are not allowed');
}
const updateStatement = makeUpdateStatement(op.updateMany.filter, op.updateMany.update, {
...op.updateMany,
multi: true
});
if (!hasAtomicOperators(updateStatement.u)) {
throw new TypeError('Update document requires atomic operators');
throw new MongoDriverError('Update document requires atomic operators');
}
return this.addToOperationsList(BatchType.UPDATE, updateStatement);
}
}

if ('deleteOne' in op) {
if ('q' in op.deleteOne) {
throw new TypeError('Raw operations are not allowed');
throw new MongoDriverError('Raw operations are not allowed');
}
return this.addToOperationsList(
BatchType.DELETE,
Expand All @@ -1127,7 +1134,7 @@ export abstract class BulkOperationBase {

if ('deleteMany' in op) {
if ('q' in op.deleteMany) {
throw new TypeError('Raw operations are not allowed');
throw new MongoDriverError('Raw operations are not allowed');
}
return this.addToOperationsList(
BatchType.DELETE,
Expand All @@ -1136,7 +1143,7 @@ export abstract class BulkOperationBase {
}

// otherwise an unknown operation was provided
throw TypeError(
throw new MongoDriverError(
'bulkWrite only supports insertOne, updateOne, updateMany, deleteOne, deleteMany'
);
}
Expand Down Expand Up @@ -1170,7 +1177,7 @@ export abstract class BulkOperationBase {
options = options ?? {};

if (this.s.executed) {
return handleEarlyError(new MongoError('Batch cannot be re-executed'), callback);
return handleEarlyError(new MongoDriverError('Batch cannot be re-executed'), callback);
}

const writeConcern = WriteConcern.fromOptions(options);
Expand All @@ -1188,7 +1195,7 @@ export abstract class BulkOperationBase {
}
// If we have no operations in the bulk raise an error
if (this.s.batches.length === 0) {
const emptyBatchError = new TypeError('Invalid BulkOperation, Batch cannot be empty');
const emptyBatchError = new MongoDriverError('Invalid BulkOperation, Batch cannot be empty');
return handleEarlyError(emptyBatchError, callback);
}

Expand All @@ -1211,7 +1218,7 @@ export abstract class BulkOperationBase {

callback(
new MongoBulkWriteError(
new MongoError({
new MongoServerError({
message: msg,
code: this.s.bulkResult.writeErrors[0].code,
writeErrors: this.s.bulkResult.writeErrors
Expand All @@ -1225,7 +1232,7 @@ export abstract class BulkOperationBase {

const writeConcernError = writeResult.getWriteConcernError();
if (writeConcernError) {
callback(new MongoBulkWriteError(new MongoError(writeConcernError), writeResult));
callback(new MongoBulkWriteError(new MongoServerError(writeConcernError), writeResult));
return true;
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/bulk/ordered.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { Document } from '../bson';
import type { Collection } from '../collection';
import type { UpdateStatement } from '../operations/update';
import type { DeleteStatement } from '../operations/delete';
import { MongoDriverError } from '../error';

/** @public */
export class OrderedBulkOperation extends BulkOperationBase {
Expand All @@ -25,7 +26,9 @@ export class OrderedBulkOperation extends BulkOperationBase {

// Throw error if the doc is bigger than the max BSON size
if (bsonSize >= this.s.maxBsonObjectSize)
throw new TypeError(`Document is larger than the maximum size ${this.s.maxBsonObjectSize}`);
throw new MongoDriverError(
`Document is larger than the maximum size ${this.s.maxBsonObjectSize}`
);

// Create a new batch object if we don't have a current one
if (this.s.currentBatch == null) {
Expand Down Expand Up @@ -65,7 +68,7 @@ export class OrderedBulkOperation extends BulkOperationBase {

// We have an array of documents
if (Array.isArray(document)) {
throw new TypeError('Operation passed in cannot be an Array');
throw new MongoDriverError('Operation passed in cannot be an Array');
}

this.s.currentBatch.originalIndexes.push(this.s.currentIndex);
Expand Down
7 changes: 5 additions & 2 deletions src/bulk/unordered.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { Document } from '../bson';
import type { Collection } from '../collection';
import type { UpdateStatement } from '../operations/update';
import type { DeleteStatement } from '../operations/delete';
import { MongoDriverError } from '../error';

/** @public */
export class UnorderedBulkOperation extends BulkOperationBase {
Expand Down Expand Up @@ -35,7 +36,9 @@ export class UnorderedBulkOperation extends BulkOperationBase {

// Throw error if the doc is bigger than the max BSON size
if (bsonSize >= this.s.maxBsonObjectSize) {
throw new TypeError(`Document is larger than the maximum size ${this.s.maxBsonObjectSize}`);
throw new MongoDriverError(
`Document is larger than the maximum size ${this.s.maxBsonObjectSize}`
);
}

// Holds the current batch
Expand Down Expand Up @@ -76,7 +79,7 @@ export class UnorderedBulkOperation extends BulkOperationBase {

// We have an array of documents
if (Array.isArray(document)) {
throw new TypeError('Operation passed in cannot be an Array');
throw new MongoDriverError('Operation passed in cannot be an Array');
}

this.s.currentBatch.operations.push(document);
Expand Down
33 changes: 16 additions & 17 deletions src/change_stream.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Denque = require('denque');
import { MongoError, AnyError, isResumableError } from './error';
import { MongoError, AnyError, isResumableError, MongoDriverError } from './error';
import { AggregateOperation, AggregateOptions } from './operations/aggregate';
import {
maxWireVersion,
Expand Down Expand Up @@ -46,11 +46,10 @@ const CHANGE_DOMAIN_TYPES = {
CLUSTER: Symbol('Cluster')
};

const NO_RESUME_TOKEN_ERROR = new MongoError(
'A change stream document has been received that lacks a resume token (_id).'
);
const NO_CURSOR_ERROR = new MongoError('ChangeStream has no cursor');
const CHANGESTREAM_CLOSED_ERROR = new MongoError('ChangeStream is closed');
const NO_RESUME_TOKEN_ERROR =
'A change stream document has been received that lacks a resume token (_id).';
const NO_CURSOR_ERROR = 'ChangeStream has no cursor';
const CHANGESTREAM_CLOSED_ERROR = 'ChangeStream is closed';

/** @public */
export interface ResumeOptions {
Expand Down Expand Up @@ -254,7 +253,7 @@ export class ChangeStream<TSchema extends Document> extends TypedEventEmitter<Ch
} else if (parent instanceof MongoClient) {
this.type = CHANGE_DOMAIN_TYPES.CLUSTER;
} else {
throw new TypeError(
throw new MongoDriverError(
'parent provided to ChangeStream constructor is not an instance of Collection, Db, or MongoClient'
);
}
Expand Down Expand Up @@ -352,11 +351,11 @@ export class ChangeStream<TSchema extends Document> extends TypedEventEmitter<Ch

/**
* Return a modified Readable stream including a possible transform method.
* @throws MongoError if this.cursor is undefined
* @throws MongoDriverError if this.cursor is undefined
*/
stream(options?: CursorStreamOptions): Readable {
this.streamOptions = options;
if (!this.cursor) throw NO_CURSOR_ERROR;
if (!this.cursor) throw new MongoDriverError(NO_CURSOR_ERROR);
return this.cursor.stream(options);
}

Expand Down Expand Up @@ -606,7 +605,7 @@ function waitForTopologyConnected(
}

if (calculateDurationInMs(start) > timeout) {
return callback(new MongoError('Timed out waiting for connection'));
return callback(new MongoDriverError('Timed out waiting for connection'));
}

waitForTopologyConnected(topology, options, callback);
Expand Down Expand Up @@ -651,17 +650,17 @@ function processNewChange<TSchema>(
callback?: Callback<ChangeStreamDocument<TSchema>>
) {
if (changeStream[kClosed]) {
if (callback) callback(CHANGESTREAM_CLOSED_ERROR);
if (callback) callback(new MongoDriverError(CHANGESTREAM_CLOSED_ERROR));
return;
}

// a null change means the cursor has been notified, implicitly closing the change stream
if (change == null) {
return closeWithError(changeStream, CHANGESTREAM_CLOSED_ERROR, callback);
return closeWithError(changeStream, new MongoDriverError(CHANGESTREAM_CLOSED_ERROR), callback);
}

if (change && !change._id) {
return closeWithError(changeStream, NO_RESUME_TOKEN_ERROR, callback);
return closeWithError(changeStream, new MongoDriverError(NO_RESUME_TOKEN_ERROR), callback);
}

// cache the resume token
Expand All @@ -685,7 +684,7 @@ function processError<TSchema>(

// If the change stream has been closed explicitly, do not process error.
if (changeStream[kClosed]) {
if (callback) callback(CHANGESTREAM_CLOSED_ERROR);
if (callback) callback(new MongoDriverError(CHANGESTREAM_CLOSED_ERROR));
return;
}

Expand Down Expand Up @@ -745,7 +744,7 @@ function processError<TSchema>(
*/
function getCursor<T>(changeStream: ChangeStream<T>, callback: Callback<ChangeStreamCursor<T>>) {
if (changeStream[kClosed]) {
callback(CHANGESTREAM_CLOSED_ERROR);
callback(new MongoDriverError(CHANGESTREAM_CLOSED_ERROR));
return;
}

Expand All @@ -770,11 +769,11 @@ function processResumeQueue<TSchema>(changeStream: ChangeStream<TSchema>, err?:
const request = changeStream[kResumeQueue].pop();
if (!err) {
if (changeStream[kClosed]) {
request(CHANGESTREAM_CLOSED_ERROR);
request(new MongoDriverError(CHANGESTREAM_CLOSED_ERROR));
return;
}
if (!changeStream.cursor) {
request(NO_CURSOR_ERROR);
request(new MongoDriverError(NO_CURSOR_ERROR));
return;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/cmap/auth/auth_provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { Connection, ConnectionOptions } from '../connection';
import type { MongoCredentials } from './mongo_credentials';
import type { HandshakeDocument } from '../connect';
import type { ClientMetadataOptions, Callback } from '../../utils';
import { MongoDriverError } from '../../error';

export type AuthContextOptions = ConnectionOptions & ClientMetadataOptions;

Expand Down Expand Up @@ -53,6 +54,6 @@ export class AuthProvider {
* @param callback - The callback to return the result from the authentication
*/
auth(context: AuthContext, callback: Callback): void {
callback(new TypeError('`auth` method must be overridden by subclass'));
callback(new MongoDriverError('`auth` method must be overridden by subclass'));
}
}
Loading

0 comments on commit b8d59d9

Please sign in to comment.