fix(mongo-adapter): abort a transaction whose commit did not complete (#11602)

Co-authored-by: Gustavo Valverde <g.valverde02@gmail.com>

.changeset/mongo-abort-failed-commit.md

@@ -0,0 +1,5 @@
+---
+"@better-auth/mongo-adapter": patch
+---
+
+When you pass `client` to `mongodbAdapter`, a transaction whose commit fails before it reaches the server, for example because the commit timed out, is now aborted on the server. Before, the transaction stayed open until MongoDB's `transactionLifetimeLimitSeconds` expired (60 seconds by default), and other writes to the same documents waited or failed with write conflicts. The original commit error is still the one your code receives.

packages/mongo-adapter/src/mongodb-adapter.test.ts

@@ -1,5 +1,11 @@
 import type { Db } from "mongodb";
-import { MongoClient, MongoServerError, ObjectId, UUID } from "mongodb";
+import {
+	MongoClient,
+	MongoErrorLabel,
+	MongoServerError,
+	ObjectId,
+	UUID,
+} from "mongodb";
 import { describe, expect, it, vi } from "vitest";
 import { mongodbAdapter } from "./mongodb-adapter";
 
@@ -89,6 +95,136 @@ describe("mongodb-adapter", () => {
 		await client.close();
 	});
 
+	/**
+	 * @see https://github.com/better-auth/better-auth/issues/11554
+	 */
+	it("aborts a transaction whose commit did not complete", async () => {
+		const client = new MongoClient("mongodb://localhost:27017");
+		const session = client.startSession();
+		vi.spyOn(client, "startSession").mockReturnValue(session);
+		const commitError = new MongoServerError({
+			errmsg: "commit did not complete",
+		});
+		const commit = vi
+			.spyOn(session, "commitTransaction")
+			.mockRejectedValue(commitError);
+		// Once a commit has been attempted the driver stops reporting the session
+		// as being in a transaction, even when the commit never reached the server.
+		vi.spyOn(session, "inTransaction").mockReturnValue(false);
+		const abort = vi.spyOn(session, "abortTransaction");
+		const end = vi.spyOn(session, "endSession");
+		const db = client.db("test");
+		const command = vi.fn().mockResolvedValue({ ok: 1 });
+		vi.spyOn(client, "db").mockReturnValue({ command } as unknown as Db);
+		const adapter = mongodbAdapter(db, { client })({});
+		const transaction = adapter.options?.adapterConfig.transaction;
+		if (typeof transaction !== "function") {
+			throw new Error("MongoDB transaction is not configured");
+		}
+
+		await expect(transaction(async () => "ok")).rejects.toBe(commitError);
+		expect(commit).toHaveBeenCalledOnce();
+		expect(abort).not.toHaveBeenCalled();
+		expect(command).toHaveBeenCalledWith({ abortTransaction: 1 }, { session });
+		expect(end).toHaveBeenCalledOnce();
+		await client.close();
+	});
+
+	it("preserves the commit error when the transaction is already gone", async () => {
+		const client = new MongoClient("mongodb://localhost:27017");
+		const session = client.startSession();
+		vi.spyOn(client, "startSession").mockReturnValue(session);
+		const commitError = new MongoServerError({
+			errmsg: "commit did not complete",
+		});
+		vi.spyOn(session, "commitTransaction").mockRejectedValue(commitError);
+		vi.spyOn(session, "inTransaction").mockReturnValue(false);
+		const end = vi.spyOn(session, "endSession");
+		const db = client.db("test");
+		// NoSuchTransaction: the server has nothing left open, so the abort
+		// failing must not replace the error the caller needs to see.
+		const command = vi.fn().mockRejectedValue(
+			Object.assign(new MongoServerError({ errmsg: "no such transaction" }), {
+				code: 251,
+			}),
+		);
+		vi.spyOn(client, "db").mockReturnValue({ command } as unknown as Db);
+		const adapter = mongodbAdapter(db, { client })({});
+		const transaction = adapter.options?.adapterConfig.transaction;
+		if (typeof transaction !== "function") {
+			throw new Error("MongoDB transaction is not configured");
+		}
+
+		await expect(transaction(async () => "ok")).rejects.toBe(commitError);
+		expect(command).toHaveBeenCalledOnce();
+		expect(end).toHaveBeenCalledOnce();
+		await client.close();
+	});
+
+	it("does not send an abort when the failure came before the commit", async () => {
+		const client = new MongoClient("mongodb://localhost:27017");
+		const session = client.startSession();
+		vi.spyOn(client, "startSession").mockReturnValue(session);
+		const commit = vi.spyOn(session, "commitTransaction");
+		// A transaction that never started leaves nothing open on the server, and
+		// reports itself the same way a failed commit does.
+		vi.spyOn(session, "inTransaction").mockReturnValue(false);
+		const abort = vi.spyOn(session, "abortTransaction");
+		const end = vi.spyOn(session, "endSession");
+		const db = client.db("test");
+		const command = vi.fn().mockResolvedValue({ ok: 1 });
+		vi.spyOn(client, "db").mockReturnValue({ command } as unknown as Db);
+		const adapter = mongodbAdapter(db, { client })({});
+		const transaction = adapter.options?.adapterConfig.transaction;
+		if (typeof transaction !== "function") {
+			throw new Error("MongoDB transaction is not configured");
+		}
+
+		const callbackError = new Error("the work failed");
+		await expect(
+			transaction(async () => {
+				throw callbackError;
+			}),
+		).rejects.toBe(callbackError);
+		expect(commit).not.toHaveBeenCalled();
+		expect(abort).not.toHaveBeenCalled();
+		expect(command).not.toHaveBeenCalled();
+		expect(end).toHaveBeenCalledOnce();
+		await client.close();
+	});
+
+	it("retries the abort once when it fails with a retryable write", async () => {
+		const client = new MongoClient("mongodb://localhost:27017");
+		const session = client.startSession();
+		vi.spyOn(client, "startSession").mockReturnValue(session);
+		const commitError = new MongoServerError({
+			errmsg: "commit did not complete",
+		});
+		vi.spyOn(session, "commitTransaction").mockRejectedValue(commitError);
+		vi.spyOn(session, "inTransaction").mockReturnValue(false);
+		const end = vi.spyOn(session, "endSession");
+		const db = client.db("test");
+		// The driver retries `abortTransaction()` itself on a transient error, so
+		// the raw command has to do the same or the transaction stays open.
+		const retryable = new MongoServerError({ errmsg: "connection reset" });
+		retryable.addErrorLabel(MongoErrorLabel.RetryableWriteError);
+		const command = vi
+			.fn()
+			.mockRejectedValueOnce(retryable)
+			.mockResolvedValueOnce({ ok: 1 });
+		vi.spyOn(client, "db").mockReturnValue({ command } as unknown as Db);
+		const adapter = mongodbAdapter(db, { client })({});
+		const transaction = adapter.options?.adapterConfig.transaction;
+		if (typeof transaction !== "function") {
+			throw new Error("MongoDB transaction is not configured");
+		}
+
+		await expect(transaction(async () => "ok")).rejects.toBe(commitError);
+		expect(command).toHaveBeenCalledTimes(2);
+		expect(end).toHaveBeenCalledOnce();
+		await client.close();
+	});
+
 	it("creates configured compound indexes before the first write", async () => {
 		let resolveIndexSetup: (indexName: string) => void = () => {};
 		const indexSetup = new Promise<string>((resolve) => {

packages/mongo-adapter/src/mongodb-adapter.ts

@@ -8,8 +8,9 @@ import type {
 } from "@better-auth/core/db/adapter";
 import { createAdapterFactory } from "@better-auth/core/db/adapter";
 import { resolveDatabaseTableIndexes } from "@better-auth/core/db/internal";
+import { logger } from "@better-auth/core/env";
 import type { ClientSession, Db, MongoClient } from "mongodb";
-import { ObjectId, UUID } from "mongodb";
+import { MongoError, MongoErrorLabel, ObjectId, UUID } from "mongodb";
 import {
 	escapeForMongoRegex,
 	insensitiveContains,
@@ -31,6 +32,52 @@ class MongoAdapterError extends Error {
 	}
 }
 
+/**
+ * NoSuchTransaction (251) and TransactionCommitted (256) both mean the server
+ * has nothing left open, so the abort has already achieved what it was for.
+ */
+const ABORT_SETTLED_CODES = new Set([251, 256]);
+
+/**
+ * Abort a transaction whose commit did not complete, as a command on its own
+ * session.
+ *
+ * `session.abortTransaction()` is retried by the driver on a transient error;
+ * a raw command is not, and giving up after one attempt would leave the
+ * transaction open until `transactionLifetimeLimitSeconds` — the leak this is
+ * here to close. So a retryable write is tried once more.
+ *
+ * The abort is best-effort either way: the caller's original error is the one
+ * that must surface, so a failure here is only logged.
+ */
+async function abortTransactionCommand(
+	client: MongoClient,
+	session: ClientSession,
+): Promise<void> {
+	for (let attempt = 0; ; attempt++) {
+		try {
+			await client.db("admin").command({ abortTransaction: 1 }, { session });
+			return;
+		} catch (abortError) {
+			const code = (abortError as { code?: number })?.code;
+			if (code !== undefined && ABORT_SETTLED_CODES.has(code)) {
+				return;
+			}
+			const retryable =
+				abortError instanceof MongoError &&
+				abortError.hasErrorLabel(MongoErrorLabel.RetryableWriteError);
+			if (retryable && attempt === 0) {
+				continue;
+			}
+			logger.error(
+				"Failed to abort a MongoDB transaction whose commit did not complete",
+				abortError,
+			);
+			return;
+		}
+	}
+}
+
 export interface MongoDBAdapterConfig {
 	/**
 	 * MongoDB client instance
@@ -764,6 +811,7 @@ export const mongodbAdapter = (
 							}
 
 							const session = config.client.startSession();
+							let commitAttempted = false;
 
 							try {
 								session.startTransaction();
@@ -778,11 +826,25 @@ export const mongodbAdapter = (
 
 								const result = await cb(adapter);
 
+								commitAttempted = true;
 								await session.commitTransaction();
 								return result;
 							} catch (err) {
 								if (session.inTransaction()) {
 									await session.abortTransaction();
+								} else if (commitAttempted) {
+									// A commit that failed in flight leaves the transaction
+									// open on the server, but the driver no longer reports the
+									// session as being in one, so `abortTransaction()` is not
+									// an option. Send the abort as a command on the same
+									// session so the server does not hold the writes until
+									// `transactionLifetimeLimitSeconds`.
+									//
+									// Only a failed commit can leave that state behind: if the
+									// transaction never started, or failed before the commit,
+									// there is nothing open and sending the command would just
+									// produce a second error to log.
+									await abortTransactionCommand(config.client, session);
 								}
 								throw err;
 							} finally {