From 450257894870a215ff60cfe315fdcbc2e3917ed3 Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Tue, 13 Sep 2022 12:53:10 -0400
Subject: [PATCH 01/12] refactor(NODE-4631): change_stream, gridfs to use
 maybeCallback

---
 src/change_stream.ts    |  99 +++++++++++++++-------------------
 src/gridfs/index.ts     |  72 ++++++++-----------------
 src/gridfs/upload.ts    |  12 ++---
 src/promise_provider.ts |   1 -
 src/utils.ts            |  29 ++++++++++
 test/unit/utils.test.ts | 115 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 216 insertions(+), 112 deletions(-)

diff --git a/src/change_stream.ts b/src/change_stream.ts
index e51f9adeb80..5887e3f338f 100644
--- a/src/change_stream.ts
+++ b/src/change_stream.ts
@@ -20,7 +20,7 @@ import type { AggregateOptions } from './operations/aggregate';
 import type { CollationOptions, OperationParent } from './operations/command';
 import type { ReadPreference } from './read_preference';
 import type { ServerSessionId } from './sessions';
-import { Callback, filterOptions, getTopology, maybePromise, MongoDBNamespace } from './utils';
+import { Callback, filterOptions, getTopology, maybeCallback, MongoDBNamespace } from './utils';
 
 /** @internal */
 const kCursorStream = Symbol('cursorStream');
@@ -649,29 +649,25 @@ export class ChangeStream<
   hasNext(callback: Callback<boolean>): void;
   hasNext(callback?: Callback): Promise<boolean> | void {
     this._setIsIterator();
-    // TOOD(NODE-4319): Add eslint rule preventing accidental variable shadowing
-    // Shadowing is intentional here.  We want to override the `callback` variable
-    // from the outer scope so that the inner scope doesn't accidentally call the wrong callback.
-    return maybePromise(callback, callback => {
-      (async () => {
+    return maybeCallback(async () => {
+      try {
+        const hasNext = await this.cursor.hasNext();
+        return hasNext;
+      } catch (error) {
         try {
+          await this._processErrorIteratorMode(error);
           const hasNext = await this.cursor.hasNext();
           return hasNext;
         } catch (error) {
           try {
-            await this._processErrorIteratorMode(error);
-            const hasNext = await this.cursor.hasNext();
-            return hasNext;
-          } catch (error) {
-            await this.close().catch(err => err);
-            throw error;
+            await this.close();
+          } catch {
+            // We are not concerned with errors from close()
           }
+          throw error;
         }
-      })().then(
-        hasNext => callback(undefined, hasNext),
-        error => callback(error)
-      );
-    });
+      }
+    }, callback);
   }
 
   /** Get the next available document from the Change Stream. */
@@ -680,31 +676,27 @@ export class ChangeStream<
   next(callback: Callback<TChange>): void;
   next(callback?: Callback<TChange>): Promise<TChange> | void {
     this._setIsIterator();
-    // TOOD(NODE-4319): Add eslint rule preventing accidental variable shadowing
-    // Shadowing is intentional here.  We want to override the `callback` variable
-    // from the outer scope so that the inner scope doesn't accidentally call the wrong callback.
-    return maybePromise(callback, callback => {
-      (async () => {
+    return maybeCallback(async () => {
+      try {
+        const change = await this.cursor.next();
+        const processedChange = this._processChange(change ?? null);
+        return processedChange;
+      } catch (error) {
         try {
+          await this._processErrorIteratorMode(error);
           const change = await this.cursor.next();
           const processedChange = this._processChange(change ?? null);
           return processedChange;
         } catch (error) {
           try {
-            await this._processErrorIteratorMode(error);
-            const change = await this.cursor.next();
-            const processedChange = this._processChange(change ?? null);
-            return processedChange;
-          } catch (error) {
-            await this.close().catch(err => err);
-            throw error;
+            await this.close();
+          } catch {
+            // We are not concerned with errors from close()
           }
+          throw error;
         }
-      })().then(
-        change => callback(undefined, change),
-        error => callback(error)
-      );
-    });
+      }
+    }, callback);
   }
 
   /**
@@ -715,29 +707,25 @@ export class ChangeStream<
   tryNext(callback: Callback<Document | null>): void;
   tryNext(callback?: Callback<Document | null>): Promise<Document | null> | void {
     this._setIsIterator();
-    // TOOD(NODE-4319): Add eslint rule preventing accidental variable shadowing
-    // Shadowing is intentional here.  We want to override the `callback` variable
-    // from the outer scope so that the inner scope doesn't accidentally call the wrong callback.
-    return maybePromise(callback, callback => {
-      (async () => {
+    return maybeCallback(async () => {
+      try {
+        const change = await this.cursor.tryNext();
+        return change ?? null;
+      } catch (error) {
         try {
+          await this._processErrorIteratorMode(error);
           const change = await this.cursor.tryNext();
           return change ?? null;
         } catch (error) {
           try {
-            await this._processErrorIteratorMode(error);
-            const change = await this.cursor.tryNext();
-            return change ?? null;
-          } catch (error) {
-            await this.close().catch(err => err);
-            throw error;
+            await this.close();
+          } catch {
+            // We are not concerned with errors from close()
           }
+          throw error;
         }
-      })().then(
-        change => callback(undefined, change),
-        error => callback(error)
-      );
-    });
+      }
+    }, callback);
   }
 
   /** Is the cursor closed */
