From 90127ff2a1d46b554f1d3cbf855a0fa3a6f14420 Mon Sep 17 00:00:00 2001 From: Bereket Engida Date: Thu, 3 Sep 2026 22:14:45 -0700 Subject: [PATCH] fix(core): make consumeOne and incrementOne optional on custom adapters 1.7.0 made both methods required on CustomAdapter and removed the factory fallback, so a custom adapter written for 1.6 type-checks against nothing and throws at request time on the first OTP, magic link, password reset, or two-factor attempt. Restore the fallbacks. consumeOne without a native implementation runs findOne then an id-guarded deleteMany and wins only when exactly one row was deleted. incrementOne runs a bounded compare-and-swap loop: findOne, then updateMany guarded on the id and the counter values just read, retried when a concurrent writer changed the row first. Both are single guarded statements per attempt, so they stay atomic across processes without transactions. --- .changeset/optional-atomic-adapter-methods.md | 5 + .../content/docs/guides/1-7-upgrade-guide.mdx | 6 +- .../docs/guides/create-a-db-adapter.mdx | 8 +- packages/core/src/db/adapter/factory.test.ts | 144 +++++++++++++++++- packages/core/src/db/adapter/factory.ts | 110 ++++++++++--- packages/core/src/db/adapter/index.ts | 12 +- 6 files changed, 252 insertions(+), 33 deletions(-) create mode 100644 .changeset/optional-atomic-adapter-methods.md diff --git a/.changeset/optional-atomic-adapter-methods.md b/.changeset/optional-atomic-adapter-methods.md new file mode 100644 index 0000000000..9752da0236 --- /dev/null +++ b/.changeset/optional-atomic-adapter-methods.md @@ -0,0 +1,5 @@ +--- +"@better-auth/core": patch +--- + +Make `consumeOne` and `incrementOne` optional on custom adapters again. When an adapter omits them, the factory falls back to `findOne` plus an id-guarded `deleteMany`, or a compare-and-swap `updateMany` loop, instead of throwing at request time. diff --git a/docs/content/docs/guides/1-7-upgrade-guide.mdx b/docs/content/docs/guides/1-7-upgrade-guide.mdx index b31802aa2b..4ad0a7cbb6 100644 --- a/docs/content/docs/guides/1-7-upgrade-guide.mdx +++ b/docs/content/docs/guides/1-7-upgrade-guide.mdx @@ -707,11 +707,11 @@ Two OAuth-provider behaviors read the incoming request origin. Behind a custom s The atomic-state work introduces required methods. If you use only the built-in adapters and storage, you can skip this. -### Database adapters must implement `incrementOne` and `consumeOne` +### Database adapters can implement `incrementOne` and `consumeOne` -`incrementOne` updates one row's counter atomically and returns the row, or null when the guard did not match. `consumeOne` reads and deletes a row in one step for single-use credentials. Both are now required, and the old fallback is gone. +`incrementOne` updates one row's counter atomically and returns the row, or null when the guard did not match. `consumeOne` reads and deletes a row in one step for single-use credentials. Both are optional. When a custom adapter omits them, Better Auth falls back to `findOne` plus an id-guarded `deleteMany` or a compare-and-swap `updateMany`, which stay atomic across processes because each is a single guarded statement. -**What to do:** implement both `incrementOne` and `consumeOne` in any custom adapter. A missing `consumeOne` throws at runtime. All built-in adapters already do. +**What to do:** nothing is required. Implement both natively in a custom adapter to save a round trip on hot paths. All built-in adapters already do. ### Secondary storage must implement `increment` and `getAndDelete` diff --git a/docs/content/docs/guides/create-a-db-adapter.mdx b/docs/content/docs/guides/create-a-db-adapter.mdx index b1cebf2588..344bb8e568 100644 --- a/docs/content/docs/guides/create-a-db-adapter.mdx +++ b/docs/content/docs/guides/create-a-db-adapter.mdx @@ -271,8 +271,8 @@ consumeOne: async ({ model, where }) => { }; ``` - - `consumeOne` is required. Better Auth does not provide a `findMany` plus `deleteMany` fallback because that shape cannot guarantee single-use semantics across adapters without a native atomic delete-and-return operation. + + `consumeOne` is optional. When it is missing, Better Auth falls back to `findOne` followed by a `deleteMany` guarded on the row id, and treats the caller as the winner only when exactly one row was deleted. Implement it natively to save a round trip. ### `incrementOne` method @@ -302,8 +302,8 @@ incrementOne: async ({ model, where, increment, set }) => { }; ``` - - `incrementOne` is required. Better Auth uses it as a compare-and-swap primitive: the `where` clause is part of the guard, so concurrent requests cannot all update a stale snapshot. + + `incrementOne` is optional. When it is missing, Better Auth falls back to a compare-and-swap loop: `findOne`, then `updateMany` guarded on the row id and the counter values it just read, retried when a concurrent writer got there first. Implement it natively for one round trip under contention. ### `findOne` method diff --git a/packages/core/src/db/adapter/factory.test.ts b/packages/core/src/db/adapter/factory.test.ts index 34fc10e3ab..e349aa3d46 100644 --- a/packages/core/src/db/adapter/factory.test.ts +++ b/packages/core/src/db/adapter/factory.test.ts @@ -209,10 +209,55 @@ describe("createAdapterFactory atomic primitives", () => { ).rejects.toThrow(/resolved to an empty update/); }); - it("throws a clear error when consumeOne is missing at runtime", async () => { + it("falls back to findOne plus id-guarded deleteMany when consumeOne is missing", async () => { + const deleteCalls: CleanedWhere[][] = []; const adapter = createTestAdapter({ adapter: createCustomAdapter({ - consumeOne: undefined as unknown as CustomAdapter["consumeOne"], + consumeOne: undefined, + findOne: async () => + ({ id: "verification-id", identifier_text: "stored-token" }) as T, + deleteMany: async ({ where }) => { + deleteCalls.push(where); + return 1; + }, + }), + }); + + const result = await adapter.consumeOne<{ id: string; identifier: string }>( + { + model: "verification", + where: [{ field: "identifier", value: "token" }], + }, + ); + + expect(result).toEqual({ + id: "verification-id", + identifier: "stored-token:output", + }); + expect(deleteCalls[0]).toEqual([ + { + field: "identifier_text", + value: "token:consumeOne", + operator: "eq", + connector: "AND", + mode: "sensitive", + }, + { + field: "id", + value: "verification-id", + operator: "eq", + connector: "AND", + mode: "sensitive", + }, + ]); + }); + + it("returns null from the consumeOne fallback when another caller deleted the row first", async () => { + const adapter = createTestAdapter({ + adapter: createCustomAdapter({ + consumeOne: undefined, + findOne: async () => ({ id: "verification-id" }) as T, + deleteMany: async () => 0, }), }); @@ -221,13 +266,100 @@ describe("createAdapterFactory atomic primitives", () => { model: "verification", where: [{ field: "identifier", value: "token" }], }), - ).rejects.toThrow(/must implement consumeOne/); + ).resolves.toBeNull(); }); - it("throws a clear error when incrementOne is missing at runtime", async () => { + it("falls back to a compare-and-swap updateMany when incrementOne is missing", async () => { + const updateCalls: { + where: CleanedWhere[]; + update: Record; + }[] = []; + let attemptCount = 2; const adapter = createTestAdapter({ adapter: createCustomAdapter({ - incrementOne: undefined as unknown as CustomAdapter["incrementOne"], + incrementOne: undefined, + findOne: async () => + ({ id: "verification-id", attempt_count: attemptCount }) as T, + updateMany: async ({ where, update }) => { + updateCalls.push({ where, update }); + attemptCount = update.attempt_count; + return 1; + }, + }), + }); + + const result = await adapter.incrementOne<{ id: string; attempts: number }>( + { + model: "verification", + where: [{ field: "identifier", value: "token" }], + increment: { attempts: 1 }, + }, + ); + + expect(result).toEqual({ id: "verification-id", attempts: 3 }); + expect(updateCalls).toHaveLength(1); + expect(updateCalls[0]!.update).toEqual({ attempt_count: 3 }); + expect(updateCalls[0]!.where).toEqual([ + { + field: "identifier_text", + value: "token:incrementOne", + operator: "eq", + connector: "AND", + mode: "sensitive", + }, + { + field: "id", + value: "verification-id", + operator: "eq", + connector: "AND", + mode: "sensitive", + }, + { + field: "attempt_count", + value: 2, + operator: "eq", + connector: "AND", + mode: "sensitive", + }, + ]); + }); + + it("retries the incrementOne fallback after losing a compare-and-swap race", async () => { + let stored = 2; + let updateCalls = 0; + const adapter = createTestAdapter({ + adapter: createCustomAdapter({ + incrementOne: undefined, + findOne: async () => + ({ id: "verification-id", attempt_count: stored }) as T, + updateMany: async ({ update }) => { + updateCalls++; + if (updateCalls === 1) { + // A concurrent writer bumped the counter between read and write. + stored = 3; + return 0; + } + stored = update.attempt_count; + return 1; + }, + }), + }); + + const result = await adapter.incrementOne<{ attempts: number }>({ + model: "verification", + where: [{ field: "identifier", value: "token" }], + increment: { attempts: 1 }, + }); + + expect(updateCalls).toBe(2); + expect(result?.attempts).toBe(4); + }); + + it("returns null from the incrementOne fallback when the guard matches no row", async () => { + const adapter = createTestAdapter({ + adapter: createCustomAdapter({ + incrementOne: undefined, + findOne: async () => null, }), }); @@ -237,7 +369,7 @@ describe("createAdapterFactory atomic primitives", () => { where: [{ field: "identifier", value: "token" }], increment: { attempts: 1 }, }), - ).rejects.toThrow(/must implement incrementOne/); + ).resolves.toBeNull(); }); it("throws a clear error when updateMany does not return a finite count", async () => { diff --git a/packages/core/src/db/adapter/factory.ts b/packages/core/src/db/adapter/factory.ts index 7fa1601067..8c67fc51c5 100644 --- a/packages/core/src/db/adapter/factory.ts +++ b/packages/core/src/db/adapter/factory.ts @@ -1366,18 +1366,47 @@ export const createAdapterFactory = { model, where }, ); - if (typeof adapterInstance.consumeOne !== "function") { - throw new BetterAuthError( - `Adapter "${config.adapterId}" must implement consumeOne for atomic single-use credential consumption.`, - ); - } + const nativeConsumeOne = adapterInstance.consumeOne; const res = await withSpan( `db consumeOne ${model}`, { [ATTR_DB_OPERATION_NAME]: "consumeOne", [ATTR_DB_COLLECTION_NAME]: model, }, - () => adapterInstance.consumeOne({ model, where }), + async () => { + if (typeof nativeConsumeOne === "function") { + return nativeConsumeOne({ model, where }); + } + // Fallback for adapters without a native consumeOne: read the + // candidate, then delete it guarded on its id. A single-row + // DELETE is atomic on every supported store, so exactly one of + // several concurrent callers sees a deleted count of 1. + const idField = getFieldName({ model: unsafeModel, field: "id" }); + const target = await adapterInstance.findOne>({ + model, + where, + }); + if (!target) return null; + const deleted = await adapterInstance.deleteMany({ + model, + where: [ + ...where, + { + field: idField, + value: target[idField], + operator: "eq", + connector: "AND", + mode: "sensitive", + }, + ], + }); + if (typeof deleted !== "number" || !Number.isFinite(deleted)) { + throw new BetterAuthError( + `Adapter "${config.adapterId}" returned a non-numeric value from deleteMany during the consumeOne fallback. Return the number of deleted rows, or implement a native consumeOne.`, + ); + } + return deleted > 0 ? (target as T) : null; + }, ); debugLog( @@ -1440,11 +1469,6 @@ export const createAdapterFactory = { model, where, increment: unsafeIncrement, set: unsafeSet }, ); - if (typeof adapterInstance.incrementOne !== "function") { - throw new BetterAuthError( - `Adapter "${config.adapterId}" must implement incrementOne for atomic guarded counter updates.`, - ); - } const mappedKeys = config.mapKeysTransformInput ?? {}; const increment: Record = {}; for (const [field, delta] of Object.entries(unsafeIncrement)) { @@ -1466,19 +1490,69 @@ export const createAdapterFactory = "incrementOne resolved to an empty update: every increment/set field was unknown to the schema or transformed away.", ); } + const nativeIncrementOne = adapterInstance.incrementOne; const res = await withSpan( `db incrementOne ${model}`, { [ATTR_DB_OPERATION_NAME]: "incrementOne", [ATTR_DB_COLLECTION_NAME]: model, }, - () => - adapterInstance.incrementOne({ - model, - where, - increment, - set, - }), + async () => { + if (typeof nativeIncrementOne === "function") { + return nativeIncrementOne({ model, where, increment, set }); + } + // Fallback for adapters without a native incrementOne: a + // compare-and-swap loop. Read the row, then update it guarded on + // its id and the counter values just read. A lost race returns + // an affected count of 0 while the selector still matches, so + // retry; a selector that no longer matches returns null. + const idField = getFieldName({ model: unsafeModel, field: "id" }); + // Bounded retries: adapters under heavy contention should implement + // incrementOne natively instead of relying on this loop. + for (let attempt = 0; attempt < 5; attempt++) { + const current = await adapterInstance.findOne< + Record + >({ model, where }); + if (!current) return null; + const idWhere: CleanedWhere = { + field: idField, + value: current[idField], + operator: "eq", + connector: "AND", + mode: "sensitive", + }; + const guard: CleanedWhere[] = [...where, idWhere]; + const update: Record = { ...(set ?? {}) }; + for (const [field, delta] of Object.entries(increment)) { + const previous = current[field]; + update[field] = + (typeof previous === "number" ? previous : 0) + delta; + if (typeof previous === "number") { + guard.push({ + field, + value: previous, + operator: "eq", + connector: "AND", + mode: "sensitive", + }); + } + } + const updated = await adapterInstance.updateMany({ + model, + where: guard, + update, + }); + if (typeof updated !== "number" || !Number.isFinite(updated)) { + throw new BetterAuthError( + `Adapter "${config.adapterId}" returned a non-numeric value from updateMany during the incrementOne fallback. Return the number of updated rows, or implement a native incrementOne.`, + ); + } + if (updated > 0) { + return adapterInstance.findOne({ model, where: [idWhere] }); + } + } + return null; + }, ); debugLog( diff --git a/packages/core/src/db/adapter/index.ts b/packages/core/src/db/adapter/index.ts index 5181a5591e..8bcf4b9992 100644 --- a/packages/core/src/db/adapter/index.ts +++ b/packages/core/src/db/adapter/index.ts @@ -653,8 +653,12 @@ export interface CustomAdapter { * `findOneAndDelete`, `OUTPUT deleted.*`) gives one round trip and the * strongest race-safety guarantee. Implementations must delete at most * one matching row. + * + * Optional. When omitted, the factory falls back to `findOne` followed by a + * `deleteMany` guarded on the row id, and treats the caller as the winner + * only when exactly one row was deleted. */ - consumeOne: (data: { + consumeOne?: (data: { model: string; where: CleanedWhere[]; }) => Promise; @@ -668,8 +672,12 @@ export interface CustomAdapter { * Implementing this natively (e.g. `UPDATE ... SET n = n + $delta WHERE ... * RETURNING *`) gives one round trip and the strongest race-safety * guarantee. + * + * Optional. When omitted, the factory falls back to a compare-and-swap + * loop: `findOne`, then `updateMany` guarded on the row id and the current + * counter values, retried when a concurrent writer changed the row first. */ - incrementOne: (data: { + incrementOne?: (data: { model: string; where: CleanedWhere[]; increment: Record;