From 08cf9ac0fd259d7293820501356b75659d7350fa Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 19 Jul 2021 22:59:28 -0400 Subject: [PATCH 01/24] refactor(NODE-3405): replace MongoDriverError where appropriate --- src/change_stream.ts | 20 ++++++++++---------- src/cmap/commands.ts | 4 ++-- src/cmap/connection_pool.ts | 5 +++-- src/gridfs/upload.ts | 5 +++-- src/index.ts | 7 ++++++- src/operations/common_functions.ts | 4 ++-- src/operations/execute_operation.ts | 13 ++++++------- src/sdam/server.ts | 15 ++++++++------- src/sdam/topology.ts | 13 ++++++++++--- src/sessions.ts | 21 ++++++++++++--------- src/transactions.ts | 4 ++-- src/utils.ts | 9 ++++++--- 12 files changed, 70 insertions(+), 50 deletions(-) diff --git a/src/change_stream.ts b/src/change_stream.ts index 1e80cf7e58e..e3181e79377 100644 --- a/src/change_stream.ts +++ b/src/change_stream.ts @@ -683,15 +683,15 @@ function processNewChange( callback?: Callback> ) { if (changeStream[kClosed]) { - // TODO(NODE-3405): Replace with MongoStreamClosedError - if (callback) callback(new MongoDriverError(CHANGESTREAM_CLOSED_ERROR)); + // TODO(NODE-3485): Replace with MongoChangeStreamStreamClosedError + if (callback) callback(new MongoAPIError(CHANGESTREAM_CLOSED_ERROR)); return; } // a null change means the cursor has been notified, implicitly closing the change stream if (change == null) { - // TODO(NODE-3405): Replace with MongoStreamClosedError - return closeWithError(changeStream, new MongoDriverError(CHANGESTREAM_CLOSED_ERROR), callback); + // TODO(NODE-3485): Replace with MongoChangeStreamStreamClosedError + return closeWithError(changeStream, new MongoAPIError(CHANGESTREAM_CLOSED_ERROR), callback); } if (change && !change._id) { @@ -723,8 +723,8 @@ function processError( // If the change stream has been closed explicitly, do not process error. if (changeStream[kClosed]) { - // TODO(NODE-3405): Replace with MongoStreamClosedError - if (callback) callback(new MongoDriverError(CHANGESTREAM_CLOSED_ERROR)); + // TODO(NODE-3485): Replace with MongoChangeStreamStreamClosedError + if (callback) callback(new MongoAPIError(CHANGESTREAM_CLOSED_ERROR)); return; } @@ -784,8 +784,8 @@ function processError( */ function getCursor(changeStream: ChangeStream, callback: Callback>) { if (changeStream[kClosed]) { - // TODO(NODE-3405): Replace with MongoStreamClosedError - callback(new MongoDriverError(CHANGESTREAM_CLOSED_ERROR)); + // TODO(NODE-3485): Replace with MongoChangeStreamStreamClosedError + callback(new MongoAPIError(CHANGESTREAM_CLOSED_ERROR)); return; } @@ -810,8 +810,8 @@ function processResumeQueue(changeStream: ChangeStream, err?: const request = changeStream[kResumeQueue].pop(); if (!err) { if (changeStream[kClosed]) { - // TODO(NODE-3405): Replace with MongoStreamClosedError - request(new MongoDriverError(CHANGESTREAM_CLOSED_ERROR)); + // TODO(NODE-3485): Replace with MongoChangeStreamStreamClosedError + request(new MongoAPIError(CHANGESTREAM_CLOSED_ERROR)); return; } if (!changeStream.cursor) { diff --git a/src/cmap/commands.ts b/src/cmap/commands.ts index 140fea90cd2..149a693c147 100644 --- a/src/cmap/commands.ts +++ b/src/cmap/commands.ts @@ -84,8 +84,8 @@ export class Query { // Validate that we are not passing 0x00 in the collection name if (ns.indexOf('\x00') !== -1) { - // TODO(NODE-3483): Replace with MongoCommandError - throw new MongoDriverError('Namespace cannot contain a null character'); + // TODO(NODE-3483): Use MongoNamespace static method + throw new MongoDriverError('namespace cannot contain a null character'); } // Basic options diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index d63c10d8b97..cb5a637f480 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -5,7 +5,7 @@ import { Logger } from '../logger'; import { ConnectionPoolMetrics } from './metrics'; import { connect } from './connect'; import { eachAsync, makeCounter, Callback } from '../utils'; -import { MongoDriverError, MongoError, MongoInvalidArgumentError } from '../error'; +import { MongoError, MongoInvalidArgumentError, MongoDriverError, MongoAPIError } from '../error'; import { PoolClosedError, WaitQueueTimeoutError } from './errors'; import { ConnectionPoolCreatedEvent, @@ -388,7 +388,8 @@ export class ConnectionPool extends TypedEventEmitter { clearTimeout(waitQueueMember.timer); } if (!waitQueueMember[kCancelled]) { - waitQueueMember.callback(new MongoDriverError('connection pool closed')); + // TODO(NODE-3485): Replace with MongoConnectionPoolClosedError + waitQueueMember.callback(new MongoAPIError('Connection pool closed')); } } } diff --git a/src/gridfs/upload.ts b/src/gridfs/upload.ts index ff52e526633..c0cf175939e 100644 --- a/src/gridfs/upload.ts +++ b/src/gridfs/upload.ts @@ -4,6 +4,7 @@ import { ObjectId } from '../bson'; import type { Collection } from '../collection'; import { AnyError, + MongoAPIError, MONGODB_ERROR_CODES, MongoDriverError, MongoError, @@ -565,8 +566,8 @@ function writeRemnant(stream: GridFSBucketWriteStream, callback?: Callback): boo function checkAborted(stream: GridFSBucketWriteStream, callback?: Callback): boolean { if (stream.state.aborted) { if (typeof callback === 'function') { - // TODO(NODE-3405): Replace with MongoStreamClosedError - callback(new MongoDriverError('this stream has been aborted')); + // TODO(NODE-3485): Replace with MongoGridFSStreamClosedError + callback(new MongoAPIError('Stream has been aborted')); } return true; } diff --git a/src/index.ts b/src/index.ts index f58caa191ca..b06f4ac0ea1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -52,7 +52,12 @@ export { MongoBatchReExecutionError, MongoCursorExhaustedError, MongoCursorInUseError, - MongoNotConnectedError + MongoNotConnectedError, + MongoExpiredSessionError, + MongoTransactionError, + MongoKerberosError, + MongoServerClosedError, + MongoTopologyClosedError } from './error'; export { MongoBulkWriteError, BulkWriteOptions, AnyBulkWriteOperation } from './bulk/common'; export { diff --git a/src/operations/common_functions.ts b/src/operations/common_functions.ts index 2ac555bfd43..049a16b76fd 100644 --- a/src/operations/common_functions.ts +++ b/src/operations/common_functions.ts @@ -1,4 +1,4 @@ -import { MongoDriverError } from '../error'; +import { MongoDriverError, MongoTopologyClosedError } from '../error'; import { Callback, getTopology } from '../utils'; import type { Document } from '../bson'; import type { Db } from '../db'; @@ -42,7 +42,7 @@ export function indexInformation( // Did the user destroy the topology if (getTopology(db).isDestroyed()) - return callback(new MongoDriverError('topology was destroyed')); + return callback(new MongoTopologyClosedError('Topology was destroyed')); // Process all the results from the index command and collection function processResults(indexes: any) { // Contains all the information diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts index 48591a68b0d..bb23db16782 100644 --- a/src/operations/execute_operation.ts +++ b/src/operations/execute_operation.ts @@ -6,7 +6,9 @@ import { MongoDriverError, MongoNetworkError, MongoCompatibilityError, - MongoServerError + MongoServerError, + MongoExpiredSessionError, + MongoTransactionError } from '../error'; import { Aspect, AbstractOperation } from './operation'; import { maxWireVersion, maybePromise, Callback } from '../utils'; @@ -88,8 +90,7 @@ export function executeOperation< owner = Symbol(); session = topology.startSession({ owner, explicit: false }); } else if (session.hasEnded) { - // TODO(NODE-3405): Change this out for MongoExpiredSessionError - return cb(new MongoDriverError('Use of expired sessions is not permitted')); + return cb(new MongoExpiredSessionError('Use of expired sessions is not permitted')); } else if (session.snapshotEnabled && !topology.capabilities.supportsSnapshotReads) { return cb(new MongoDriverError('Snapshot reads require MongoDB 5.0 or later')); } @@ -132,8 +133,7 @@ function executeWithServerSelection( if (inTransaction && !readPreference.equals(ReadPreference.primary)) { callback( - // TODO(NODE-3405): Change this out for MongoTransactionError - new MongoDriverError( + new MongoTransactionError( `Read preference in a transaction must be primary, not: ${readPreference.mode}` ) ); @@ -218,8 +218,7 @@ function executeWithServerSelection( session.inTransaction() ) { callback( - // TODO(NODE-3405): Change this out for MongoTransactionError - new MongoDriverError( + new MongoTransactionError( `Read preference in a transaction must be primary, not: ${readPreference.mode}` ) ); diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 3900483aebd..e821802176e 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -33,9 +33,9 @@ import { isRetryableWriteError, isNodeShuttingDownError, isNetworkErrorBeforeHandshake, - MongoDriverError, MongoCompatibilityError, - MongoInvalidArgumentError + MongoInvalidArgumentError, + MongoServerClosedError } from '../error'; import { Connection, @@ -65,6 +65,8 @@ const stateTransition = makeStateMachine({ [STATE_CLOSING]: [STATE_CLOSING, STATE_CLOSED] }); +const SERVER_CLOSED_ERROR = 'Server is closed'; + /** @internal */ const kMonitor = Symbol('monitor'); @@ -292,8 +294,7 @@ export class Server extends TypedEventEmitter { } if (this.s.state === STATE_CLOSING || this.s.state === STATE_CLOSED) { - // TODO(NODE-3405): Change this out for MongoServerClosedError - callback(new MongoDriverError('Server is closed')); + callback(new MongoServerClosedError(SERVER_CLOSED_ERROR)); return; } @@ -351,7 +352,7 @@ export class Server extends TypedEventEmitter { */ query(ns: MongoDBNamespace, cmd: Document, options: QueryOptions, callback: Callback): void { if (this.s.state === STATE_CLOSING || this.s.state === STATE_CLOSED) { - callback(new MongoDriverError('server is closed')); + callback(new MongoServerClosedError(SERVER_CLOSED_ERROR)); return; } @@ -385,7 +386,7 @@ export class Server extends TypedEventEmitter { callback: Callback ): void { if (this.s.state === STATE_CLOSING || this.s.state === STATE_CLOSED) { - callback(new MongoDriverError('server is closed')); + callback(new MongoServerClosedError(SERVER_CLOSED_ERROR)); return; } @@ -420,7 +421,7 @@ export class Server extends TypedEventEmitter { ): void { if (this.s.state === STATE_CLOSING || this.s.state === STATE_CLOSED) { if (typeof callback === 'function') { - callback(new MongoDriverError('server is closed')); + callback(new MongoServerClosedError(SERVER_CLOSED_ERROR)); } return; diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index 48ee8e4f5df..5dbd9a303c0 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -11,7 +11,12 @@ import { } from '../sessions'; import { SrvPoller, SrvPollingEvent } from './srv_polling'; import { CMAP_EVENTS, ConnectionPoolEvents } from '../cmap/connection_pool'; -import { MongoServerSelectionError, MongoCompatibilityError, MongoDriverError } from '../error'; +import { + MongoServerSelectionError, + MongoCompatibilityError, + MongoDriverError, + MongoTopologyClosedError +} from '../error'; import { readPreferenceServerSelector, ServerSelector } from './server_selection'; import { makeStateMachine, @@ -83,6 +88,8 @@ const stateTransition = makeStateMachine({ [STATE_CLOSING]: [STATE_CLOSING, STATE_CLOSED] }); +const TOPOLOGY_CLOSED_ERROR = 'Topology is closed'; + /** @internal */ const kCancelled = Symbol('cancelled'); /** @internal */ @@ -491,7 +498,7 @@ export class Topology extends TypedEventEmitter { stateTransition(this, STATE_CLOSING); - drainWaitQueue(this[kWaitQueue], new MongoDriverError('Topology closed')); + drainWaitQueue(this[kWaitQueue], new MongoTopologyClosedError(TOPOLOGY_CLOSED_ERROR)); drainTimerQueue(this.s.connectionTimers); if (this.s.srvPoller) { @@ -981,7 +988,7 @@ function processWaitQueue(topology: Topology) { if (topology.s.state === STATE_CLOSED) { drainWaitQueue( topology[kWaitQueue], - new MongoDriverError('Topology is closed, please connect') + new MongoTopologyClosedError(TOPOLOGY_CLOSED_ERROR + ', please connect') ); return; } diff --git a/src/sessions.ts b/src/sessions.ts index a7a4428d5d7..6e87ab746be 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -14,7 +14,9 @@ import { MONGODB_ERROR_CODES, MongoDriverError, MongoServerError, - AnyError + AnyError, + MongoExpiredSessionError, + MongoTransactionError } from './error'; import { now, @@ -41,7 +43,7 @@ const minWireVersionForShardedTransactions = 8; function assertAlive(session: ClientSession, callback?: Callback): boolean { if (session.serverSession == null) { - const error = new MongoDriverError('Cannot use a session that has ended'); + const error = new MongoExpiredSessionError('Cannot use a session that has ended'); if (typeof callback === 'function') { callback(error); return false; @@ -370,12 +372,12 @@ export class ClientSession extends TypedEventEmitter { */ startTransaction(options?: TransactionOptions): void { if (this[kSnapshotEnabled]) { - throw new MongoDriverError('Transactions are not allowed with snapshot sessions'); + throw new MongoTransactionError('Transactions are not allowed with snapshot sessions'); } assertAlive(this); if (this.inTransaction()) { - throw new MongoDriverError('Transaction already in progress'); + throw new MongoTransactionError('Transaction already in progress'); } if (this.isPinned && this.transaction.isCommitted) { @@ -441,6 +443,7 @@ export class ClientSession extends TypedEventEmitter { * This is here to ensure that ClientSession is never serialized to BSON. */ toBSON(): never { + // TODO(NODE-3405): Replace with MongoBSONParseError throw new MongoDriverError('ClientSession cannot be serialized to BSON.'); } @@ -643,7 +646,7 @@ function endTransaction(session: ClientSession, commandName: string, callback: C const txnState = session.transaction.state; if (txnState === TxnState.NO_TRANSACTION) { - callback(new MongoDriverError('No transaction started')); + callback(new MongoTransactionError('No transaction started')); return; } @@ -660,7 +663,7 @@ function endTransaction(session: ClientSession, commandName: string, callback: C if (txnState === TxnState.TRANSACTION_ABORTED) { callback( - new MongoDriverError('Cannot call commitTransaction after calling abortTransaction') + new MongoTransactionError('Cannot call commitTransaction after calling abortTransaction') ); return; } @@ -673,7 +676,7 @@ function endTransaction(session: ClientSession, commandName: string, callback: C } if (txnState === TxnState.TRANSACTION_ABORTED) { - callback(new MongoDriverError('Cannot call abortTransaction twice')); + callback(new MongoTransactionError('Cannot call abortTransaction twice')); return; } @@ -682,7 +685,7 @@ function endTransaction(session: ClientSession, commandName: string, callback: C txnState === TxnState.TRANSACTION_COMMITTED_EMPTY ) { callback( - new MongoDriverError('Cannot call abortTransaction after calling commitTransaction') + new MongoTransactionError('Cannot call abortTransaction after calling commitTransaction') ); return; } @@ -957,7 +960,7 @@ export function applySession( ): MongoDriverError | undefined { // TODO: merge this with `assertAlive`, did not want to throw a try/catch here if (session.hasEnded) { - return new MongoDriverError('Attempted to use a session that has ended'); + return new MongoExpiredSessionError('Attempted to use a session that has ended'); } const serverSession = session.serverSession; diff --git a/src/transactions.ts b/src/transactions.ts index 4802e6d3bd3..9ab9fa0ea20 100644 --- a/src/transactions.ts +++ b/src/transactions.ts @@ -1,5 +1,5 @@ import { ReadPreference } from './read_preference'; -import { MongoDriverError } from './error'; +import { MongoDriverError, MongoTransactionError } from './error'; import { ReadConcern } from './read_concern'; import { WriteConcern } from './write_concern'; import type { Server } from './sdam/server'; @@ -94,7 +94,7 @@ export class Transaction { const writeConcern = WriteConcern.fromOptions(options); if (writeConcern) { if (writeConcern.w === 0) { - throw new MongoDriverError('Transactions do not support unacknowledged write concern'); + throw new MongoTransactionError('Transactions do not support unacknowledged write concern'); } this.options.writeConcern = writeConcern; diff --git a/src/utils.ts b/src/utils.ts index f6b28a4ff10..22efc31ce77 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -7,7 +7,8 @@ import { MongoDriverError, MongoCompatibilityError, MongoNotConnectedError, - MongoInvalidArgumentError + MongoInvalidArgumentError, + MongoExpiredSessionError } from './error'; import { WriteConcern, WriteConcernOptions, W } from './write_concern'; import type { Server } from './sdam/server'; @@ -74,15 +75,18 @@ export function checkCollectionName(collectionName: string): void { collectionName.indexOf('$') !== -1 && collectionName.match(/((^\$cmd)|(oplog\.\$main))/) == null ) { + // TODO(NODE-3483): Use MongoNamespace static method throw new MongoInvalidArgumentError("Collection names must not contain '$'"); } if (collectionName.match(/^\.|\.$/) != null) { + // TODO(NODE-3483): Use MongoNamespace static method throw new MongoInvalidArgumentError("Collection names must not start or end with '.'"); } // Validate that we are not passing 0x00 in the collection name if (collectionName.indexOf('\x00') !== -1) { + // TODO(NODE-3483): Use MongoNamespace static method throw new MongoInvalidArgumentError('Collection names cannot contain a null character'); } } @@ -258,8 +262,7 @@ export function executeLegacyOperation( const optionsIndex = args.length - 2; args[optionsIndex] = Object.assign({}, args[optionsIndex], { session: session }); } else if (opOptions.session && opOptions.session.hasEnded) { - // TODO(NODE-3405): Replace this with MongoExpiredSessionError - throw new MongoDriverError('Use of expired sessions is not permitted'); + throw new MongoExpiredSessionError('Use of expired sessions is not permitted'); } } From 978e6af46c8f6ecdf2194f523cabb3662479f744 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 19 Jul 2021 23:21:05 -0400 Subject: [PATCH 02/24] test(NODE-3405): adjust tests to expect MongoStreamClosedError --- test/functional/gridfs_stream.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/gridfs_stream.test.js b/test/functional/gridfs_stream.test.js index bfe6a5ec8fc..1fc742d109b 100644 --- a/test/functional/gridfs_stream.test.js +++ b/test/functional/gridfs_stream.test.js @@ -518,11 +518,11 @@ describe('GridFS Stream', function () { expect(c).to.equal(0); uploadStream.write('b', 'utf8', function (error) { expect(error.toString()).to.equal( - 'MongoDriverError: this stream has been aborted' + 'MongoStreamClosedError: Stream has been aborted' ); uploadStream.end('c', 'utf8', function (error) { expect(error.toString()).to.equal( - 'MongoDriverError: this stream has been aborted' + 'MongoStreamClosedError: Stream has been aborted' ); // Fail if user tries to abort an aborted stream uploadStream.abort().then(null, function (error) { From c735100a1e5caba4fe3542d104da51ebb3c40a76 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 19 Jul 2021 23:32:43 -0400 Subject: [PATCH 03/24] test(NODE-3405): fix gridfs tests to expect MongoStreamClosedError --- test/functional/gridfs_stream.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/functional/gridfs_stream.test.js b/test/functional/gridfs_stream.test.js index 1fc742d109b..2dbf24ec3eb 100644 --- a/test/functional/gridfs_stream.test.js +++ b/test/functional/gridfs_stream.test.js @@ -464,11 +464,11 @@ describe('GridFS Stream', function () { expect(c).to.equal(0); uploadStream.write('b', 'utf8', function (error) { expect(error.toString()).to.equal( - 'MongoDriverError: this stream has been aborted' + 'MongoStreamClosedError: Stream has been aborted' ); uploadStream.end('c', 'utf8', function (error) { expect(error.toString()).to.equal( - 'MongoDriverError: this stream has been aborted' + 'MongoStreamClosedError: Stream has been aborted' ); // Fail if user tries to abort an aborted stream uploadStream.abort().then(null, function (error) { @@ -1175,12 +1175,12 @@ describe('GridFS Stream', function () { uploadStream.write('b', 'utf8', function (error) { expect(error.toString()).to.equal( - 'MongoDriverError: this stream has been aborted' + 'MongoStreamClosedError: Stream has been aborted' ); uploadStream.end(function (error) { expect(error.toString()).to.equal( - 'MongoDriverError: this stream has been aborted' + 'MongoStreamClosedError: Stream has been aborted' ); // Fail if user tries to abort an aborted stream From 959cc540b12874da8aec091939412e3739d6681d Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 19 Jul 2021 23:40:55 -0400 Subject: [PATCH 04/24] style(NODE-3405): fix linter errors --- src/operations/common_functions.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/operations/common_functions.ts b/src/operations/common_functions.ts index 049a16b76fd..5bf7cd24915 100644 --- a/src/operations/common_functions.ts +++ b/src/operations/common_functions.ts @@ -1,10 +1,10 @@ -import { MongoDriverError, MongoTopologyClosedError } from '../error'; -import { Callback, getTopology } from '../utils'; import type { Document } from '../bson'; +import type { Collection } from '../collection'; import type { Db } from '../db'; -import type { ClientSession } from '../sessions'; +import { MongoTopologyClosedError } from '../error'; import type { ReadPreference } from '../read_preference'; -import type { Collection } from '../collection'; +import type { ClientSession } from '../sessions'; +import { Callback, getTopology } from '../utils'; /** @public */ export interface IndexInformationOptions { From 0b85ae283f7582bd34f7c496419924d226415ed8 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 21 Jul 2021 14:11:21 -0400 Subject: [PATCH 05/24] refactor(NODE-3405): replace more MongoDriverError --- src/change_stream.ts | 2 +- src/gridfs/upload.ts | 9 ++++----- test/functional/gridfs_stream.test.js | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/change_stream.ts b/src/change_stream.ts index e3181e79377..23ac28d072c 100644 --- a/src/change_stream.ts +++ b/src/change_stream.ts @@ -58,7 +58,7 @@ const CHANGE_DOMAIN_TYPES = { 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'; +const CHANGESTREAM_CLOSED_ERROR = 'ChangeStream has been closed'; /** @public */ export interface ResumeOptions { diff --git a/src/gridfs/upload.ts b/src/gridfs/upload.ts index c0cf175939e..ad6888c17f4 100644 --- a/src/gridfs/upload.ts +++ b/src/gridfs/upload.ts @@ -6,7 +6,6 @@ import { AnyError, MongoAPIError, MONGODB_ERROR_CODES, - MongoDriverError, MongoError, MongoGridFSStreamError } from '../error'; @@ -153,16 +152,16 @@ export class GridFSBucketWriteStream extends Writable { const Promise = PromiseProvider.get(); let error: MongoGridFSStreamError; if (this.state.streamEnd) { - // TODO(NODE-3405): Replace with MongoStreamClosedError - error = new MongoDriverError('Cannot abort a stream that has already completed'); + // TODO(NODE-3485): Replace with MongoGridFSStreamClosedError + error = new MongoAPIError('Cannot abort a stream that has already completed'); if (typeof callback === 'function') { return callback(error); } return Promise.reject(error); } if (this.state.aborted) { - // TODO(NODE-3405): Replace with MongoStreamClosedError - error = new MongoDriverError('Cannot call abort() on a stream twice'); + // TODO(NODE-3485): Replace with MongoGridFSStreamClosedError + error = new MongoAPIError('Cannot call abort() on a stream twice'); if (typeof callback === 'function') { return callback(error); } diff --git a/test/functional/gridfs_stream.test.js b/test/functional/gridfs_stream.test.js index 2dbf24ec3eb..5aab5e03c62 100644 --- a/test/functional/gridfs_stream.test.js +++ b/test/functional/gridfs_stream.test.js @@ -473,8 +473,8 @@ describe('GridFS Stream', function () { // Fail if user tries to abort an aborted stream uploadStream.abort().then(null, function (error) { expect(error.toString()).to.equal( - // TODO(NODE-3405): Replace with MongoStreamClosedError - 'MongoDriverError: Cannot call abort() on a stream twice' + // TODO(NODE-3485): Replace with MongoGridFSStreamClosedError + 'MongoAPIError: Cannot call abort() on a stream twice' ); client.close(done); }); From 3ae8af9dd7aa8c0278e368a8fae3dd0131810e7f Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 21 Jul 2021 14:22:59 -0400 Subject: [PATCH 06/24] test(NODE-3405): fix GridFS test waiting for the wrong error --- test/functional/gridfs_stream.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/gridfs_stream.test.js b/test/functional/gridfs_stream.test.js index 5aab5e03c62..9990dfd0ab6 100644 --- a/test/functional/gridfs_stream.test.js +++ b/test/functional/gridfs_stream.test.js @@ -527,8 +527,8 @@ describe('GridFS Stream', function () { // Fail if user tries to abort an aborted stream uploadStream.abort().then(null, function (error) { expect(error.toString()).to.equal( - // TODO(NODE-3405): Replace with MongoStreamClosedError - 'MongoDriverError: Cannot call abort() on a stream twice' + // TODO(NODE-3485): Replace with MongoGridFSStreamClosedError + 'MongoAPIError: Cannot call abort() on a stream twice' ); client.close(done); }); From 86e92d4abb85c9a3215e4bbd4a048c96fefdee4f Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 21 Jul 2021 15:13:12 -0400 Subject: [PATCH 07/24] test(NODE-3405): fix tests waiting for outdated errors --- test/functional/change_stream.test.js | 2 +- test/functional/gridfs_stream.test.js | 4 ++-- test/unit/sdam/server_selection/select_servers.test.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/functional/change_stream.test.js b/test/functional/change_stream.test.js index 5508a0fbb18..d8786cdff75 100644 --- a/test/functional/change_stream.test.js +++ b/test/functional/change_stream.test.js @@ -2887,7 +2887,7 @@ context('NODE-2626 - handle null changes without error', function () { changeStream.next((err, doc) => { expect(err).to.exist; expect(doc).to.not.exist; - expect(err.message).to.equal('ChangeStream is closed'); + expect(err.message).to.equal('ChangeStream has been closed'); changeStream.close(() => client.close(done)); }); }); diff --git a/test/functional/gridfs_stream.test.js b/test/functional/gridfs_stream.test.js index 9990dfd0ab6..446d16d529d 100644 --- a/test/functional/gridfs_stream.test.js +++ b/test/functional/gridfs_stream.test.js @@ -1186,8 +1186,8 @@ describe('GridFS Stream', function () { // Fail if user tries to abort an aborted stream uploadStream.abort().then(null, function (error) { expect(error.toString()).to.equal( - // TODO(NODE-3405): Replace with MongoStreamClosedError - 'MongoDriverError: Cannot call abort() on a stream twice' + // TODO(NODE-3485): Replace with MongoGridFSStreamClosedError + 'MongoAPIError: Cannot call abort() on a stream twice' ); client.close(done); }); diff --git a/test/unit/sdam/server_selection/select_servers.test.js b/test/unit/sdam/server_selection/select_servers.test.js index e5637624e66..25b82722bff 100644 --- a/test/unit/sdam/server_selection/select_servers.test.js +++ b/test/unit/sdam/server_selection/select_servers.test.js @@ -57,7 +57,7 @@ describe('selectServer', function () { topology.close(() => { topology.selectServer(ReadPreference.primary, { serverSelectionTimeoutMS: 2000 }, err => { expect(err).to.exist; - expect(err).to.match(/Topology is closed/); + expect(err).to.match(/Topology has been closed/); done(); }); }); From bf7a3d738f0f4789caf10a17f40ebd481e55682c Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 21 Jul 2021 15:53:08 -0400 Subject: [PATCH 08/24] test(NODE-3405): fix ChangeStream test --- test/functional/change_stream.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/change_stream.test.js b/test/functional/change_stream.test.js index d8786cdff75..c2e638bb8ff 100644 --- a/test/functional/change_stream.test.js +++ b/test/functional/change_stream.test.js @@ -1956,7 +1956,7 @@ describe('Change Streams', function () { return Promise.all([read(), write()]).then( () => Promise.reject(new Error('Expected operation to fail with error')), - err => expect(err.message).to.equal('ChangeStream is closed') + err => expect(err.message).to.equal('ChangeStream has been closed') ); } }); From f7da7afe90b352bb24770a7c2f940fa5095f17f4 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 21 Jul 2021 16:20:30 -0400 Subject: [PATCH 09/24] test(NODE-3405): fix ChangeStream test waiting for outdated error --- test/functional/change_stream.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/change_stream.test.js b/test/functional/change_stream.test.js index c2e638bb8ff..2e14a7a7d72 100644 --- a/test/functional/change_stream.test.js +++ b/test/functional/change_stream.test.js @@ -1976,7 +1976,7 @@ describe('Change Streams', function () { try { expect(err) .property('message') - .to.match(/ChangeStream is closed/); + .to.match(/ChangeStream has been closed/); Promise.all(ops).then(() => done(), done); } catch (e) { done(e); From f69a7701009055940fb99c7be89463a125ccf4ab Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 22 Jul 2021 14:06:25 -0400 Subject: [PATCH 10/24] style(NODE-3405): update TODO comments to point to correct tix --- src/cmap/commands.ts | 12 ++++++++---- src/sessions.ts | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/cmap/commands.ts b/src/cmap/commands.ts index 149a693c147..c71c41c3a2d 100644 --- a/src/cmap/commands.ts +++ b/src/cmap/commands.ts @@ -1,11 +1,15 @@ -import { ReadPreference } from '../read_preference'; +import type { BSONSerializeOptions, Document, Long } from '../bson'; import * as BSON from '../bson'; -import { databaseNamespace } from '../utils'; -import { OP_QUERY, OP_GETMORE, OP_KILL_CURSORS, OP_MSG } from './wire_protocol/constants'; -import type { Long, Document, BSONSerializeOptions } from '../bson'; +import { MongoDriverError } from '../error'; +import { ReadPreference } from '../read_preference'; import type { ClientSession } from '../sessions'; +import { databaseNamespace } from '../utils'; import type { CommandOptions } from './connection'; +<<<<<<< HEAD import { MongoDriverError, MongoInvalidArgumentError } from '../error'; +======= +import { OP_GETMORE, OP_KILL_CURSORS, OP_MSG, OP_QUERY } from './wire_protocol/constants'; +>>>>>>> 2f59b41fb (style(NODE-3405): update TODO comments to point to correct tix) // Incrementing request id let _requestId = 0; diff --git a/src/sessions.ts b/src/sessions.ts index 6e87ab746be..00621df8b05 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -443,7 +443,7 @@ export class ClientSession extends TypedEventEmitter { * This is here to ensure that ClientSession is never serialized to BSON. */ toBSON(): never { - // TODO(NODE-3405): Replace with MongoBSONParseError + // TODO(NODE-3484): Replace with MongoBSONParseError throw new MongoDriverError('ClientSession cannot be serialized to BSON.'); } From 1626d3f2def62a220739d801196fabca5643ecf1 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 26 Jul 2021 11:33:18 -0400 Subject: [PATCH 11/24] style(NODE-3405): revert import order --- src/change_stream.ts | 2 +- src/cmap/commands.ts | 12 ++++-------- src/gridfs/upload.ts | 14 +++++++------- src/operations/common_functions.ts | 8 ++++---- 4 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/change_stream.ts b/src/change_stream.ts index 23ac28d072c..e3181e79377 100644 --- a/src/change_stream.ts +++ b/src/change_stream.ts @@ -58,7 +58,7 @@ const CHANGE_DOMAIN_TYPES = { 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 has been closed'; +const CHANGESTREAM_CLOSED_ERROR = 'ChangeStream is closed'; /** @public */ export interface ResumeOptions { diff --git a/src/cmap/commands.ts b/src/cmap/commands.ts index c71c41c3a2d..149a693c147 100644 --- a/src/cmap/commands.ts +++ b/src/cmap/commands.ts @@ -1,15 +1,11 @@ -import type { BSONSerializeOptions, Document, Long } from '../bson'; -import * as BSON from '../bson'; -import { MongoDriverError } from '../error'; import { ReadPreference } from '../read_preference'; -import type { ClientSession } from '../sessions'; +import * as BSON from '../bson'; import { databaseNamespace } from '../utils'; +import { OP_QUERY, OP_GETMORE, OP_KILL_CURSORS, OP_MSG } from './wire_protocol/constants'; +import type { Long, Document, BSONSerializeOptions } from '../bson'; +import type { ClientSession } from '../sessions'; import type { CommandOptions } from './connection'; -<<<<<<< HEAD import { MongoDriverError, MongoInvalidArgumentError } from '../error'; -======= -import { OP_GETMORE, OP_KILL_CURSORS, OP_MSG, OP_QUERY } from './wire_protocol/constants'; ->>>>>>> 2f59b41fb (style(NODE-3405): update TODO comments to point to correct tix) // Incrementing request id let _requestId = 0; diff --git a/src/gridfs/upload.ts b/src/gridfs/upload.ts index ad6888c17f4..1c7c61f7fa6 100644 --- a/src/gridfs/upload.ts +++ b/src/gridfs/upload.ts @@ -1,20 +1,20 @@ import { Writable } from 'stream'; -import type { Document } from '../bson'; -import { ObjectId } from '../bson'; -import type { Collection } from '../collection'; import { + MongoError, AnyError, MongoAPIError, MONGODB_ERROR_CODES, - MongoError, MongoGridFSStreamError } from '../error'; +import { WriteConcern } from './../write_concern'; import { PromiseProvider } from '../promise_provider'; +import { ObjectId } from '../bson'; import type { Callback } from '../utils'; -import type { WriteConcernOptions } from '../write_concern'; -import { WriteConcern } from './../write_concern'; -import type { GridFSFile } from './download'; +import type { Document } from '../bson'; import type { GridFSBucket } from './index'; +import type { GridFSFile } from './download'; +import type { WriteConcernOptions } from '../write_concern'; +import type { Collection } from '../collection'; /** @public */ export interface GridFSChunk { diff --git a/src/operations/common_functions.ts b/src/operations/common_functions.ts index 5bf7cd24915..d835d35460c 100644 --- a/src/operations/common_functions.ts +++ b/src/operations/common_functions.ts @@ -1,10 +1,10 @@ +import { MongoTopologyClosedError } from '../error'; +import { Callback, getTopology } from '../utils'; import type { Document } from '../bson'; -import type { Collection } from '../collection'; import type { Db } from '../db'; -import { MongoTopologyClosedError } from '../error'; -import type { ReadPreference } from '../read_preference'; import type { ClientSession } from '../sessions'; -import { Callback, getTopology } from '../utils'; +import type { ReadPreference } from '../read_preference'; +import type { Collection } from '../collection'; /** @public */ export interface IndexInformationOptions { From aa033725d87d9bb4ea2720971607e020bd9ba8dc Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 26 Jul 2021 11:48:26 -0400 Subject: [PATCH 12/24] refactor(NODE-3405): replace MongoDriverError with MongoRuntime children --- src/change_stream.ts | 2 +- src/sdam/server.ts | 2 +- src/sdam/topology.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/change_stream.ts b/src/change_stream.ts index e3181e79377..23ac28d072c 100644 --- a/src/change_stream.ts +++ b/src/change_stream.ts @@ -58,7 +58,7 @@ const CHANGE_DOMAIN_TYPES = { 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'; +const CHANGESTREAM_CLOSED_ERROR = 'ChangeStream has been closed'; /** @public */ export interface ResumeOptions { diff --git a/src/sdam/server.ts b/src/sdam/server.ts index e821802176e..3e89bc54d5c 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -65,7 +65,7 @@ const stateTransition = makeStateMachine({ [STATE_CLOSING]: [STATE_CLOSING, STATE_CLOSED] }); -const SERVER_CLOSED_ERROR = 'Server is closed'; +const SERVER_CLOSED_ERROR = 'Server has been closed'; /** @internal */ const kMonitor = Symbol('monitor'); diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index 5dbd9a303c0..1da7cf1e87b 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -88,7 +88,7 @@ const stateTransition = makeStateMachine({ [STATE_CLOSING]: [STATE_CLOSING, STATE_CLOSED] }); -const TOPOLOGY_CLOSED_ERROR = 'Topology is closed'; +const TOPOLOGY_CLOSED_ERROR = 'Topology has been closed'; /** @internal */ const kCancelled = Symbol('cancelled'); @@ -988,7 +988,7 @@ function processWaitQueue(topology: Topology) { if (topology.s.state === STATE_CLOSED) { drainWaitQueue( topology[kWaitQueue], - new MongoTopologyClosedError(TOPOLOGY_CLOSED_ERROR + ', please connect') + new MongoTopologyClosedError(`${TOPOLOGY_CLOSED_ERROR}, please connect`) ); return; } From 74116f7a7d7ef209b833c362b04225a2e511c83d Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 26 Jul 2021 13:41:18 -0400 Subject: [PATCH 13/24] style(NODE-3405): refine TODOs --- src/cmap/commands.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmap/commands.ts b/src/cmap/commands.ts index 149a693c147..0ee6da3c86f 100644 --- a/src/cmap/commands.ts +++ b/src/cmap/commands.ts @@ -85,7 +85,7 @@ export class Query { // Validate that we are not passing 0x00 in the collection name if (ns.indexOf('\x00') !== -1) { // TODO(NODE-3483): Use MongoNamespace static method - throw new MongoDriverError('namespace cannot contain a null character'); + throw new MongoDriverError('Namespace cannot contain a null character'); } // Basic options From 673aa1b1ad702f9f5b98b666c760084c7e452986 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Mon, 26 Jul 2021 15:32:48 -0400 Subject: [PATCH 14/24] refactor(NODE-3405): add default msg to error classes --- src/sdam/server.ts | 10 ++++------ src/sdam/topology.ts | 2 +- src/sessions.ts | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 3e89bc54d5c..e3f1fb40965 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -65,8 +65,6 @@ const stateTransition = makeStateMachine({ [STATE_CLOSING]: [STATE_CLOSING, STATE_CLOSED] }); -const SERVER_CLOSED_ERROR = 'Server has been closed'; - /** @internal */ const kMonitor = Symbol('monitor'); @@ -294,7 +292,7 @@ export class Server extends TypedEventEmitter { } if (this.s.state === STATE_CLOSING || this.s.state === STATE_CLOSED) { - callback(new MongoServerClosedError(SERVER_CLOSED_ERROR)); + callback(new MongoServerClosedError()); return; } @@ -352,7 +350,7 @@ export class Server extends TypedEventEmitter { */ query(ns: MongoDBNamespace, cmd: Document, options: QueryOptions, callback: Callback): void { if (this.s.state === STATE_CLOSING || this.s.state === STATE_CLOSED) { - callback(new MongoServerClosedError(SERVER_CLOSED_ERROR)); + callback(new MongoServerClosedError()); return; } @@ -386,7 +384,7 @@ export class Server extends TypedEventEmitter { callback: Callback ): void { if (this.s.state === STATE_CLOSING || this.s.state === STATE_CLOSED) { - callback(new MongoServerClosedError(SERVER_CLOSED_ERROR)); + callback(new MongoServerClosedError()); return; } @@ -421,7 +419,7 @@ export class Server extends TypedEventEmitter { ): void { if (this.s.state === STATE_CLOSING || this.s.state === STATE_CLOSED) { if (typeof callback === 'function') { - callback(new MongoServerClosedError(SERVER_CLOSED_ERROR)); + callback(new MongoServerClosedError()); } return; diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index 1da7cf1e87b..1ec6c084e40 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -498,7 +498,7 @@ export class Topology extends TypedEventEmitter { stateTransition(this, STATE_CLOSING); - drainWaitQueue(this[kWaitQueue], new MongoTopologyClosedError(TOPOLOGY_CLOSED_ERROR)); + drainWaitQueue(this[kWaitQueue], new MongoTopologyClosedError()); drainTimerQueue(this.s.connectionTimers); if (this.s.srvPoller) { diff --git a/src/sessions.ts b/src/sessions.ts index 00621df8b05..6aacfc6a07f 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -960,7 +960,7 @@ export function applySession( ): MongoDriverError | undefined { // TODO: merge this with `assertAlive`, did not want to throw a try/catch here if (session.hasEnded) { - return new MongoExpiredSessionError('Attempted to use a session that has ended'); + return new MongoExpiredSessionError('Cannot use a session that has ended'); } const serverSession = session.serverSession; From 528412414b1a04e6187910319d0bf3cf43b3f99b Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 28 Jul 2021 14:01:25 -0400 Subject: [PATCH 15/24] refactor(NODE-3405): use MongoAPIError --- src/operations/execute_operation.ts | 2 +- src/sessions.ts | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts index bb23db16782..2777565c4a9 100644 --- a/src/operations/execute_operation.ts +++ b/src/operations/execute_operation.ts @@ -92,7 +92,7 @@ export function executeOperation< } else if (session.hasEnded) { return cb(new MongoExpiredSessionError('Use of expired sessions is not permitted')); } else if (session.snapshotEnabled && !topology.capabilities.supportsSnapshotReads) { - return cb(new MongoDriverError('Snapshot reads require MongoDB 5.0 or later')); + return cb(new MongoCompatibilityError('Snapshot reads require MongoDB 5.0 or later')); } } else if (session) { // If the user passed an explicit session and we are still, after server selection, diff --git a/src/sessions.ts b/src/sessions.ts index 6aacfc6a07f..c279743bd41 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -16,7 +16,8 @@ import { MongoServerError, AnyError, MongoExpiredSessionError, - MongoTransactionError + MongoTransactionError, + MongoAPIError } from './error'; import { now, @@ -372,12 +373,12 @@ export class ClientSession extends TypedEventEmitter { */ startTransaction(options?: TransactionOptions): void { if (this[kSnapshotEnabled]) { - throw new MongoTransactionError('Transactions are not allowed with snapshot sessions'); + throw new MongoCompatibilityError('Transactions are not allowed with snapshot sessions'); } assertAlive(this); if (this.inTransaction()) { - throw new MongoTransactionError('Transaction already in progress'); + throw new MongoAPIError('Transaction already in progress'); } if (this.isPinned && this.transaction.isCommitted) { From a638d0e391ae82bc6cb5d606d62c0479d4dd1255 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 10 Aug 2021 14:05:58 -0400 Subject: [PATCH 16/24] refactor(NODE-3405): use maybePromise in GridFS abort --- src/error.ts | 4 +-- src/gridfs/upload.ts | 45 ++++++++++----------------- test/functional/gridfs_stream.test.js | 24 ++++---------- 3 files changed, 25 insertions(+), 48 deletions(-) diff --git a/src/error.ts b/src/error.ts index 2e8c0a5d195..613a02dc84e 100644 --- a/src/error.ts +++ b/src/error.ts @@ -409,7 +409,7 @@ export class MongoCursorInUseError extends MongoAPIError { * @category Error */ export class MongoServerClosedError extends MongoAPIError { - constructor(message: string) { + constructor(message = 'Server has been closed') { super(message); } @@ -442,7 +442,7 @@ export class MongoCursorExhaustedError extends MongoAPIError { * @category Error */ export class MongoTopologyClosedError extends MongoAPIError { - constructor(message: string) { + constructor(message = 'Topology has been closed') { super(message); } diff --git a/src/gridfs/upload.ts b/src/gridfs/upload.ts index 1c7c61f7fa6..c280962fed7 100644 --- a/src/gridfs/upload.ts +++ b/src/gridfs/upload.ts @@ -1,15 +1,8 @@ import { Writable } from 'stream'; -import { - MongoError, - AnyError, - MongoAPIError, - MONGODB_ERROR_CODES, - MongoGridFSStreamError -} from '../error'; +import { MongoError, AnyError, MongoAPIError, MONGODB_ERROR_CODES } from '../error'; import { WriteConcern } from './../write_concern'; -import { PromiseProvider } from '../promise_provider'; import { ObjectId } from '../bson'; -import type { Callback } from '../utils'; +import { Callback, maybePromise } from '../utils'; import type { Document } from '../bson'; import type { GridFSBucket } from './index'; import type { GridFSFile } from './download'; @@ -149,27 +142,23 @@ export class GridFSBucketWriteStream extends Writable { abort(): Promise; abort(callback: Callback): void; abort(callback?: Callback): Promise | void { - const Promise = PromiseProvider.get(); - let error: MongoGridFSStreamError; - if (this.state.streamEnd) { - // TODO(NODE-3485): Replace with MongoGridFSStreamClosedError - error = new MongoAPIError('Cannot abort a stream that has already completed'); - if (typeof callback === 'function') { - return callback(error); + return maybePromise(callback, done => { + if (this.state.streamEnd) { + // TODO(NODE-3485): Replace with MongoGridFSStreamClosed + return done(new MongoAPIError('Cannot abort a stream that has already completed')); } - return Promise.reject(error); - } - if (this.state.aborted) { - // TODO(NODE-3485): Replace with MongoGridFSStreamClosedError - error = new MongoAPIError('Cannot call abort() on a stream twice'); - if (typeof callback === 'function') { - return callback(error); + + if (this.state.aborted) { + // TODO(NODE-3485): Replace with MongoGridFSStreamClosed + return done(new MongoAPIError('Cannot call abort() on a stream twice')); } - return Promise.reject(error); - } - this.state.aborted = true; - this.chunks.deleteMany({ files_id: this.id }, error => { - if (typeof callback === 'function') callback(error); + + this.state.aborted = true; + this.chunks.deleteMany({ files_id: this.id }, error => { + return done(error); + }); + + return done(); }); } diff --git a/test/functional/gridfs_stream.test.js b/test/functional/gridfs_stream.test.js index 446d16d529d..e23970e3c22 100644 --- a/test/functional/gridfs_stream.test.js +++ b/test/functional/gridfs_stream.test.js @@ -463,13 +463,9 @@ describe('GridFS Stream', function () { expect(error).to.not.exist; expect(c).to.equal(0); uploadStream.write('b', 'utf8', function (error) { - expect(error.toString()).to.equal( - 'MongoStreamClosedError: Stream has been aborted' - ); + expect(error.toString()).to.equal('MongoAPIError: Stream has been aborted'); uploadStream.end('c', 'utf8', function (error) { - expect(error.toString()).to.equal( - 'MongoStreamClosedError: Stream has been aborted' - ); + expect(error.toString()).to.equal('MongoAPIError: Stream has been aborted'); // Fail if user tries to abort an aborted stream uploadStream.abort().then(null, function (error) { expect(error.toString()).to.equal( @@ -517,13 +513,9 @@ describe('GridFS Stream', function () { expect(error).to.not.exist; expect(c).to.equal(0); uploadStream.write('b', 'utf8', function (error) { - expect(error.toString()).to.equal( - 'MongoStreamClosedError: Stream has been aborted' - ); + expect(error.toString()).to.equal('MongoAPIError: Stream has been aborted'); uploadStream.end('c', 'utf8', function (error) { - expect(error.toString()).to.equal( - 'MongoStreamClosedError: Stream has been aborted' - ); + expect(error.toString()).to.equal('MongoAPIError: Stream has been aborted'); // Fail if user tries to abort an aborted stream uploadStream.abort().then(null, function (error) { expect(error.toString()).to.equal( @@ -1174,14 +1166,10 @@ describe('GridFS Stream', function () { expect(c).to.equal(0); uploadStream.write('b', 'utf8', function (error) { - expect(error.toString()).to.equal( - 'MongoStreamClosedError: Stream has been aborted' - ); + expect(error.toString()).to.equal('MongoAPIError: Stream has been aborted'); uploadStream.end(function (error) { - expect(error.toString()).to.equal( - 'MongoStreamClosedError: Stream has been aborted' - ); + expect(error.toString()).to.equal('MongoAPIError: Stream has been aborted'); // Fail if user tries to abort an aborted stream uploadStream.abort().then(null, function (error) { From dda537ac62b154c470acc176a9b4308e95d4f0ee Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 10 Aug 2021 14:43:54 -0400 Subject: [PATCH 17/24] refactor(NODE-3405): use maybePromise in GridFS abort --- src/gridfs/upload.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/gridfs/upload.ts b/src/gridfs/upload.ts index c280962fed7..be0288a8a01 100644 --- a/src/gridfs/upload.ts +++ b/src/gridfs/upload.ts @@ -142,23 +142,19 @@ export class GridFSBucketWriteStream extends Writable { abort(): Promise; abort(callback: Callback): void; abort(callback?: Callback): Promise | void { - return maybePromise(callback, done => { + return maybePromise(callback, callback => { if (this.state.streamEnd) { // TODO(NODE-3485): Replace with MongoGridFSStreamClosed - return done(new MongoAPIError('Cannot abort a stream that has already completed')); + return callback(new MongoAPIError('Cannot abort a stream that has already completed')); } if (this.state.aborted) { // TODO(NODE-3485): Replace with MongoGridFSStreamClosed - return done(new MongoAPIError('Cannot call abort() on a stream twice')); + return callback(new MongoAPIError('Cannot call abort() on a stream twice')); } this.state.aborted = true; - this.chunks.deleteMany({ files_id: this.id }, error => { - return done(error); - }); - - return done(); + this.chunks.deleteMany({ files_id: this.id }, error => callback(error)); }); } From 3264b927716e14964ea249ba091c946b31e64108 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 10 Aug 2021 16:15:18 -0400 Subject: [PATCH 18/24] style(NODE-3405): add TODO comment to replace error --- src/sessions.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/sessions.ts b/src/sessions.ts index c279743bd41..cf847faf598 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -16,8 +16,7 @@ import { MongoServerError, AnyError, MongoExpiredSessionError, - MongoTransactionError, - MongoAPIError + MongoTransactionError } from './error'; import { now, @@ -378,7 +377,7 @@ export class ClientSession extends TypedEventEmitter { assertAlive(this); if (this.inTransaction()) { - throw new MongoAPIError('Transaction already in progress'); + throw new MongoTransactionError('Transaction already in progress'); } if (this.isPinned && this.transaction.isCommitted) { From 3f0f768b7a183ec9bee2550001fafe61d4cafaf8 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Tue, 10 Aug 2021 18:10:49 -0400 Subject: [PATCH 19/24] refactor(NODE-3405): add default message for MongoExpiredSessionError --- src/error.ts | 2 +- src/gridfs/upload.ts | 12 ++++++------ src/sdam/topology.ts | 7 +------ src/sessions.ts | 9 +++++---- src/utils.ts | 2 +- 5 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/error.ts b/src/error.ts index 613a02dc84e..0081f41eed9 100644 --- a/src/error.ts +++ b/src/error.ts @@ -294,7 +294,7 @@ export class MongoTransactionError extends MongoAPIError { * @category Error */ export class MongoExpiredSessionError extends MongoAPIError { - constructor(message: string) { + constructor(message = 'Cannot use a session that has ended') { super(message); } diff --git a/src/gridfs/upload.ts b/src/gridfs/upload.ts index be0288a8a01..5f3cad54d6b 100644 --- a/src/gridfs/upload.ts +++ b/src/gridfs/upload.ts @@ -1,13 +1,13 @@ import { Writable } from 'stream'; -import { MongoError, AnyError, MongoAPIError, MONGODB_ERROR_CODES } from '../error'; -import { WriteConcern } from './../write_concern'; +import type { Document } from '../bson'; import { ObjectId } from '../bson'; +import type { Collection } from '../collection'; +import { AnyError, MONGODB_ERROR_CODES, MongoError, MongoAPIError } from '../error'; import { Callback, maybePromise } from '../utils'; -import type { Document } from '../bson'; -import type { GridFSBucket } from './index'; -import type { GridFSFile } from './download'; import type { WriteConcernOptions } from '../write_concern'; -import type { Collection } from '../collection'; +import { WriteConcern } from './../write_concern'; +import type { GridFSFile } from './download'; +import type { GridFSBucket } from './index'; /** @public */ export interface GridFSChunk { diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index 1ec6c084e40..8edc47a7f8e 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -88,8 +88,6 @@ const stateTransition = makeStateMachine({ [STATE_CLOSING]: [STATE_CLOSING, STATE_CLOSED] }); -const TOPOLOGY_CLOSED_ERROR = 'Topology has been closed'; - /** @internal */ const kCancelled = Symbol('cancelled'); /** @internal */ @@ -986,10 +984,7 @@ function drainWaitQueue(queue: Denque, err?: MongoDriver function processWaitQueue(topology: Topology) { if (topology.s.state === STATE_CLOSED) { - drainWaitQueue( - topology[kWaitQueue], - new MongoTopologyClosedError(`${TOPOLOGY_CLOSED_ERROR}, please connect`) - ); + drainWaitQueue(topology[kWaitQueue], new MongoTopologyClosedError()); return; } diff --git a/src/sessions.ts b/src/sessions.ts index cf847faf598..a7ec82bf50e 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -16,7 +16,8 @@ import { MongoServerError, AnyError, MongoExpiredSessionError, - MongoTransactionError + MongoTransactionError, + MongoRuntimeError } from './error'; import { now, @@ -43,7 +44,7 @@ const minWireVersionForShardedTransactions = 8; function assertAlive(session: ClientSession, callback?: Callback): boolean { if (session.serverSession == null) { - const error = new MongoExpiredSessionError('Cannot use a session that has ended'); + const error = new MongoExpiredSessionError(); if (typeof callback === 'function') { callback(error); return false; @@ -444,7 +445,7 @@ export class ClientSession extends TypedEventEmitter { */ toBSON(): never { // TODO(NODE-3484): Replace with MongoBSONParseError - throw new MongoDriverError('ClientSession cannot be serialized to BSON.'); + throw new MongoRuntimeError('ClientSession cannot be serialized to BSON.'); } /** @@ -960,7 +961,7 @@ export function applySession( ): MongoDriverError | undefined { // TODO: merge this with `assertAlive`, did not want to throw a try/catch here if (session.hasEnded) { - return new MongoExpiredSessionError('Cannot use a session that has ended'); + return new MongoExpiredSessionError(); } const serverSession = session.serverSession; diff --git a/src/utils.ts b/src/utils.ts index 22efc31ce77..cd3c6fa43ca 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -262,7 +262,7 @@ export function executeLegacyOperation( const optionsIndex = args.length - 2; args[optionsIndex] = Object.assign({}, args[optionsIndex], { session: session }); } else if (opOptions.session && opOptions.session.hasEnded) { - throw new MongoExpiredSessionError('Use of expired sessions is not permitted'); + throw new MongoExpiredSessionError(); } } From 79c4da166d3b0109192b1cffbd1355e44e9c9fcd Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 11 Aug 2021 13:52:04 -0400 Subject: [PATCH 20/24] refactor(NODE-3405): revert use MongoAPIError and tidy up errmsgs --- src/change_stream.ts | 7 ++++--- src/cmap/connection_pool.ts | 11 ++++++++--- src/error.ts | 4 ++-- src/operations/common_functions.ts | 3 +-- src/sessions.ts | 1 - 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/change_stream.ts b/src/change_stream.ts index 23ac28d072c..17ef2cd4c92 100644 --- a/src/change_stream.ts +++ b/src/change_stream.ts @@ -58,7 +58,7 @@ const CHANGE_DOMAIN_TYPES = { 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 has been closed'; +const CHANGESTREAM_CLOSED_ERROR = 'ChangeStream is closed'; /** @public */ export interface ResumeOptions { @@ -558,8 +558,9 @@ function setIsEmitter(changeStream: ChangeStream): void { function setIsIterator(changeStream: ChangeStream): void { if (changeStream[kMode] === 'emitter') { + // TODO(NODE-3485): Replace with MongoChangeStreamModeError throw new MongoAPIError( - 'ChangeStream cannot be used as an EventEmitter after being used as an iterator' + 'ChangeStream cannot be used as an iterator after being used as an EventEmitter' ); } changeStream[kMode] = 'iterator'; @@ -683,7 +684,7 @@ function processNewChange( callback?: Callback> ) { if (changeStream[kClosed]) { - // TODO(NODE-3485): Replace with MongoChangeStreamStreamClosedError + // TODO(NODE-3485): Replace with MongoChangeStreamClosedError if (callback) callback(new MongoAPIError(CHANGESTREAM_CLOSED_ERROR)); return; } diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index cb5a637f480..a2c12ca75ad 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -5,7 +5,12 @@ import { Logger } from '../logger'; import { ConnectionPoolMetrics } from './metrics'; import { connect } from './connect'; import { eachAsync, makeCounter, Callback } from '../utils'; -import { MongoError, MongoInvalidArgumentError, MongoDriverError, MongoAPIError } from '../error'; +import { + MongoError, + MongoInvalidArgumentError, + MongoDriverError, + MongoRuntimeError +} from '../error'; import { PoolClosedError, WaitQueueTimeoutError } from './errors'; import { ConnectionPoolCreatedEvent, @@ -388,8 +393,8 @@ export class ConnectionPool extends TypedEventEmitter { clearTimeout(waitQueueMember.timer); } if (!waitQueueMember[kCancelled]) { - // TODO(NODE-3485): Replace with MongoConnectionPoolClosedError - waitQueueMember.callback(new MongoAPIError('Connection pool closed')); + // TODO(NODE-3483): Replace with MongoConnectionPoolClosedError + waitQueueMember.callback(new MongoRuntimeError('Connection pool closed')); } } } diff --git a/src/error.ts b/src/error.ts index 0081f41eed9..c41d0319179 100644 --- a/src/error.ts +++ b/src/error.ts @@ -409,7 +409,7 @@ export class MongoCursorInUseError extends MongoAPIError { * @category Error */ export class MongoServerClosedError extends MongoAPIError { - constructor(message = 'Server has been closed') { + constructor(message = 'Server is closed') { super(message); } @@ -442,7 +442,7 @@ export class MongoCursorExhaustedError extends MongoAPIError { * @category Error */ export class MongoTopologyClosedError extends MongoAPIError { - constructor(message = 'Topology has been closed') { + constructor(message = 'Topology is closed') { super(message); } diff --git a/src/operations/common_functions.ts b/src/operations/common_functions.ts index d835d35460c..f056f32a64a 100644 --- a/src/operations/common_functions.ts +++ b/src/operations/common_functions.ts @@ -41,8 +41,7 @@ export function indexInformation( const full = options.full == null ? false : options.full; // Did the user destroy the topology - if (getTopology(db).isDestroyed()) - return callback(new MongoTopologyClosedError('Topology was destroyed')); + if (getTopology(db).isDestroyed()) return callback(new MongoTopologyClosedError()); // Process all the results from the index command and collection function processResults(indexes: any) { // Contains all the information diff --git a/src/sessions.ts b/src/sessions.ts index a7ec82bf50e..0d6483a795b 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -444,7 +444,6 @@ export class ClientSession extends TypedEventEmitter { * This is here to ensure that ClientSession is never serialized to BSON. */ toBSON(): never { - // TODO(NODE-3484): Replace with MongoBSONParseError throw new MongoRuntimeError('ClientSession cannot be serialized to BSON.'); } From 7c56f63b848e591ce793fd6c4ee7a6ff6647ba1a Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 11 Aug 2021 22:04:28 -0400 Subject: [PATCH 21/24] test(NODE-3405): fix test waiting for the wrong error --- test/functional/change_stream.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/functional/change_stream.test.js b/test/functional/change_stream.test.js index 2e14a7a7d72..5508a0fbb18 100644 --- a/test/functional/change_stream.test.js +++ b/test/functional/change_stream.test.js @@ -1956,7 +1956,7 @@ describe('Change Streams', function () { return Promise.all([read(), write()]).then( () => Promise.reject(new Error('Expected operation to fail with error')), - err => expect(err.message).to.equal('ChangeStream has been closed') + err => expect(err.message).to.equal('ChangeStream is closed') ); } }); @@ -1976,7 +1976,7 @@ describe('Change Streams', function () { try { expect(err) .property('message') - .to.match(/ChangeStream has been closed/); + .to.match(/ChangeStream is closed/); Promise.all(ops).then(() => done(), done); } catch (e) { done(e); @@ -2887,7 +2887,7 @@ context('NODE-2626 - handle null changes without error', function () { changeStream.next((err, doc) => { expect(err).to.exist; expect(doc).to.not.exist; - expect(err.message).to.equal('ChangeStream has been closed'); + expect(err.message).to.equal('ChangeStream is closed'); changeStream.close(() => client.close(done)); }); }); From 47563f266d3bb7c07e77adf6f4212d49bb0fa930 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Wed, 11 Aug 2021 22:41:21 -0400 Subject: [PATCH 22/24] test(NODE-3405): fix Topology test waiting for an outdated error --- test/unit/sdam/server_selection/select_servers.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/sdam/server_selection/select_servers.test.js b/test/unit/sdam/server_selection/select_servers.test.js index 25b82722bff..e5637624e66 100644 --- a/test/unit/sdam/server_selection/select_servers.test.js +++ b/test/unit/sdam/server_selection/select_servers.test.js @@ -57,7 +57,7 @@ describe('selectServer', function () { topology.close(() => { topology.selectServer(ReadPreference.primary, { serverSelectionTimeoutMS: 2000 }, err => { expect(err).to.exist; - expect(err).to.match(/Topology has been closed/); + expect(err).to.match(/Topology is closed/); done(); }); }); From 719b3bfccf9960785678df28f6a089c67135585e Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 12 Aug 2021 16:52:25 -0400 Subject: [PATCH 23/24] style(NODE-3405): fix spelling in TODO comments and update use of MongoAPIError --- src/change_stream.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/change_stream.ts b/src/change_stream.ts index 17ef2cd4c92..9d195583dca 100644 --- a/src/change_stream.ts +++ b/src/change_stream.ts @@ -5,7 +5,8 @@ import { isResumableError, MongoDriverError, MongoAPIError, - MongoChangeStreamError + MongoChangeStreamError, + MongoRuntimeError } from './error'; import { AggregateOperation, AggregateOptions } from './operations/aggregate'; import { @@ -691,8 +692,8 @@ function processNewChange( // a null change means the cursor has been notified, implicitly closing the change stream if (change == null) { - // TODO(NODE-3485): Replace with MongoChangeStreamStreamClosedError - return closeWithError(changeStream, new MongoAPIError(CHANGESTREAM_CLOSED_ERROR), callback); + // TODO(NODE-3485): Replace with MongoChangeStreamClosedError + return closeWithError(changeStream, new MongoRuntimeError(CHANGESTREAM_CLOSED_ERROR), callback); } if (change && !change._id) { @@ -724,7 +725,7 @@ function processError( // If the change stream has been closed explicitly, do not process error. if (changeStream[kClosed]) { - // TODO(NODE-3485): Replace with MongoChangeStreamStreamClosedError + // TODO(NODE-3485): Replace with MongoChangeStreamClosedError if (callback) callback(new MongoAPIError(CHANGESTREAM_CLOSED_ERROR)); return; } @@ -785,7 +786,7 @@ function processError( */ function getCursor(changeStream: ChangeStream, callback: Callback>) { if (changeStream[kClosed]) { - // TODO(NODE-3485): Replace with MongoChangeStreamStreamClosedError + // TODO(NODE-3485): Replace with MongoChangeStreamClosedError callback(new MongoAPIError(CHANGESTREAM_CLOSED_ERROR)); return; } @@ -811,7 +812,7 @@ function processResumeQueue(changeStream: ChangeStream, err?: const request = changeStream[kResumeQueue].pop(); if (!err) { if (changeStream[kClosed]) { - // TODO(NODE-3485): Replace with MongoChangeStreamStreamClosedError + // TODO(NODE-3485): Replace with MongoChangeStreamClosedError request(new MongoAPIError(CHANGESTREAM_CLOSED_ERROR)); return; } From 5332a67a52dc1b05eae01290ef47b52b4be5dc14 Mon Sep 17 00:00:00 2001 From: Andy Mina Date: Thu, 12 Aug 2021 17:13:55 -0400 Subject: [PATCH 24/24] style(NODE-3405): unify default param format in error constructors --- src/error.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/error.ts b/src/error.ts index c41d0319179..9e90599119a 100644 --- a/src/error.ts +++ b/src/error.ts @@ -226,8 +226,8 @@ export class MongoRuntimeError extends MongoDriverError { * @category Error */ export class MongoBatchReExecutionError extends MongoAPIError { - constructor(message?: string) { - super(message || 'This batch has already been executed, create new batch to execute'); + constructor(message = 'This batch has already been executed, create new batch to execute') { + super(message); } get name(): string { @@ -343,8 +343,8 @@ export class MongoChangeStreamError extends MongoRuntimeError { * @category Error */ export class MongoTailableCursorError extends MongoAPIError { - constructor(message?: string) { - super(message || 'Tailable cursor does not support this operation'); + constructor(message = 'Tailable cursor does not support this operation') { + super(message); } get name(): string { @@ -392,8 +392,8 @@ export class MongoGridFSChunkError extends MongoRuntimeError { * @category Error */ export class MongoCursorInUseError extends MongoAPIError { - constructor(message?: string) { - super(message || 'Cursor is already initialized'); + constructor(message = 'Cursor is already initialized') { + super(message); } get name(): string {