[auth] fix potential issue with refresh tokens, however the JWT being 86400 probably fixes this too

apps/auth/src/refresh-db.ts

@@ -47,24 +47,55 @@ export async function issueRefreshToken(db: D1Database, accountId: number): Prom
 	return token
 }
 
+/**
+ * Attempts per redemption. D1 occasionally answers a perfectly good statement with
+ * `D1_ERROR: internal error` — a storage-side hiccup carrying a support reference, not a
+ * verdict on the query. Unretried, one of those logs a player out: the grant throws, the
+ * shared error handler turns it into a 500, and the client falls back to the login
+ * screen with a refresh token it never got to spend.
+ *
+ * Deliberately small, like the Meta nonce retry next door: a refresh blocks the client on
+ * a loading screen, so two quick retries ride out a blip and a longer outage fails fast
+ * rather than hanging.
+ */
+const MAX_ATTEMPTS = 3
+
 /**
  * Redeem a refresh token: if it exists and hasn't expired, delete it (single-use
  * rotation) and return the account it logs in; otherwise return null. The delete is
  * atomic (`DELETE ... RETURNING`), so a token can't be redeemed twice — a
  * concurrent second attempt finds no row. An expired token is deleted and rejected.
+ *
+ * A D1 error is retried (see {@link MAX_ATTEMPTS}). That is safe precisely BECAUSE the
+ * statement is atomic and single-use: an attempt that actually committed before failing
+ * to answer leaves no row, so the retry returns null and the player re-logs in — exactly
+ * what a replayed token does. There is no interleaving in which retrying redeems one
+ * token twice, and none in which it lands worse than the 500 it replaces.
+ *
+ * Only the D1 call is retried; the hashing around it is pure.
  */
-export async function consumeRefreshToken(
+export async function consumeRefreshToken(db: D1Database, token: string): Promise<number | null> {
-	db: D1Database,
-	token: string
-): Promise<number | null> {
 	const now = Math.floor(Date.now() / 1000)
-	const row = await db
+	const statement = db
 		.prepare(
 			`DELETE FROM refresh_tokens WHERE token_hash = ?1
 			 RETURNING account_id AS accountId, expires_at AS expiresAt`
 		)
 		.bind(await hashToken(token))
-		.first<{ accountId: number; expiresAt: number }>()
+
+	let row: { accountId: number; expiresAt: number } | null = null
+	for (let attempt = 1; ; attempt++) {
+		try {
+			row = await statement.first<{ accountId: number; expiresAt: number }>()
+			break
+		} catch (err) {
+			// The last attempt rethrows: a D1 outage is a 500, not a silent "bad token" that
+			// would tell the player to log in again over something that isn't their fault.
+			if (attempt === MAX_ATTEMPTS) throw err
+			await new Promise((resolve) => setTimeout(resolve, attempt * attempt * 250))
+		}
+	}
+
 	if (!row || row.expiresAt < now) return null
 	return row.accountId
 }

apps/auth/src/test/integration/api.test.ts

@@ -27,7 +27,7 @@ import {
 	PLATFORM_BACKFILL_SQL,
 	PLATFORM_SCHEMA_DDL,
 } from '../../platform-db'
-import { REFRESH_SCHEMA_DDL } from '../../refresh-db'
+import { consumeRefreshToken, issueRefreshToken, REFRESH_SCHEMA_DDL } from '../../refresh-db'
 
 import type { Env } from '../../context'
 
@@ -1083,6 +1083,44 @@ describe('auth worker routes', () => {
 		expect(reuse.json.error).toBe('invalid_grant')
 	})
 
+	/**
+	 * A D1 that throws the storage-side `internal error` on its first `n` reads and then
+	 * behaves. Only what `consumeRefreshToken` touches is wrapped — prepare/bind/first.
+	 */
+	const flakyDb = (failures: number): D1Database => {
+		let remaining = failures
+		const wrap = (stmt: D1PreparedStatement): D1PreparedStatement =>
+			({
+				bind: (...values: unknown[]) => wrap(stmt.bind(...values)),
+				first: async <T>() => {
+					if (remaining > 0) {
+						remaining--
+						throw new Error('D1_ERROR: internal error; reference = testref0000')
+					}
+					return stmt.first<T>()
+				},
+			}) as unknown as D1PreparedStatement
+		return { prepare: (sql: string) => wrap(env.DB.prepare(sql)) } as unknown as D1Database
+	}
+
+	test('consumeRefreshToken rides out a transient D1 error rather than logging the player out', async () => {
+		const token = await issueRefreshToken(env.DB, 77)
+		// Two hiccups, then the real thing — the redemption still succeeds, so the player
+		// keeps their session instead of being bounced to the login screen by a 500.
+		expect(await consumeRefreshToken(flakyDb(2), token)).toBe(77)
+		// And it was genuinely consumed: the retry didn't leave the row behind.
+		expect(await consumeRefreshToken(env.DB, token)).toBeNull()
+	})
+
+	test('consumeRefreshToken rethrows once the retries are spent, never a silent null', async () => {
+		const token = await issueRefreshToken(env.DB, 77)
+		// A real D1 outage has to surface as a 500. Answering null would tell the player
+		// their token was bad and make them log in again over something that isn't theirs.
+		await expect(consumeRefreshToken(flakyDb(99), token)).rejects.toThrow('internal error')
+		// The token survived, so a later attempt still works.
+		expect(await consumeRefreshToken(env.DB, token)).toBe(77)
+	})
+
 	test('POST /connect/token 400s on an unknown refresh_token', async () => {
 		const res = await postToken('grant_type=refresh_token&refresh_token=NOPE-1')
 		expect(res.status).toBe(400)