From 0abd248fc07ebca0f3dcb63bc36c70562d7d5f84 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 21 Dec 2022 10:43:17 -0500 Subject: [PATCH 1/3] refactor(NODE-4906): remove internal callback usage --- src/cursor/abstract_cursor.ts | 18 ++++++++++----- src/encrypter.ts | 7 +++++- src/operations/bulk_write.ts | 13 ++++------- src/operations/collections.ts | 36 +++++++++++++++-------------- src/operations/common_functions.ts | 15 +++++++----- src/operations/indexes.ts | 28 +++++++++++++--------- src/operations/is_capped.ts | 20 ++++++++-------- src/operations/options_operation.ts | 19 ++++++++------- 8 files changed, 89 insertions(+), 67 deletions(-) diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts index 8aa59aa651d..7bd899cfdd4 100644 --- a/src/cursor/abstract_cursor.ts +++ b/src/cursor/abstract_cursor.ts @@ -853,7 +853,10 @@ function cleanupCursor( if (session) { if (session.owner === cursor) { - return session.endSession({ error }, callback); + session.endSession({ error }).finally(() => { + callback(); + }); + return; } if (!session.inTransaction()) { @@ -867,10 +870,11 @@ function cleanupCursor( function completeCleanup() { if (session) { if (session.owner === cursor) { - return session.endSession({ error }, () => { + session.endSession({ error }).finally(() => { cursor.emit(AbstractCursor.CLOSE); callback(); }); + return; } if (!session.inTransaction()) { @@ -884,11 +888,13 @@ function cleanupCursor( cursor[kKilled] = true; - return executeOperation( + executeOperation( cursor[kClient], - new KillCursorsOperation(cursorId, cursorNs, server, { session }), - completeCleanup - ); + new KillCursorsOperation(cursorId, cursorNs, server, { session }) + ).finally(() => { + completeCleanup(); + }); + return; } /** @internal */ diff --git a/src/encrypter.ts b/src/encrypter.ts index 8fe18aa3675..ee95f0ba1bf 100644 --- a/src/encrypter.ts +++ b/src/encrypter.ts @@ -1,4 +1,5 @@ /* eslint-disable @typescript-eslint/no-var-requires */ + import { deserialize, serialize } from './bson'; import { MONGO_CLIENT_EVENTS } from './constants'; import type { AutoEncrypter, AutoEncryptionOptions } from './deps'; @@ -114,7 +115,11 @@ export class Encrypter { this.autoEncrypter.teardown(!!force, e => { const internalClient = this[kInternalClient]; if (internalClient != null && client !== internalClient) { - return internalClient.close(force, callback); + internalClient.close(force).then( + () => callback(), + error => callback(error) + ); + return; } callback(e); }); diff --git a/src/operations/bulk_write.ts b/src/operations/bulk_write.ts index 627dc709a70..f56fd8c5a17 100644 --- a/src/operations/bulk_write.ts +++ b/src/operations/bulk_write.ts @@ -52,15 +52,10 @@ export class BulkWriteOperation extends AbstractOperation { } // Execute the bulk - bulk.execute({ ...options, session }, (err, r) => { - // We have connection level error - if (!r && err) { - return callback(err); - } - - // Return the results - callback(undefined, r); - }); + bulk.execute({ ...options, session }).then( + result => callback(undefined, result), + error => callback(error) + ); } } diff --git a/src/operations/collections.ts b/src/operations/collections.ts index 8b314865a92..14a1ccbb3c7 100644 --- a/src/operations/collections.ts +++ b/src/operations/collections.ts @@ -25,24 +25,26 @@ export class CollectionsOperation extends AbstractOperation { session: ClientSession | undefined, callback: Callback ): void { - const db = this.db; - // Let's get the collection names - db.listCollections( - {}, - { ...this.options, nameOnly: true, readPreference: this.readPreference, session } - ).toArray((err, documents) => { - if (err || !documents) return callback(err); - // Filter collections removing any illegal ones - documents = documents.filter(doc => doc.name.indexOf('$') === -1); - - // Return the collection objects - callback( - undefined, - documents.map(d => { - return new Collection(db, d.name, db.s.options); - }) + this.db + .listCollections( + {}, + { ...this.options, nameOnly: true, readPreference: this.readPreference, session } + ) + .toArray() + .then( + documents => { + const collections = []; + for (const { name } of documents) { + if (!name.includes('$')) { + // Filter collections removing any illegal ones + collections.push(new Collection(this.db, name, this.db.s.options)); + } + } + // Return the collection objects + callback(undefined, collections); + }, + error => callback(error) ); - }); } } diff --git a/src/operations/common_functions.ts b/src/operations/common_functions.ts index a439aca7eb4..63f7e1f2525 100644 --- a/src/operations/common_functions.ts +++ b/src/operations/common_functions.ts @@ -69,12 +69,15 @@ export function indexInformation( // Get the list of indexes of the specified collection db.collection(name) .listIndexes(options) - .toArray((err, indexes) => { - if (err) return callback(err); - if (!Array.isArray(indexes)) return callback(undefined, []); - if (full) return callback(undefined, indexes); - callback(undefined, processResults(indexes)); - }); + .toArray() + .then( + indexes => { + if (!Array.isArray(indexes)) return callback(undefined, []); + if (full) return callback(undefined, indexes); + callback(undefined, processResults(indexes)); + }, + error => callback(error) + ); } export function prepareDocs( diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 3a3b72dc30c..c3e7a9f14b0 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -1,7 +1,12 @@ import type { Document } from '../bson'; import type { Collection } from '../collection'; import type { Db } from '../db'; -import { MongoCompatibilityError, MONGODB_ERROR_CODES, MongoServerError } from '../error'; +import { + MongoCompatibilityError, + MONGODB_ERROR_CODES, + MongoError, + MongoServerError +} from '../error'; import type { OneOrMore } from '../mongo_types'; import { ReadPreference } from '../read_preference'; import type { Server } from '../sdam/server'; @@ -320,22 +325,23 @@ export class EnsureIndexOperation extends CreateIndexOperation { override execute(server: Server, session: ClientSession | undefined, callback: Callback): void { const indexName = this.indexes[0].name; const cursor = this.db.collection(this.collectionName).listIndexes({ session }); - cursor.toArray((err, indexes) => { - /// ignore "NamespaceNotFound" errors - if (err && (err as MongoServerError).code !== MONGODB_ERROR_CODES.NamespaceNotFound) { - return callback(err); - } - - if (indexes) { + cursor.toArray().then( + indexes => { indexes = Array.isArray(indexes) ? indexes : [indexes]; if (indexes.some(index => index.name === indexName)) { callback(undefined, indexName); return; } + super.execute(server, session, callback); + }, + error => { + if (error instanceof MongoError && error.code !== MONGODB_ERROR_CODES.NamespaceNotFound) { + // ignore "NamespaceNotFound" errors + return callback(error); + } + super.execute(server, session, callback); } - - super.execute(server, session, callback); - }); + ); } } diff --git a/src/operations/is_capped.ts b/src/operations/is_capped.ts index 938c72f41cf..1219e79cf72 100644 --- a/src/operations/is_capped.ts +++ b/src/operations/is_capped.ts @@ -28,15 +28,17 @@ export class IsCappedOperation extends AbstractOperation { { name: coll.collectionName }, { ...this.options, nameOnly: false, readPreference: this.readPreference, session } ) - .toArray((err, collections) => { - if (err || !collections) return callback(err); - if (collections.length === 0) { - // TODO(NODE-3485) - return callback(new MongoAPIError(`collection ${coll.namespace} not found`)); - } + .toArray() + .then( + collections => { + if (collections.length === 0) { + // TODO(NODE-3485) + return callback(new MongoAPIError(`collection ${coll.namespace} not found`)); + } - const collOptions = collections[0].options; - callback(undefined, !!(collOptions && collOptions.capped)); - }); + callback(undefined, !!collections[0].options?.capped); + }, + error => callback(error) + ); } } diff --git a/src/operations/options_operation.ts b/src/operations/options_operation.ts index 27853d34edb..9c053938bf3 100644 --- a/src/operations/options_operation.ts +++ b/src/operations/options_operation.ts @@ -29,14 +29,17 @@ export class OptionsOperation extends AbstractOperation { { name: coll.collectionName }, { ...this.options, nameOnly: false, readPreference: this.readPreference, session } ) - .toArray((err, collections) => { - if (err || !collections) return callback(err); - if (collections.length === 0) { - // TODO(NODE-3485) - return callback(new MongoAPIError(`collection ${coll.namespace} not found`)); - } + .toArray() + .then( + collections => { + if (collections.length === 0) { + // TODO(NODE-3485) + return callback(new MongoAPIError(`collection ${coll.namespace} not found`)); + } - callback(err, collections[0].options); - }); + callback(undefined, collections[0].options); + }, + error => callback(error) + ); } } From f84bd8279605fee5fc311e8f77207d83c95b55e8 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 21 Dec 2022 12:16:04 -0500 Subject: [PATCH 2/3] fix: lint --- src/operations/indexes.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index c3e7a9f14b0..43fc3740edf 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -1,12 +1,7 @@ import type { Document } from '../bson'; import type { Collection } from '../collection'; import type { Db } from '../db'; -import { - MongoCompatibilityError, - MONGODB_ERROR_CODES, - MongoError, - MongoServerError -} from '../error'; +import { MongoCompatibilityError, MONGODB_ERROR_CODES, MongoError } from '../error'; import type { OneOrMore } from '../mongo_types'; import { ReadPreference } from '../read_preference'; import type { Server } from '../sdam/server'; From 6b27699be4c0472ed17f27be20cf7f5ba9ab990f Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 21 Dec 2022 12:20:27 -0500 Subject: [PATCH 3/3] fix: error check --- src/operations/indexes.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 43fc3740edf..8d095e16501 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -330,11 +330,11 @@ export class EnsureIndexOperation extends CreateIndexOperation { super.execute(server, session, callback); }, error => { - if (error instanceof MongoError && error.code !== MONGODB_ERROR_CODES.NamespaceNotFound) { + if (error instanceof MongoError && error.code === MONGODB_ERROR_CODES.NamespaceNotFound) { // ignore "NamespaceNotFound" errors - return callback(error); + return super.execute(server, session, callback); } - super.execute(server, session, callback); + return callback(error); } ); }