From 970c01998c0447f34a23b102a6b8e60f2cd39a73 Mon Sep 17 00:00:00 2001 From: Aman Ganapathy <84686202+nams1570@users.noreply.github.com> Date: Fri, 17 Jul 2026 09:20:19 -0700 Subject: [PATCH] [Refactor] [Bulldozer]: unwrap lmdb errors (#1775) CommitErrors are hard to parse as they stand so we unwrap them. --- ## Summary by cubic Unwrap `lmdb` commit failures to surface the real underlying error (e.g., ENOSPC/MDB_MAP_FULL) instead of the opaque "Commit failed" wrapper. Improves logs and error propagation across availability/durability paths. - **Refactors** - Added `unwrapLmdbCommitError` to await and rethrow the real `lmdb` commit error; returns `HexclaveAssertionError` if not an `Error` (`@hexclave/shared`). - Routed all availability/durability trackers and batched commit paths through the unwrapping helper to ensure callers see the true cause. - Tests cover pass-through, unwrap behavior, and non-Error assertions. - Updated a comment in `instant-availability` to note low-level LMDB unwrapping. Written for commit 0a1d69beaed0d0e7f8cd2f60082819e82adac921. Summary will update on new commits. Review in cubic ## Summary by CodeRabbit * **Bug Fixes** * Improved LMDB commit error reporting by exposing the underlying cause instead of an opaque wrapper. * Ensured pending availability and durability operations receive the unwrapped commit error. * Added handling for commit failures with unexpected error values. * **Tests** * Added coverage for wrapped, unwrapped, and non-standard commit error scenarios. --- .../implementations/instant-availability.ts | 2 +- .../low-level/implementations/lmdb.ts | 32 +++++++++++------ .../low-level/unwrap-commit-error.test.ts | 36 +++++++++++++++++++ .../low-level/unwrap-commit-error.ts | 26 ++++++++++++++ 4 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 apps/bulldozer-js/src/databases/low-level/unwrap-commit-error.test.ts create mode 100644 apps/bulldozer-js/src/databases/low-level/unwrap-commit-error.ts diff --git a/apps/bulldozer-js/src/databases/low-level/implementations/instant-availability.ts b/apps/bulldozer-js/src/databases/low-level/implementations/instant-availability.ts index 937fcce8a..6ec71e68d 100644 --- a/apps/bulldozer-js/src/databases/low-level/implementations/instant-availability.ts +++ b/apps/bulldozer-js/src/databases/low-level/implementations/instant-availability.ts @@ -121,7 +121,7 @@ export function declareInstantAvailabilityLowLevelDatabase(wrapped: LowLevelData // reconciled. We deliberately don't block any caller on this (no fsync on the // hot path); other awaiters of `underlyingAvailable` handle the rejection // themselves, this handler exists purely to report + avoid an unhandled - // rejection. + // rejection. (LMDB unwraps opaque commit failures in the low-level layer.) underlyingAvailable.catch(error => captureError("bulldozer-js:instant-availability-durable-commit", error)); const record = { underlyingSeq, underlyingAvailable, isSettled: false }; createdSeqRecords++; diff --git a/apps/bulldozer-js/src/databases/low-level/implementations/lmdb.ts b/apps/bulldozer-js/src/databases/low-level/implementations/lmdb.ts index 62873c110..747d9fd02 100644 --- a/apps/bulldozer-js/src/databases/low-level/implementations/lmdb.ts +++ b/apps/bulldozer-js/src/databases/low-level/implementations/lmdb.ts @@ -5,6 +5,7 @@ import { shouldSuppressPeriodicBulldozerLogs } from "../../../logging.js"; import { traceSpanHot } from "../../../otel.js"; import { DatabaseSeq } from "../../index.js"; import { LowLevelDatabase, LowLevelDatabaseDebugEntry, LowLevelKvDump, LowLevelKvStore } from "../index.js"; +import { unwrapLmdbCommitError } from "../unwrap-commit-error.js"; type LmdbSeq = readonly [dbId: string, seqId: string] & { __brand: "hexclave-low-level-kv-store-seq" }; type BinaryDatabase = lmdb.Database; @@ -206,44 +207,54 @@ export function declareLmdbLowLevelDatabase(options: { path: string, dbId?: stri const getDurabilityPromise = (seqId: string) => { return seqToDurability.get(seqId) ?? combinedSeqToDurability.get(seqId) ?? Promise.resolve(); }; + // LMDB may reject with an opaque "Commit failed" wrapper whose real status + // lives on `.commitError` (a Promise). Unwrap before surfacing to callers. + const awaitLmdbPromise = (promise: Promise) => promise.catch(async (error) => { + throw await unwrapLmdbCommitError(error); + }); const rememberAvailability = (seqId: string, promise: Promise) => { const insertedAt = performance.now(); - const availability = traceSpanHot({ description: "bulldozer-js.low-level.lmdb.availability", attributes: { "bulldozer.low_level.backend": "lmdb" } }, async () => await promise.then(() => { + const availability = traceSpanHot({ description: "bulldozer-js.low-level.lmdb.availability", attributes: { "bulldozer.low_level.backend": "lmdb" } }, async () => { + await awaitLmdbPromise(promise); activityStats.waitUntilAvailableResolveTotalMs += performance.now() - insertedAt; activityStats.waitUntilAvailableResolves++; seqToAvailability.delete(seqId); - })); + }); availability.catch(() => {}); seqToAvailability.set(seqId, availability); }; const rememberDurability = (seqId: string, promise: Promise) => { const insertedAt = performance.now(); - const durability = traceSpanHot({ description: "bulldozer-js.low-level.lmdb.durability", attributes: { "bulldozer.low_level.backend": "lmdb" } }, async () => await promise.then(async () => await root.flushed).then(() => { + const durability = traceSpanHot({ description: "bulldozer-js.low-level.lmdb.durability", attributes: { "bulldozer.low_level.backend": "lmdb" } }, async () => { + await awaitLmdbPromise(promise); + await root.flushed; activityStats.waitUntilDurableResolveTotalMs += performance.now() - insertedAt; activityStats.waitUntilDurableResolves++; seqToDurability.delete(seqId); - })); + }); durability.catch(() => {}); seqToDurability.set(seqId, durability); }; const rememberCombinedAvailability = (seqId: string, promise: Promise) => { const insertedAt = performance.now(); - const availability = traceSpanHot({ description: "bulldozer-js.low-level.lmdb.combinedAvailability", attributes: { "bulldozer.low_level.backend": "lmdb" } }, async () => await promise.then(() => { + const availability = traceSpanHot({ description: "bulldozer-js.low-level.lmdb.combinedAvailability", attributes: { "bulldozer.low_level.backend": "lmdb" } }, async () => { + await awaitLmdbPromise(promise); activityStats.combinedSeqAvailabilityResolveTotalMs += performance.now() - insertedAt; activityStats.combinedSeqAvailabilityResolves++; combinedSeqToAvailability.delete(seqId); combinedSeqDependencies.delete(seqId); - })); + }); availability.catch(() => {}); combinedSeqToAvailability.set(seqId, availability); }; const rememberCombinedDurability = (seqId: string, promise: Promise) => { const insertedAt = performance.now(); - const durability = traceSpanHot({ description: "bulldozer-js.low-level.lmdb.combinedDurability", attributes: { "bulldozer.low_level.backend": "lmdb" } }, async () => await promise.then(() => { + const durability = traceSpanHot({ description: "bulldozer-js.low-level.lmdb.combinedDurability", attributes: { "bulldozer.low_level.backend": "lmdb" } }, async () => { + await awaitLmdbPromise(promise); activityStats.combinedSeqDurabilityResolveTotalMs += performance.now() - insertedAt; activityStats.combinedSeqDurabilityResolves++; combinedSeqToDurability.delete(seqId); - })); + }); durability.catch(() => {}); combinedSeqToDurability.set(seqId, durability); }; @@ -285,8 +296,9 @@ export function declareLmdbLowLevelDatabase(options: { path: string, dbId?: stri }); for (const operation of operations) operation.resolve(); } catch (error) { - for (const operation of operations) operation.reject(error); - throw error; + const unwrapped = await unwrapLmdbCommitError(error); + for (const operation of operations) operation.reject(unwrapped); + throw unwrapped; } }; const flushPendingCommits = async () => { diff --git a/apps/bulldozer-js/src/databases/low-level/unwrap-commit-error.test.ts b/apps/bulldozer-js/src/databases/low-level/unwrap-commit-error.test.ts new file mode 100644 index 000000000..eba1550cc --- /dev/null +++ b/apps/bulldozer-js/src/databases/low-level/unwrap-commit-error.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from "vitest"; +import { unwrapLmdbCommitError } from "./unwrap-commit-error.js"; + +describe("unwrapLmdbCommitError", () => { + it("passes through values that are not LMDB commit wrappers", async () => { + expect(await unwrapLmdbCommitError("plain")).toBe("plain"); + const ordinary = new Error("ordinary failure"); + expect(await unwrapLmdbCommitError(ordinary)).toBe(ordinary); + }); + + it("returns the underlying error after LMDB's rejectCommit → commitRejectPromise.reject sequence", async () => { + // Mirrors lmdb/write.js: rejectCommit() attaches an unsettled Promise and rejects + // the write; then the same sync callback settles that Promise with lmdbError(status). + let rejectCommitError!: (error: Error) => void; + const commitError = new Promise((_resolve, reject) => { + rejectCommitError = reject; + }); + commitError.catch(() => {}); + + const wrapper = Object.assign(new Error("Commit failed (see commitError for details)"), { commitError }); + const underlying = new Error("ENOSPC: no space left on device"); + + const unwrappedPromise = unwrapLmdbCommitError(wrapper); + rejectCommitError(underlying); + + expect(await unwrappedPromise).toBe(underlying); + }); + + it("returns HexclaveAssertionError when commitError does not settle to an Error", async () => { + const wrapper = Object.assign(new Error("Commit failed (see commitError for details)"), { + commitError: Promise.resolve("not-an-error"), + }); + const result = await unwrapLmdbCommitError(wrapper); + expect(result).toMatchObject({ name: "HexclaveAssertionError" }); + }); +}); diff --git a/apps/bulldozer-js/src/databases/low-level/unwrap-commit-error.ts b/apps/bulldozer-js/src/databases/low-level/unwrap-commit-error.ts new file mode 100644 index 000000000..b3f12f4e5 --- /dev/null +++ b/apps/bulldozer-js/src/databases/low-level/unwrap-commit-error.ts @@ -0,0 +1,26 @@ +import { HexclaveAssertionError } from "@hexclave/shared/dist/utils/errors"; + +/** + * LMDB rejects failed writes as `Error("Commit failed (see commitError for details)")` + * with `.commitError` set to a Promise that later rejects with the real native status + * (ENOSPC, MDB_MAP_FULL, …). Await that before rethrowing/logging, or you only see the + * opaque wrapper. + * + * Why awaiting works: in lmdb/write.js, `rejectCommit()` attaches the Promise and rejects + * the write; then in the *same* sync callback, `commitRejectPromise.reject(lmdbError(status))` + * settles it. By the time our microtask runs, `.commitError` already has the real error. + */ +export async function unwrapLmdbCommitError(error: unknown): Promise { + if (!(error instanceof Error) || !("commitError" in error)) return error; + + const settled = error.commitError instanceof Promise + ? await error.commitError.then(value => value, rejected => rejected) + : error.commitError; + + if (settled instanceof Error) return settled; + + return new HexclaveAssertionError( + "LMDB commit failed but commitError did not settle to an Error", + { cause: error, commitError: settled }, + ); +}