From 33a7ef54850749fe41c09d5e13bdcc5e1621bd9a Mon Sep 17 00:00:00 2001 From: BilalG1 Date: Fri, 17 Jul 2026 09:09:15 -0700 Subject: [PATCH 1/2] Fix item quantity dialog balance preview flicker on submit (#1774) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem On a customer's payments view, submitting a manual item quantity change makes the "New balance: X → Y" preview in the dialog briefly flip to the post-change values (e.g. `0 → 10` flashes to `10 → 20`) right before the dialog closes. ## Root cause The preview was computed from the live `currentQuantity` prop, which comes from `useItem()` in the parent row. The SDK's `createItemQuantityChange` awaits its item cache refresh before resolving, so the prop updates to the new quantity while the dialog is still open (and stays visible through the close animation), recomputing the preview with stale input text. ## Fix Snapshot the quantity when the dialog opens (in the existing reset-on-open effect) and compute the preview from the snapshot instead of the live prop. The preview now consistently shows the balance as of when the dialog was opened. ## Before vs after Recorded against built servers (before = `dev`, after = this branch). The transactions refetch is artificially delayed by 5s in both recordings to stretch the flicker window to a visible length — on production latency it's a split-second flash. https://github.com/user-attachments/assets/c005ef3b-45cb-4b75-bd71-5f723eae0c50 ## Tradeoffs If the quantity changes externally while the dialog is open, the preview shows the value from open time. The mutation is a delta, so correctness of the applied change is unaffected. ## Summary by CodeRabbit * **Bug Fixes** * Stabilized the quantity change preview while the dialog is open. * The “New balance” display now consistently shows the quantity at opening and the calculated updated quantity, even if underlying values change during submission. --- .../payments/item-quantity-change-dialog.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/apps/dashboard/src/components/payments/item-quantity-change-dialog.tsx b/apps/dashboard/src/components/payments/item-quantity-change-dialog.tsx index 2ab965c76..f644c282c 100644 --- a/apps/dashboard/src/components/payments/item-quantity-change-dialog.tsx +++ b/apps/dashboard/src/components/payments/item-quantity-change-dialog.tsx @@ -44,6 +44,10 @@ export function ItemQuantityChangeDialog(props: Props) { const [description, setDescription] = useState(""); const [error, setError] = useState(null); const [isSubmitting, setIsSubmitting] = useState(false); + // Snapshot the quantity at dialog open: the live prop updates mid-submit + // (the SDK refreshes the item cache before resolving), which would flip the + // "X → Y" preview to the post-change values right before the dialog closes. + const [quantityAtOpen, setQuantityAtOpen] = useState(props.currentQuantity); // Reset the form whenever the dialog is (re)opened so stale input doesn't // leak across invocations. @@ -52,11 +56,13 @@ export function ItemQuantityChangeDialog(props: Props) { setQuantityText(""); setDescription(""); setError(null); + setQuantityAtOpen(props.currentQuantity); } + // eslint-disable-next-line react-hooks/exhaustive-deps -- snapshot only on open }, [props.open]); const parsedQuantity = parseQuantityChange(quantityText); - const newQuantity = parsedQuantity == null ? null : props.currentQuantity + parsedQuantity; + const newQuantity = parsedQuantity == null ? null : quantityAtOpen + parsedQuantity; const submit = async () => { if (parsedQuantity == null) { @@ -143,7 +149,7 @@ export function ItemQuantityChangeDialog(props: Props) { {newQuantity == null ? "Positive values add to the balance, negative values subtract." - : `New balance: ${props.currentQuantity} → ${newQuantity}`} + : `New balance: ${quantityAtOpen} → ${newQuantity}`} )} 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 2/2] [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 }, + ); +}