diff --git a/.changeset/mongo-abort-failed-commit.md b/.changeset/mongo-abort-failed-commit.md new file mode 100644 index 0000000000..7187916419 --- /dev/null +++ b/.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. diff --git a/packages/mongo-adapter/src/mongodb-adapter.test.ts b/packages/mongo-adapter/src/mongodb-adapter.test.ts index d699285f8f..f1d7b7e5e5 100644 --- a/packages/mongo-adapter/src/mongodb-adapter.test.ts +++ b/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((resolve) => { diff --git a/packages/mongo-adapter/src/mongodb-adapter.ts b/packages/mongo-adapter/src/mongodb-adapter.ts index 1d1409737b..db54fea29c 100644 --- a/packages/mongo-adapter/src/mongodb-adapter.ts +++ b/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 { + 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 {