@@ -752,13 +740,14 @@ export class ChangeStream<
   close(callback?: Callback): Promise<void> | void {
     this[kClosed] = true;
 
-    return maybePromise(callback, cb => {
+    return maybeCallback(async () => {
       const cursor = this.cursor;
-      return cursor.close(err => {
+      try {
+        await cursor.close();
+      } finally {
         this._endStream();
-        return cb(err);
-      });
-    });
+      }
+    }, callback);
   }
 
   /**
diff --git a/src/gridfs/index.ts b/src/gridfs/index.ts
index eb96cebca47..94aa748abf8 100644
--- a/src/gridfs/index.ts
+++ b/src/gridfs/index.ts
@@ -7,7 +7,7 @@ import type { Logger } from '../logger';
 import { Filter, TypedEventEmitter } from '../mongo_types';
 import type { ReadPreference } from '../read_preference';
 import type { Sort } from '../sort';
-import { Callback, maybePromise } from '../utils';
+import { Callback, maybeCallback } from '../utils';
 import { WriteConcern, WriteConcernOptions } from '../write_concern';
 import type { FindOptions } from './../operations/find';
 import {
@@ -144,28 +144,17 @@ export class GridFSBucket extends TypedEventEmitter<GridFSBucketEvents> {
   /** @deprecated Callbacks are deprecated and will be removed in the next major version. See [mongodb-legacy](https://github.com/mongodb-js/nodejs-mongodb-legacy) for migration assistance */
   delete(id: ObjectId, callback: Callback<void>): void;
   delete(id: ObjectId, callback?: Callback<void>): Promise<void> | void {
-    return maybePromise(callback, callback => {
-      return this.s._filesCollection.deleteOne({ _id: id }, (error, res) => {
-        if (error) {
-          return callback(error);
-        }
-
-        return this.s._chunksCollection.deleteMany({ files_id: id }, error => {
-          if (error) {
-            return callback(error);
-          }
-
-          // Delete orphaned chunks before returning FileNotFound
-          if (!res?.deletedCount) {
-            // TODO(NODE-3483): Replace with more appropriate error
-            // Consider creating new error MongoGridFSFileNotFoundError
-            return callback(new MongoRuntimeError(`File not found for id ${id}`));
-          }
-
-          return callback();
-        });
-      });
-    });
+    return maybeCallback(async () => {
+      const { deletedCount } = await this.s._filesCollection.deleteOne({ _id: id });
+      await this.s._chunksCollection.deleteMany({ files_id: id });
+
+      // Delete orphaned chunks before returning FileNotFound
+      if (deletedCount === 0) {
+        // TODO(NODE-3483): Replace with more appropriate error
+        // Consider creating new error MongoGridFSFileNotFoundError
+        throw new MongoRuntimeError(`File not found for id ${id}`);
+      }
+    }, callback);
   }
 
   /** Convenience wrapper around find on the files collection */
