From a090fba856087392ba8701ed65c809841e5f4fe6 Mon Sep 17 00:00:00 2001 From: Richard R Date: Sun, 7 Jun 2026 13:06:26 -0600 Subject: [PATCH] fix(documents): improve blob lease release error handling and test stale lease scenario Handle errors during blob lease release by logging warnings instead of allowing them to mask original results. Update unit tests to verify that releasing a stale lease does not affect a replacement lease. --- src/lib/server/documents/blob-lease.ts | 21 +++++++++++++------ tests/unit/document-blob-lease.vitest.spec.ts | 10 +++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/lib/server/documents/blob-lease.ts b/src/lib/server/documents/blob-lease.ts index 1013427..8c404cb 100644 --- a/src/lib/server/documents/blob-lease.ts +++ b/src/lib/server/documents/blob-lease.ts @@ -1,7 +1,8 @@ import { randomUUID } from 'node:crypto'; -import { and, eq, lt, or } from 'drizzle-orm'; +import { and, eq, lt } from 'drizzle-orm'; import { db } from '@/db'; import { documentBlobLeases } from '@/db/schema'; +import { errorToLog, serverLogger } from '@/lib/server/logger'; const DEFAULT_LEASE_MS = 15 * 60 * 1000; const RETRY_DELAY_MS = 100; @@ -29,10 +30,7 @@ export async function tryAcquireDocumentBlobLease( .onConflictDoUpdate({ target: documentBlobLeases.documentId, set: { leaseOwner: owner, leaseUntilMs }, - where: or( - lt(documentBlobLeases.leaseUntilMs, now), - eq(documentBlobLeases.leaseOwner, owner), - ), + where: lt(documentBlobLeases.leaseUntilMs, now), }) .returning({ owner: documentBlobLeases.leaseOwner }); @@ -70,6 +68,17 @@ export async function withDocumentBlobLease( try { return await fn(); } finally { - await lease.release(); + // Release in its own try/catch so a failing release never masks the + // original error (or result) produced by fn(). + try { + await lease.release(); + } catch (releaseError) { + serverLogger.warn({ + event: 'documents.blobLease.release.failed', + degraded: true, + documentId, + error: errorToLog(releaseError), + }, 'Failed to release document blob lease'); + } } } diff --git a/tests/unit/document-blob-lease.vitest.spec.ts b/tests/unit/document-blob-lease.vitest.spec.ts index b1500a9..684d75b 100644 --- a/tests/unit/document-blob-lease.vitest.spec.ts +++ b/tests/unit/document-blob-lease.vitest.spec.ts @@ -45,5 +45,15 @@ describe('document blob lease', () => { expect(replacement).not.toBeNull(); expect(replacement?.owner).not.toBe(first?.owner); + + // Releasing the now-stale original lease must not delete the replacement, + // since release() is scoped to the original owner. + await first?.release(); + + const rows = await holder.db + .select() + .from(sqliteSchema.documentBlobLeases); + expect(rows).toHaveLength(1); + expect(rows[0]?.leaseOwner).toBe(replacement?.owner); }); });