fix(NODE-4108): improve return type for withTransaction() (#3236)

Author: nbbeeken

global.d.ts

@@ -36,6 +36,14 @@ declare global {
   }
 
   namespace Mocha {
+    interface SuiteFunction {
+      (title: string, metadata: MongoDBMetadataUI, fn: (this: Suite) => void): Mocha.Suite;
+    }
+
+    interface PendingSuiteFunction {
+      (title: string, metadata: MongoDBMetadataUI, fn: (this: Suite) => void): Mocha.Suite;
+    }
+
     interface TestFunction {
       (title: string, metadata: MongoDBMetadataUI, fn: Mocha.Func): Mocha.Test;
       (title: string, metadata: MongoDBMetadataUI, fn: Mocha.AsyncFunc): Mocha.Test;

src/sessions.ts

@@ -457,20 +457,30 @@ export class ClientSession extends TypedEventEmitter<ClientSessionEvents> {
   }
 
   /**
-   * Runs a provided lambda within a transaction, retrying either the commit operation
+   * Runs a provided callback within a transaction, retrying either the commitTransaction operation
    * or entire transaction as needed (and when the error permits) to better ensure that
    * the transaction can complete successfully.
    *
-   * IMPORTANT: This method requires the user to return a Promise, all lambdas that do not
-   * return a Promise will result in undefined behavior.
+   * **IMPORTANT:** This method requires the user to return a Promise, and `await` all operations.
+   * Any callbacks that do not return a Promise will result in undefined behavior.
    *
-   * @param fn - A lambda to run within a transaction
-   * @param options - Optional settings for the transaction
+   * @remarks
+   * This function:
+   * - Will return the command response from the final commitTransaction if every operation is successful (can be used as a truthy object)
+   * - Will return `undefined` if the transaction is explicitly aborted with `await session.abortTransaction()`
+   * - Will throw if one of the operations throws or `throw` statement is used inside the `withTransaction` callback
+   *
+   * Checkout a descriptive example here:
+   * @see https://www.mongodb.com/developer/quickstart/node-transactions/
+   *
+   * @param fn - callback to run within a transaction
+   * @param options - optional settings for the transaction
+   * @returns A raw command response or undefined
    */
   withTransaction<T = void>(
     fn: WithTransactionCallback<T>,
     options?: TransactionOptions
-  ): ReturnType<typeof fn> {
+  ): Promise<Document | undefined> {
     const startTime = now();
     return attemptTransaction(this, startTime, fn, options);
   }

src/transactions.ts

@@ -68,7 +68,7 @@ export interface TransactionOptions extends CommandOperationOptions {
   writeConcern?: WriteConcern;
   /** A default read preference for commands in this transaction */
   readPreference?: ReadPreference;
-
+  /** Specifies the maximum amount of time to allow a commit action on a transaction to run in milliseconds */
   maxCommitTimeMS?: number;
 }
 

test/integration/transactions/transactions.test.js renamed to test/integration/transactions/transactions.test.ts

@@ -1,21 +1,25 @@
-'use strict';
+import { expect } from 'chai';
 
-const { expect } = require('chai');
-const { Topology } = require('../../../src/sdam/topology');
-const { ClientSession } = require('../../../src/sessions');
-const { MongoNetworkError } = require('../../../src/error');
+import { Collection, MongoClient, ServerSessionPool } from '../../../src';
+import { MongoNetworkError } from '../../../src/error';
+import { ClientSession } from '../../../src/sessions';
 
 describe('Transactions', function () {
   describe('withTransaction', function () {
-    let session, sessionPool;
-    beforeEach(() => {
-      const topology = new Topology('localhost:27017');
+    let session: ClientSession;
+    let sessionPool: ServerSessionPool;
+    let client: MongoClient;
+
+    beforeEach(async function () {
+      client = this.configuration.newClient();
+      const topology = (await client.connect()).topology;
       sessionPool = topology.s.sessionPool;
-      session = new ClientSession(topology, sessionPool);
+      session = new ClientSession(topology, sessionPool, {});
     });
 
-    afterEach(() => {
+    afterEach(async () => {
       sessionPool.endAllPooledSessions();
+      await client.close();
     });
 
     it('should provide a useful error if a Promise is not returned', {
@@ -27,6 +31,7 @@ describe('Transactions', function () {
           return false;
         }
 
+        // @ts-expect-error: Testing that a non promise returning function is handled correctly
         expect(() => session.withTransaction(fnThatDoesntReturnPromise)).to.throw(
           /must return a Promise/
         );
@@ -37,8 +42,7 @@ describe('Transactions', function () {
 
     it('should return readable error if promise rejected with no reason', {
       metadata: {
-        requires: { topology: ['replicaset', 'sharded'], mongodb: '>=4.0.2' },
-        serverless: 'forbid'
+        requires: { topology: ['replicaset', 'sharded'], mongodb: '>=4.2.0', serverless: 'forbid' }
       },
       test: function (done) {
         function fnThatReturnsBadPromise() {
@@ -54,6 +58,73 @@ describe('Transactions', function () {
           });
       }
     });
+
+    describe(
+      'return value semantics',
+      { requires: { mongodb: '>=4.2.0', topology: '!single' } },
+      () => {
+        let client: MongoClient;
+        let collection: Collection<{ a: number }>;
+
+        beforeEach(async function () {
+          client = this.configuration.newClient();
+          await client.connect();
+          collection = await client
+            .db('withTransactionReturnType')
+            .createCollection('withTransactionReturnType');
+        });
+
+        afterEach(async function () {
+          await collection.drop();
+          await client.close();
+        });
+
+        it('should return undefined when transaction is aborted explicitly', async () => {
+          const session = client.startSession();
+
+          const withTransactionResult = await session
+            .withTransaction(async session => {
+              await collection.insertOne({ a: 1 }, { session });
+              await collection.findOne({ a: 1 }, { session });
+              await session.abortTransaction();
+            })
+            .finally(async () => await session.endSession());
+
+          expect(withTransactionResult).to.be.undefined;
+        });
+
+        it('should return raw command when transaction is successfully committed', async () => {
+          const session = client.startSession();
+
+          const withTransactionResult = await session
+            .withTransaction(async session => {
+              await collection.insertOne({ a: 1 }, { session });
+              await collection.findOne({ a: 1 }, { session });
+            })
+            .finally(async () => await session.endSession());
+
+          expect(withTransactionResult).to.exist;
+          expect(withTransactionResult).to.be.an('object');
+          expect(withTransactionResult).to.have.property('ok', 1);
+        });
+
+        it('should throw when transaction is aborted due to an error', async () => {
+          const session = client.startSession();
+
+          const withTransactionResult = await session
+            .withTransaction(async session => {
+              await collection.insertOne({ a: 1 }, { session });
+              await collection.findOne({ a: 1 }, { session });
+              throw new Error("I don't wanna transact anymore!");
+            })
+            .catch(error => error)
+            .finally(async () => await session.endSession());
+
+          expect(withTransactionResult).to.be.instanceOf(Error);
+          expect(withTransactionResult.message).to.equal("I don't wanna transact anymore!");
+        });
+      }
+    );
   });
 
   describe('startTransaction', function () {
@@ -137,7 +208,9 @@ describe('Transactions', function () {
 
                     coll.insertOne({ b: 2 }, { session }, err => {
                       expect(err).to.exist.and.to.be.an.instanceof(MongoNetworkError);
-                      expect(err.hasErrorLabel('TransientTransactionError')).to.be.true;
+                      if (err instanceof MongoNetworkError) {
+                        expect(err.hasErrorLabel('TransientTransactionError')).to.be.true;
+                      }
 
                       session.abortTransaction(() => session.endSession(() => client.close(done)));
                     });