@@ -215,21 +204,14 @@ export class GridFSBucket extends TypedEventEmitter<GridFSBucketEvents> {
   /** @deprecated Callbacks are deprecated and will be removed in the next major version. See [mongodb-legacy](https://github.com/mongodb-js/nodejs-mongodb-legacy) for migration assistance */
   rename(id: ObjectId, filename: string, callback: Callback<void>): void;
   rename(id: ObjectId, filename: string, callback?: Callback<void>): Promise<void> | void {
-    return maybePromise(callback, callback => {
+    return maybeCallback(async () => {
       const filter = { _id: id };
       const update = { $set: { filename } };
-      return this.s._filesCollection.updateOne(filter, update, (error?, res?) => {
-        if (error) {
-          return callback(error);
-        }
-
-        if (!res?.matchedCount) {
-          return callback(new MongoRuntimeError(`File with id ${id} not found`));
-        }
-
-        return callback();
-      });
-    });
+      const { matchedCount } = await this.s._filesCollection.updateOne(filter, update);
+      if (matchedCount === 0) {
+        throw new MongoRuntimeError(`File with id ${id} not found`);
+      }
+    }, callback);
   }
 
   /** Removes this bucket's files collection, followed by its chunks collection. */
@@ -237,20 +219,10 @@ export class GridFSBucket extends TypedEventEmitter<GridFSBucketEvents> {
   /** @deprecated Callbacks are deprecated and will be removed in the next major version. See [mongodb-legacy](https://github.com/mongodb-js/nodejs-mongodb-legacy) for migration assistance */
   drop(callback: Callback<void>): void;
   drop(callback?: Callback<void>): Promise<void> | void {
-    return maybePromise(callback, callback => {
-      return this.s._filesCollection.drop(error => {
-        if (error) {
-          return callback(error);
-        }
-        return this.s._chunksCollection.drop(error => {
-          if (error) {
-            return callback(error);
-          }
-
-          return callback();
-        });
-      });
-    });
+    return maybeCallback(async () => {
+      await this.s._filesCollection.drop();
+      await this.s._chunksCollection.drop();
+    }, callback);
   }
 
   /** Get the Db scoped logger. */
diff --git a/src/gridfs/upload.ts b/src/gridfs/upload.ts
index 68d4ad224a9..7ccc3a17be6 100644
--- a/src/gridfs/upload.ts
+++ b/src/gridfs/upload.ts
@@ -4,7 +4,7 @@ import type { Document } from '../bson';
 import { ObjectId } from '../bson';
 import type { Collection } from '../collection';
 import { AnyError, MongoAPIError, MONGODB_ERROR_CODES, MongoError } from '../error';
-import { Callback, maybePromise } from '../utils';
+import { Callback, maybeCallback } from '../utils';
 import type { WriteConcernOptions } from '../write_concern';
 import { WriteConcern } from './../write_concern';
 import type { GridFSFile } from './download';
@@ -149,20 +149,20 @@ export class GridFSBucketWriteStream extends Writable implements NodeJS.Writable
   /** @deprecated Callbacks are deprecated and will be removed in the next major version. See [mongodb-legacy](https://github.com/mongodb-js/nodejs-mongodb-legacy) for migration assistance */
   abort(callback: Callback<void>): void;
   abort(callback?: Callback<void>): Promise<void> | void {
-    return maybePromise(callback, callback => {
+    return maybeCallback(async () => {
       if (this.state.streamEnd) {
         // TODO(NODE-3485): Replace with MongoGridFSStreamClosed
-        return callback(new MongoAPIError('Cannot abort a stream that has already completed'));
+        throw new MongoAPIError('Cannot abort a stream that has already completed');
       }
 
       if (this.state.aborted) {
         // TODO(NODE-3485): Replace with MongoGridFSStreamClosed
-        return callback(new MongoAPIError('Cannot call abort() on a stream twice'));
+        throw new MongoAPIError('Cannot call abort() on a stream twice');
       }
 
       this.state.aborted = true;
-      this.chunks.deleteMany({ files_id: this.id }, error => callback(error));
-    });
+      await this.chunks.deleteMany({ files_id: this.id });
+    }, callback);
   }
 
   /**
diff --git a/src/promise_provider.ts b/src/promise_provider.ts
index eb9a28c756e..d26c8ca9fe7 100644
--- a/src/promise_provider.ts
+++ b/src/promise_provider.ts
@@ -38,7 +38,6 @@ export class PromiseProvider {
       store[kPromise] = null;
       return;
     }
-
     if (!PromiseProvider.validate(lib)) {
       // validate
       return;
diff --git a/src/utils.ts b/src/utils.ts
index f871df54a7c..af198897f41 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -435,6 +435,35 @@ export function* makeCounter(seed = 0): Generator<number> {
   }
 }
 
+/**
+ * Helper for handling legacy callback support.
+ */
+export function maybeCallback<T>(
+  promiseFn: () => Promise<T>,
+  callback?: Callback<T>
+): Promise<T> | void {
+  const PromiseConstructor = PromiseProvider.get();
+
+  const promise = promiseFn();
+  if (callback == null && PromiseConstructor == null) {
+    return promise;
+  }
+
+  if (PromiseConstructor != null) {
+    return new PromiseConstructor((resolve, reject) => {
+      promise.then(resolve, reject);
+    });
+  }
+
+  promise.then(
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    result => callback!(undefined, result),
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    error => callback!(error)
+  );
+  return;
+}
+
 /**
  * Helper function for either accepting a callback, or returning a promise
  * @internal
diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts
index db4e589bc62..d6ba6cabba4 100644
--- a/test/unit/utils.test.ts
+++ b/test/unit/utils.test.ts
@@ -2,11 +2,13 @@ import { expect } from 'chai';
 
 import { LEGACY_HELLO_COMMAND } from '../../src/constants';
 import { MongoRuntimeError } from '../../src/error';
+import { Promise as PromiseProvider } from '../../src/index';
 import {
   BufferPool,
   eachAsync,
   HostAddress,
   isHello,
+  maybeCallback,
   MongoDBNamespace,
   shuffle
 } from '../../src/utils';
@@ -350,4 +352,117 @@ describe('driver utils', function () {
       });
     });
   });
+
+  describe('maybeCallback()', () => {
+    it('should accept to two arguments', () => {
+      expect(maybeCallback).to.have.lengthOf(2);
+    });
+
+    describe('when handling an error case', () => {
+      it('should pass the error to the callback provided', done => {
+        const superPromiseRejection = Promise.reject(new Error('fail'));
+        const result = maybeCallback(
+          () => superPromiseRejection,
+          (error, result) => {
+            try {
+              expect(result).to.not.exist;
+              expect(error).to.be.instanceOf(Error);
+              return done();
+            } catch (assertionError) {
+              return done(assertionError);
+            }
+          }
+        );
+        expect(result).to.be.undefined;
+      });
+
+      it('should return the rejected promise to the caller when no callback is provided', async () => {
+        const superPromiseRejection = Promise.reject(new Error('fail'));
+        const returnedPromise = maybeCallback(() => superPromiseRejection, undefined);
+        expect(returnedPromise).to.equal(superPromiseRejection);
+        // @ts-expect-error: There is no overload to change the return type not be nullish,
+        // and we do not want to add one in fear of making it too easy to neglect adding the callback argument
+        const thrownError = await returnedPromise.catch(error => error);
+        expect(thrownError).to.be.instanceOf(Error);
+      });
+
+      it('should not modify a rejection error promise', async () => {
+        class MyError extends Error {}
+        const driverError = Object.freeze(new MyError());
+        const rejection = Promise.reject(driverError);
+        // @ts-expect-error: There is no overload to change the return type not be nullish,
+        // and we do not want to add one in fear of making it too easy to neglect adding the callback argument
+        const thrownError = await maybeCallback(() => rejection, undefined).catch(error => error);
+        expect(thrownError).to.be.equal(driverError);
+      });
+
+      it('should not modify a rejection error when passed to callback', done => {
+        class MyError extends Error {}
+        const driverError = Object.freeze(new MyError());
+        const rejection = Promise.reject(driverError);
+        maybeCallback(
+          () => rejection,
+          error => {
+            try {
+              expect(error).to.exist;
+              expect(error).to.equal(driverError);
+              done();
+            } catch (assertionError) {
+              done(assertionError);
+            }
+          }
+        );
+      });
+    });
+
+    describe('when handling a success case', () => {
+      it('should pass the result and undefined error to the callback provided', done => {
+        const superPromiseSuccess = Promise.resolve(2);
+
+        const result = maybeCallback(
+          () => superPromiseSuccess,
+          (error, result) => {
+            try {
+              expect(error).to.be.undefined;
+              expect(result).to.equal(2);
+              done();
+            } catch (assertionError) {
+              done(assertionError);
+            }
+          }
+        );
+        expect(result).to.be.undefined;
+      });
+
+      it('should return the resolved promise to the caller when no callback is provided', async () => {
+        const superPromiseSuccess = Promise.resolve(2);
+        const result = maybeCallback(() => superPromiseSuccess);
+        expect(result).to.equal(superPromiseSuccess);
+        expect(await result).to.equal(2);
+      });
+    });
+
+    describe('when a custom promise constructor is set', () => {
+      class CustomPromise {
+        then() {
+          // do nothing
+        }
+      }
+
+      beforeEach(() => {
+        PromiseProvider.set(CustomPromise as unknown as PromiseConstructor);
+      });
+
+      afterEach(() => {
+        PromiseProvider.set(null);
+      });
+
+      it('should return the custom promise if no callback is provided', async () => {
+        const superPromiseSuccess = Promise.resolve(2);
+        const result = maybeCallback(() => superPromiseSuccess);
+        expect(result).to.not.equal(superPromiseSuccess);
+        expect(result).to.be.instanceOf(CustomPromise);
+      });
+    });
+  });
 });

From 44b6d0c939e0862f4db507c6d2364ef877483a46 Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Tue, 13 Sep 2022 14:02:56 -0400
Subject: [PATCH 02/12] fix: callback with custom lib

---
 src/utils.ts            | 22 ++++++++++------------
 test/unit/utils.test.ts | 11 +++++++++++
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/src/utils.ts b/src/utils.ts
index af198897f41..80ff0bb5c98 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -445,21 +445,19 @@ export function maybeCallback<T>(
   const PromiseConstructor = PromiseProvider.get();
 
   const promise = promiseFn();
-  if (callback == null && PromiseConstructor == null) {
-    return promise;
-  }
-
-  if (PromiseConstructor != null) {
-    return new PromiseConstructor((resolve, reject) => {
-      promise.then(resolve, reject);
-    });
+  if (callback == null) {
+    if (PromiseConstructor == null) {
+      return promise;
+    } else {
+      return new PromiseConstructor((resolve, reject) => {
+        promise.then(resolve, reject);
+      });
+    }
   }
 
   promise.then(
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    result => callback!(undefined, result),
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    error => callback!(error)
+    result => callback(undefined, result),
+    error => callback(error)
   );
   return;
 }
diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts
index d6ba6cabba4..7da24c95ee3 100644
--- a/test/unit/utils.test.ts
+++ b/test/unit/utils.test.ts
@@ -463,6 +463,17 @@ describe('driver utils', function () {
         expect(result).to.not.equal(superPromiseSuccess);
         expect(result).to.be.instanceOf(CustomPromise);
       });
+
+      it('should return void event if a custom promise is set and a callback is provided', async () => {
+        const superPromiseSuccess = Promise.resolve(2);
+        const result = maybeCallback(
+          () => superPromiseSuccess,
+          () => {
+            // ignore
+          }
+        );
+        expect(result).to.be.undefined;
+      });
     });
   });
 });

From 9883042166b838318518a5923c00eac485872282 Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Tue, 13 Sep 2022 14:09:21 -0400
Subject: [PATCH 03/12] test: add error case for custom promise

---
 test/unit/utils.test.ts | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts
index 7da24c95ee3..6a25dfd6c52 100644
--- a/test/unit/utils.test.ts
+++ b/test/unit/utils.test.ts
@@ -1,3 +1,4 @@
+import { Promise as BluebirdPromise } from 'bluebird';
 import { expect } from 'chai';
 
 import { LEGACY_HELLO_COMMAND } from '../../src/constants';
@@ -443,14 +444,9 @@ describe('driver utils', function () {
     });
 
     describe('when a custom promise constructor is set', () => {
-      class CustomPromise {
-        then() {
-          // do nothing
-        }
-      }
-
       beforeEach(() => {
-        PromiseProvider.set(CustomPromise as unknown as PromiseConstructor);
+        // @ts-expect-error: Bluebird does not have type info
+        PromiseProvider.set(BluebirdPromise);
       });
 
       afterEach(() => {
@@ -461,7 +457,17 @@ describe('driver utils', function () {
         const superPromiseSuccess = Promise.resolve(2);
         const result = maybeCallback(() => superPromiseSuccess);
         expect(result).to.not.equal(superPromiseSuccess);
-        expect(result).to.be.instanceOf(CustomPromise);
+        expect(result).to.be.instanceOf(BluebirdPromise);
+      });
+
+      it('should return a rejected custom promise instance if promiseFn rejects', async () => {
+        const superPromiseFailure = Promise.reject(new Error('ah!'));
+        const result = maybeCallback(() => superPromiseFailure);
+        expect(result).to.not.equal(superPromiseFailure);
+        expect(result).to.be.instanceOf(BluebirdPromise);
+        // @ts-expect-error: There is no overload to change the return type not be nullish,
+        // and we do not want to add one in fear of making it too easy to neglect adding the callback argument
+        expect(await result.catch(e => e)).to.have.property('message', 'ah!');
       });
 
       it('should return void event if a custom promise is set and a callback is provided', async () => {

From fe69c856d10b27f9b86081357fc821486cc7a672 Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Tue, 13 Sep 2022 14:52:29 -0400
Subject: [PATCH 04/12] fixes

---
 src/bulk/common.ts  | 78 ++++++++++++++++++++-------------------------
 src/gridfs/index.ts |  3 +-
 src/mongo_client.ts | 33 ++++++++-----------
 3 files changed, 50 insertions(+), 64 deletions(-)

diff --git a/src/bulk/common.ts b/src/bulk/common.ts
index 9cd81370cf6..0acc9683647 100644
--- a/src/bulk/common.ts
+++ b/src/bulk/common.ts
@@ -31,6 +31,7 @@ import {
   Callback,
   getTopology,
   hasAtomicOperators,
+  maybeCallback,
   MongoDBNamespace,
   resolveOptions
 } from '../utils';
@@ -1270,39 +1271,44 @@ export abstract class BulkOperationBase {
     options?: BulkWriteOptions | Callback<BulkWriteResult>,
     callback?: Callback<BulkWriteResult>
   ): Promise<BulkWriteResult> | void {
-    if (typeof options === 'function') (callback = options), (options = {});
-    options = options ?? {};
-
-    if (this.s.executed) {
-      return handleEarlyError(new MongoBatchReExecutionError(), callback);
-    }
+    callback =
+      typeof callback === 'function'
+        ? callback
+        : typeof options === 'function'
+        ? options
+        : undefined;
+
+    return maybeCallback(async () => {
+      options = options != null && typeof options !== 'function' ? options : {};
+
+      if (this.s.executed) {
+        throw new MongoBatchReExecutionError();
+      }
 
-    const writeConcern = WriteConcern.fromOptions(options);
-    if (writeConcern) {
-      this.s.writeConcern = writeConcern;
-    }
+      const writeConcern = WriteConcern.fromOptions(options);
+      if (writeConcern) {
+        this.s.writeConcern = writeConcern;
+      }
 
-    // If we have current batch
-    if (this.isOrdered) {
-      if (this.s.currentBatch) this.s.batches.push(this.s.currentBatch);
-    } else {
-      if (this.s.currentInsertBatch) this.s.batches.push(this.s.currentInsertBatch);
-      if (this.s.currentUpdateBatch) this.s.batches.push(this.s.currentUpdateBatch);
-      if (this.s.currentRemoveBatch) this.s.batches.push(this.s.currentRemoveBatch);
-    }
-    // If we have no operations in the bulk raise an error
-    if (this.s.batches.length === 0) {
-      const emptyBatchError = new MongoInvalidArgumentError(
-        'Invalid BulkOperation, Batch cannot be empty'
-      );
-      return handleEarlyError(emptyBatchError, callback);
-    }
+      // If we have current batch
+      if (this.isOrdered) {
+        if (this.s.currentBatch) this.s.batches.push(this.s.currentBatch);
+      } else {
+        if (this.s.currentInsertBatch) this.s.batches.push(this.s.currentInsertBatch);
+        if (this.s.currentUpdateBatch) this.s.batches.push(this.s.currentUpdateBatch);
+        if (this.s.currentRemoveBatch) this.s.batches.push(this.s.currentRemoveBatch);
+      }
+      // If we have no operations in the bulk raise an error
+      if (this.s.batches.length === 0) {
+        throw new MongoInvalidArgumentError('Invalid BulkOperation, Batch cannot be empty');
+      }
 
-    this.s.executed = true;
-    const finalOptions = { ...this.s.options, ...options };
-    const operation = new BulkWriteShimOperation(this, finalOptions);
+      this.s.executed = true;
+      const finalOptions = { ...this.s.options, ...options };
+      const operation = new BulkWriteShimOperation(this, finalOptions);
 
-    return executeOperation(this.s.collection.s.db.s.client, operation, callback);
+      return await executeOperation(this.s.collection.s.db.s.client, operation);
+    }, callback);
   }
 
   /**
@@ -1351,20 +1357,6 @@ Object.defineProperty(BulkOperationBase.prototype, 'length', {
   }
 });
 
-/** helper function to assist with promiseOrCallback behavior */
-function handleEarlyError(
-  err?: AnyError,
-  callback?: Callback<BulkWriteResult>
-): Promise<BulkWriteResult> | void {
-  if (typeof callback === 'function') {
-    callback(err);
-    return;
-  }
-
-  const PromiseConstructor = PromiseProvider.get() ?? Promise;
-  return PromiseConstructor.reject(err);
-}
-
 function shouldForceServerObjectId(bulkOperation: BulkOperationBase): boolean {
   if (typeof bulkOperation.s.options.forceServerObjectId === 'boolean') {
     return bulkOperation.s.options.forceServerObjectId;
diff --git a/src/gridfs/index.ts b/src/gridfs/index.ts
index 94aa748abf8..874762b0132 100644
--- a/src/gridfs/index.ts
+++ b/src/gridfs/index.ts
@@ -146,9 +146,10 @@ export class GridFSBucket extends TypedEventEmitter<GridFSBucketEvents> {
   delete(id: ObjectId, callback?: Callback<void>): Promise<void> | void {
     return maybeCallback(async () => {
       const { deletedCount } = await this.s._filesCollection.deleteOne({ _id: id });
-      await this.s._chunksCollection.deleteMany({ files_id: id });
 
       // Delete orphaned chunks before returning FileNotFound
+      await this.s._chunksCollection.deleteMany({ files_id: id });
+
       if (deletedCount === 0) {
         // TODO(NODE-3483): Replace with more appropriate error
         // Consider creating new error MongoGridFSFileNotFoundError
diff --git a/src/mongo_client.ts b/src/mongo_client.ts
index 321b8a63fb5..b103a875821 100644
--- a/src/mongo_client.ts
+++ b/src/mongo_client.ts
@@ -29,6 +29,7 @@ import {
   Callback,
   ClientMetadata,
   HostAddress,
+  maybeCallback,
   maybePromise,
   MongoDBNamespace,
   ns,
@@ -580,26 +581,18 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> {
     options?: MongoClientOptions | Callback<MongoClient>,
     callback?: Callback<MongoClient>
   ): Promise<MongoClient> | void {
-    if (typeof options === 'function') (callback = options), (options = {});
-    options = options ?? {};
-
-    try {
-      // Create client
-      const mongoClient = new MongoClient(url, options);
-      // Execute the connect method
-      if (callback) {
-        return mongoClient.connect(callback);
-      } else {
-        return mongoClient.connect();
-      }
-    } catch (error) {
-      if (callback) {
-        return callback(error);
-      } else {
-        const PromiseConstructor = PromiseProvider.get() ?? Promise;
-        return PromiseConstructor.reject(error);
-      }
-    }
+    callback =
+      typeof callback === 'function'
+        ? callback
+        : typeof options === 'function'
+        ? options
+        : undefined;
+
+    return maybeCallback(async () => {
+      options = typeof options !== 'function' ? options : undefined;
+      const client = new this(url, options);
+      return await client.connect();
+    }, callback);
   }
 
   /** Starts a new session on the server */

From c232a1965e95795d7e99ff1d58a893aab67057e3 Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Tue, 13 Sep 2022 17:15:32 -0400
Subject: [PATCH 05/12] lint

---
 src/bulk/common.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/bulk/common.ts b/src/bulk/common.ts
index 0acc9683647..66f0b410029 100644
--- a/src/bulk/common.ts
+++ b/src/bulk/common.ts
@@ -22,7 +22,6 @@ import { executeOperation } from '../operations/execute_operation';
 import { InsertOperation } from '../operations/insert';
 import { AbstractOperation, Hint } from '../operations/operation';
 import { makeUpdateStatement, UpdateOperation, UpdateStatement } from '../operations/update';
-import { PromiseProvider } from '../promise_provider';
 import type { Server } from '../sdam/server';
 import type { Topology } from '../sdam/topology';
 import type { ClientSession } from '../sessions';

From 3acfb6d6ab43b8df953f4141b61711934c4e17ea Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Tue, 13 Sep 2022 17:25:05 -0400
Subject: [PATCH 06/12] fix: withSession

---
 src/mongo_client.ts | 16 +++++++---------
 src/utils.ts        |  5 +++++
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/mongo_client.ts b/src/mongo_client.ts
index b103a875821..0a8bace22cb 100644
--- a/src/mongo_client.ts
+++ b/src/mongo_client.ts
@@ -642,16 +642,14 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> {
     }
 
     const session = this.startSession(options);
-    const PromiseConstructor = PromiseProvider.get() ?? Promise;
-
-    return PromiseConstructor.resolve()
-      .then(() => withSessionCallback(session))
-      .then(() => {
-        // Do not return the result of callback
-      })
-      .finally(() => {
+
+    return maybeCallback(async () => {
+      try {
+        await withSessionCallback(session);
+      } finally {
         session.endSession().catch(() => null);
-      });
+      }
+    }, null);
   }
 
   /**
diff --git a/src/utils.ts b/src/utils.ts
index 80ff0bb5c98..b2321d3a8c3 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -438,9 +438,14 @@ export function* makeCounter(seed = 0): Generator<number> {
 /**
  * Helper for handling legacy callback support.
  */
+export function maybeCallback<T>(promiseFn: () => Promise<T>, callback: null): Promise<T>;
 export function maybeCallback<T>(
   promiseFn: () => Promise<T>,
   callback?: Callback<T>
+): Promise<T> | void;
+export function maybeCallback<T>(
+  promiseFn: () => Promise<T>,
+  callback?: Callback<T> | null
 ): Promise<T> | void {
   const PromiseConstructor = PromiseProvider.get();
 

From 2ab77fb3826a9f94ba6c985559b6e16e3013ca23 Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Tue, 13 Sep 2022 17:53:03 -0400
Subject: [PATCH 07/12] fix: bulk

---
 src/bulk/common.ts                            | 53 ++++++++++---------
 .../sessions/sessions.spec.prose.test.ts      |  2 +-
 2 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/bulk/common.ts b/src/bulk/common.ts
index 66f0b410029..21e19f15d12 100644
--- a/src/bulk/common.ts
+++ b/src/bulk/common.ts
@@ -1276,38 +1276,41 @@ export abstract class BulkOperationBase {
         : typeof options === 'function'
         ? options
         : undefined;
+    options = options != null && typeof options !== 'function' ? options : {};
 
-    return maybeCallback(async () => {
-      options = options != null && typeof options !== 'function' ? options : {};
-
-      if (this.s.executed) {
+    if (this.s.executed) {
+      // eslint-disable-next-line @typescript-eslint/require-await
+      return maybeCallback(async () => {
         throw new MongoBatchReExecutionError();
-      }
+      }, callback);
+    }
 
-      const writeConcern = WriteConcern.fromOptions(options);
-      if (writeConcern) {
-        this.s.writeConcern = writeConcern;
-      }
+    const writeConcern = WriteConcern.fromOptions(options);
+    if (writeConcern) {
+      this.s.writeConcern = writeConcern;
+    }
 
-      // If we have current batch
-      if (this.isOrdered) {
-        if (this.s.currentBatch) this.s.batches.push(this.s.currentBatch);
-      } else {
-        if (this.s.currentInsertBatch) this.s.batches.push(this.s.currentInsertBatch);
-        if (this.s.currentUpdateBatch) this.s.batches.push(this.s.currentUpdateBatch);
-        if (this.s.currentRemoveBatch) this.s.batches.push(this.s.currentRemoveBatch);
-      }
-      // If we have no operations in the bulk raise an error
-      if (this.s.batches.length === 0) {
+    // If we have current batch
+    if (this.isOrdered) {
+      if (this.s.currentBatch) this.s.batches.push(this.s.currentBatch);
+    } else {
+      if (this.s.currentInsertBatch) this.s.batches.push(this.s.currentInsertBatch);
+      if (this.s.currentUpdateBatch) this.s.batches.push(this.s.currentUpdateBatch);
+      if (this.s.currentRemoveBatch) this.s.batches.push(this.s.currentRemoveBatch);
+    }
+    // If we have no operations in the bulk raise an error
+    if (this.s.batches.length === 0) {
+      // eslint-disable-next-line @typescript-eslint/require-await
+      return maybeCallback(async () => {
         throw new MongoInvalidArgumentError('Invalid BulkOperation, Batch cannot be empty');
-      }
+      }, callback);
+    }
 
-      this.s.executed = true;
-      const finalOptions = { ...this.s.options, ...options };
-      const operation = new BulkWriteShimOperation(this, finalOptions);
+    this.s.executed = true;
+    const finalOptions = { ...this.s.options, ...options };
+    const operation = new BulkWriteShimOperation(this, finalOptions);
 
-      return await executeOperation(this.s.collection.s.db.s.client, operation);
-    }, callback);
+    return executeOperation(this.s.collection.s.db.s.client, operation, callback);
   }
 
   /**
diff --git a/test/integration/sessions/sessions.spec.prose.test.ts b/test/integration/sessions/sessions.spec.prose.test.ts
index 870c017c2e6..d55b0dea2b8 100644
--- a/test/integration/sessions/sessions.spec.prose.test.ts
+++ b/test/integration/sessions/sessions.spec.prose.test.ts
@@ -46,6 +46,6 @@ describe('ServerSession', () => {
     expect(events).to.have.lengthOf(operations.length);
 
     // This is a guarantee in node, unless you are performing a transaction (which is not being done in this test)
-    expect(new Set(events.map(ev => ev.command.lsid.id.toString('hex'))).size).to.equal(1);
+    expect(new Set(events.map(ev => ev.command.lsid.id.toString('hex')))).to.have.lengthOf(1);
   });
 });

From 894a380416fc9fcf2616003d99b27a2626283f12 Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Tue, 13 Sep 2022 18:56:23 -0400
Subject: [PATCH 08/12] lint

---
 src/mongo_client.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/mongo_client.ts b/src/mongo_client.ts
index 0a8bace22cb..37811cfc224 100644
--- a/src/mongo_client.ts
+++ b/src/mongo_client.ts
@@ -17,7 +17,6 @@ import { MongoInvalidArgumentError } from './error';
 import type { Logger, LoggerLevel } from './logger';
 import { TypedEventEmitter } from './mongo_types';
 import { connect } from './operations/connect';
-import { PromiseProvider } from './promise_provider';
 import type { ReadConcern, ReadConcernLevel, ReadConcernLike } from './read_concern';
 import { ReadPreference, ReadPreferenceMode } from './read_preference';
 import type { TagSet } from './sdam/server_description';

From f50dc36fba07d7de593e6088d4fed82b8bd499c6 Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Wed, 14 Sep 2022 10:22:20 -0400
Subject: [PATCH 09/12] clean up endSessions silencing

---
 src/mongo_client.ts | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/mongo_client.ts b/src/mongo_client.ts
index 37811cfc224..979f2a21253 100644
--- a/src/mongo_client.ts
+++ b/src/mongo_client.ts
@@ -646,7 +646,11 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> {
       try {
         await withSessionCallback(session);
       } finally {
-        session.endSession().catch(() => null);
+        try {
+          await session.endSession();
+        } catch {
+          // We are not concerned with errors from endSession()
+        }
       }
     }, null);
   }

From d9b5794e7a62667c6a49602eeb6a6abe6d612132 Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Wed, 14 Sep 2022 13:27:15 -0400
Subject: [PATCH 10/12] rm change in promise_provider

---
 src/promise_provider.ts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/promise_provider.ts b/src/promise_provider.ts
index d26c8ca9fe7..eb9a28c756e 100644
--- a/src/promise_provider.ts
+++ b/src/promise_provider.ts
@@ -38,6 +38,7 @@ export class PromiseProvider {
       store[kPromise] = null;
       return;
     }
+
     if (!PromiseProvider.validate(lib)) {
       // validate
       return;

From 7b7322e8b057e7000cd3670b6a7040c7383394f0 Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Fri, 16 Sep 2022 13:29:53 -0400
Subject: [PATCH 11/12] suggestion(daria): fixed!

Co-authored-by: Daria Pardue <daria.pardue@mongodb.com>
---
 test/unit/utils.test.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts
index 6a25dfd6c52..4def18f8d71 100644
--- a/test/unit/utils.test.ts
+++ b/test/unit/utils.test.ts
@@ -470,7 +470,7 @@ describe('driver utils', function () {
         expect(await result.catch(e => e)).to.have.property('message', 'ah!');
       });
 
-      it('should return void event if a custom promise is set and a callback is provided', async () => {
+      it('should return void even if a custom promise is set and a callback is provided', async () => {
         const superPromiseSuccess = Promise.resolve(2);
         const result = maybeCallback(
           () => superPromiseSuccess,

From bf668c2c2cd723b14d8f7a3cf93aec1219adc037 Mon Sep 17 00:00:00 2001
From: Neal Beeken <neal.beeken@mongodb.com>
Date: Fri, 16 Sep 2022 15:03:11 -0400
Subject: [PATCH 12/12] fix: test name

---
 test/unit/utils.test.ts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts
index 4def18f8d71..2dd0a687309 100644
--- a/test/unit/utils.test.ts
+++ b/test/unit/utils.test.ts
@@ -355,7 +355,7 @@ describe('driver utils', function () {
   });
 
   describe('maybeCallback()', () => {
-    it('should accept to two arguments', () => {
+    it('should accept two arguments', () => {
       expect(maybeCallback).to.have.lengthOf(2);
     });
 
@@ -445,7 +445,6 @@ describe('driver utils', function () {
 
     describe('when a custom promise constructor is set', () => {
       beforeEach(() => {
-        // @ts-expect-error: Bluebird does not have type info
         PromiseProvider.set(BluebirdPromise);
       });