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

refactor(NODE-3291)!: Standardize error representation in the driver #2824

Merged
merged 37 commits into from
Jun 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
828ee8c
refactor: Replace TypeError with MongoParseError when validating conn…
dariakp May 20, 2021
7ef5b6a
fix: Better stack traces for change stream errors
dariakp May 20, 2021
92193d7
refactor: Change all driver errors to MongoError or subclass
dariakp May 20, 2021
70cab37
refactor: convert more errors to specific types
dariakp May 21, 2021
61735db
refactor: eliminate more error edge cases
dariakp May 26, 2021
4d0c6b0
test: fix expected message
dariakp May 28, 2021
67d6401
Add MongoDriverError and MongoServerError to exports
dariakp May 28, 2021
6741b55
Keep MongoError for debugging
dariakp May 28, 2021
cccb211
remove unnecessary call to capture stack trace
dariakp May 28, 2021
4dffb45
Add sanity check code for MongoDriverError
dariakp May 28, 2021
5c99ad8
Make MongoParseError extend MongoDriverError
dariakp Jun 1, 2021
7093af5
Remove leftover driver error opts
dariakp Jun 1, 2021
4fb29eb
Add sanity check for findOne operation error handler
dariakp Jun 1, 2021
70c1505
Add sanity check to rename collection error handler
dariakp Jun 1, 2021
754b12f
Remove unnecessary wrapping of message parse errors
dariakp Jun 1, 2021
189be89
Update cmap errors to extend MongoDriverError instead of MongoError
dariakp Jun 1, 2021
2139c46
Add MongoSystemError type
dariakp Jun 2, 2021
a90cfbd
Make MongoWriteConcernError extend MongoServerError
dariakp Jun 2, 2021
6859329
Flatten MongoError class logic
dariakp Jun 2, 2021
f7c15df
Refine error types
dariakp Jun 2, 2021
796a2a4
Simplify unrecoverable error logic
dariakp Jun 2, 2021
14eae5d
Remove deprecated MongoError.create method
dariakp Jun 2, 2021
aa6e4f9
Use more precise error type for server and topology descriptions
dariakp Jun 3, 2021
57a2808
Remove test sanity check code
dariakp Jun 3, 2021
49f3210
Remove extra wrapping of errors in listIndex toArray as those errors …
dariakp Jun 3, 2021
2101fa0
Use shared code to set errorLabels on MongoWriteConcernError
dariakp Jun 3, 2021
b3e32c9
Fix incorrect message type in MongoWriteConcernError
dariakp Jun 3, 2021
27ff6db
Move server error parsing logic to MongoServerError class
dariakp Jun 3, 2021
235525b
Update spec test mock
dariakp Jun 3, 2021
99e2ecc
Fix errors test
dariakp Jun 3, 2021
953dfb4
Move codeName to be property on MongoServerError only
dariakp Jun 4, 2021
3d1c540
Move writeConcernError property to be only on MongoServerError
dariakp Jun 4, 2021
e409b78
Apply correct types to error codes
dariakp Jun 4, 2021
a998b92
Remove as any from gridfs download error
dariakp Jun 4, 2021
e0bb546
Clean up error type use
dariakp Jun 4, 2021
cc145dd
Remove unused string option to MongoServerError constructor
dariakp Jun 4, 2021
ed91110
code review feedback
dariakp Jun 8, 2021
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
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