Skip to content

Recover from recurring LMDB corruption and unhandled commit rejections - #636

Open
gemammercado wants to merge 3 commits into
mainfrom
fix/lmdb-corruption-recovery
Open

Recover from recurring LMDB corruption and unhandled commit rejections#636
gemammercado wants to merge 3 commits into
mainfrom
fix/lmdb-corruption-recovery

Conversation

@gemammercado

Copy link
Copy Markdown
Contributor

Problem

LMDB errors are a top source of uncaught exceptions / unhandled promise rejections from the language server. Two distinct paths were involved:

  1. On-disk corruption is never actually cleared at runtime. handleError routes corruption errors (MDB_CORRUPTED, MDB_PAGE_NOTFOUND, MDB_BAD_VALSIZE, MDB_CURSOR_FULL, MDB_BAD_TXN) to recoverFromError, which calls reopenEnv() first. Re-opening re-maps the same on-disk files and does not throw for page-level corruption (open/openDB are lazy), so recovery reports success without ever escalating to deleteAndRecreate(). The retried op hits the same corrupt page and fails again, and every subsequent operation in that process keeps failing — a loop that only ends when the editor restarts.

  2. lmdb-js commit failures reject a promise nobody awaits. On a failed commit, lmdb-js rejects each per-op promise with Error('Commit failed (see commitError for details)') and separately rejects an internal commitRejectPromise (exposed only as error.commitError) with the real underlying error. Nothing awaits commitError, so it surfaces as an unhandled rejection with a stack entirely inside lmdb/dist/index.cjs and no application frames.

Fix

  • LMDBStoreFactory.handleError: route corruption codes to a new recoverFromCorruption() that escalates. The first corruption within a window reopens (cheap, preserves data, covers rare transient/handle-level failures); if corruption recurs within CorruptionEscalationWindowMs, it calls deleteAndRecreate() to clear the corrupt files so the retry and future ops succeed on a fresh env. Emits corruption.detected / corruption.recreate counters.
  • LMDBStore.execAsync: when a caught error carries a commitError promise, attach a handler that routes its underlying cause through the normal recovery path. This prevents the unhandled rejection and lets the real corruption code drive escalation.

The escalation preserves existing behavior on the first occurrence (reopen), so transient failures still recover without data loss.

Testing

  • npm run build, lint, and the full LMDB unit suite pass (181 tests, including new escalation and commit-rejection tests).
  • New tests: first corruption reopens without deleting; recurring corruption deletes+recreates the version dir; execAsync consumes the commitError promise and routes its cause to recovery.

@gemammercado
gemammercado requested a review from a team as a code owner July 13, 2026 21:20
@github-code-quality

github-code-quality Bot commented Jul 13, 2026

Copy link
Copy Markdown

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/vitest

The overall coverage in the fix/lmdb-corruption-... branch remains at 90%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main d580f61 fix/lmdb-corruption-... b6087d6 +/-
src/utils/Errors.ts 100% 0% -100%
src/utils/Delayer.ts 91% 91% 0%
src/datastore/l...db/LMDBStore.ts 100% 100% 0%
src/datastore/L...StoreFactory.ts 73% 75% +2%
src/utils/error...ErrorClasses.ts 0% 79% +79%
src/utils/error...rorStackInfo.ts 0% 95% +95%
src/utils/error...sErrorMapper.ts 0% 98% +98%
src/utils/errors/ErrorUtils.ts 0% 100% +100%
src/utils/error...tSuppression.ts 0% 100% +100%
src/utils/error...FeatureError.ts 0% 100% +100%

Updated July 14, 2026 15:59 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@satyakigh satyakigh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets hold off on this change since #615 has not been deployed yet. This might be a non issue.

Some other issues

  • Concurrent writes wipe the database on the first corruption - irst corruption → cheap reopen (preserves data); recurring corruption within 30s → deleteAndRecreate() (wipes data). But when two or more writes are in flight in the same failed commit batch, they each carry the same commitError promise. The PR attaches a separate catch per operation, so a single underlying corruption event drives handleError N times back-to-back in the same microtask window. The second call sees now - lastCorruptionAt < 30s and escalates straight to deleteAndRecreate()
  • The new .catch is attached only around the first await fn() (line 55). The retry return await fn() (line 60) is outside that guard. When the retry also fails with a commitError (persistent corruption — precisely the scenario the PR targets), that second promise has no handler and surfaces as an unhandled rejection with a pure-lmdb stack — the original symptom.
  • MDB_BAD_TXN/MDB_CURSOR_FULL are frequently transient (aborted/nested txn, cursor exhaustion) and don't imply on-disk corruption. the new logic, a recurring transient MDB_BAD_TXN within 30s now triggers a full data wipe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants