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.
This commit is contained in:
parent
cb5020cff4
commit
a090fba856
2 changed files with 25 additions and 6 deletions
|
|
@ -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<T>(
|
|||
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');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue