From 19f3660528ca511bcbdaeed34668d9ea16a432d5 Mon Sep 17 00:00:00 2001 From: VeveyZhan Date: Wed, 29 Jul 2026 14:26:52 -0400 Subject: [PATCH] fix: catch MDB_BAD_VALSIZE in LMDBStore.put to prevent unhandled promise rejections --- src/datastore/Utils.ts | 11 +++ src/datastore/lmdb/LMDBStore.ts | 31 +++++++-- tst/unit/datastore/LMDB.valueSize.test.ts | 82 +++++++++++++++++++++++ 3 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 tst/unit/datastore/LMDB.valueSize.test.ts diff --git a/src/datastore/Utils.ts b/src/datastore/Utils.ts index 6b16f9d2..c612ca6a 100644 --- a/src/datastore/Utils.ts +++ b/src/datastore/Utils.ts @@ -23,6 +23,17 @@ export enum StoreOperation { export type ErrorHandler = (error: unknown, op: StoreOperation) => void | Promise; +/** + * A value was rejected by LMDB for exceeding its per-value/page size limits + * (`MDB_BAD_VALSIZE`). This is deterministic: retrying the same write, even after env + * recovery, fails identically, so callers should skip the generic retry/recovery path for + * this error rather than pay for a reopen that cannot help. Checks the whole cause chain + * since lmdb-js often wraps this behind a `Commit failed` error (see `CommitError.ts`). + */ +export function isValueTooLarge(error: unknown): boolean { + return errorCauseChain(error).some((link) => extractErrorMessage(link).includes('MDB_BAD_VALSIZE')); +} + /** Why stored data could not be read back. Reported as a suffix on {@link StoreMetric.dataDiscarded}. */ export enum DiscardReason { /** Too short to hold a complete record — a write that never finished. */ diff --git a/src/datastore/lmdb/LMDBStore.ts b/src/datastore/lmdb/LMDBStore.ts index f9c52f4d..e1db281a 100644 --- a/src/datastore/lmdb/LMDBStore.ts +++ b/src/datastore/lmdb/LMDBStore.ts @@ -1,14 +1,15 @@ import { Database } from 'lmdb'; +import { LoggerFactory } from '../../telemetry/LoggerFactory'; import { ScopedTelemetry } from '../../telemetry/ScopedTelemetry'; import { TelemetryService } from '../../telemetry/TelemetryService'; -import { LMDBError } from '../../utils/errors/ErrorClasses'; import { DataStore, StoreName } from '../DataStore'; -import { ErrorHandler, StoreOperation } from '../Utils'; +import { ErrorHandler, isValueTooLarge, StoreOperation } from '../Utils'; import { attachCommitCause, resolveCommitError } from './CommitError'; import { stats, StoreStatsType } from './Stats'; export class LMDBStore implements DataStore { private readonly telemetry: ScopedTelemetry; + private readonly log: ReturnType; constructor( public readonly name: StoreName, @@ -19,6 +20,7 @@ export class LMDBStore implements DataStore { private readonly beginOp: () => () => void = () => () => {}, ) { this.telemetry = TelemetryService.instance.get(`LMDB.${name}`); + this.log = LoggerFactory.getLogger(`LMDB.${name}`); } updateStore(store: Database) { @@ -33,7 +35,7 @@ export class LMDBStore implements DataStore { try { const initialRecovery = this.validateDatabase(); if (initialRecovery !== undefined) { - throw new LMDBError('Database recovery is in progress'); + throw new Error('Database recovery is in progress'); } try { @@ -73,6 +75,13 @@ export class LMDBStore implements DataStore { const cause = await resolveCommitError(e); attachCommitCause(e, cause); + // MDB_BAD_VALSIZE is deterministic - the same value fails identically after + // recovery, so retrying (and the recovery work itself) is wasted. Skip + // straight to the caller instead of going through the generic retry path. + if (isValueTooLarge(cause ?? e)) { + throw e; + } + await this.onError(e, op); this.telemetry.count(`retry.${op}`, 1); await this.validateDatabase(); @@ -90,8 +99,20 @@ export class LMDBStore implements DataStore { return this.exec(StoreOperation.get, () => this.store.get(key) as T | undefined); } - put(key: string, value: T): Promise { - return this.execAsync(StoreOperation.put, () => this.store.put(key, value)); + async put(key: string, value: T): Promise { + try { + return await this.execAsync(StoreOperation.put, () => this.store.put(key, value)); + } catch (error) { + if (isValueTooLarge(error)) { + this.telemetry.error('put.valueTooLarge', error, undefined, { + captureErrorAttributes: true, + attributes: { key }, + }); + this.log.warn({ store: this.name, key }, 'Skipping cache write: value exceeds LMDB size limits'); + return false; + } + throw error; + } } remove(key: string): Promise { diff --git a/tst/unit/datastore/LMDB.valueSize.test.ts b/tst/unit/datastore/LMDB.valueSize.test.ts new file mode 100644 index 00000000..d547e3bb --- /dev/null +++ b/tst/unit/datastore/LMDB.valueSize.test.ts @@ -0,0 +1,82 @@ +import { randomUUID as v4 } from 'crypto'; +import fs from 'fs'; +import { join } from 'path'; +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { DataStore, StoreName } from '../../../src/datastore/DataStore'; +import { LMDBStoreFactory } from '../../../src/datastore/LMDBStoreFactory'; + +describe('LMDB value size handling', () => { + let lmdbFactory: LMDBStoreFactory; + let lmdbStore: DataStore; + const testDir = join(process.cwd(), 'node_modules', '.cache', 'lmdb-valuesize-tests', v4()); + + beforeEach(async () => { + fs.mkdirSync(testDir, { recursive: true }); + lmdbFactory = new LMDBStoreFactory(testDir); + await lmdbFactory.initialize(); + lmdbStore = lmdbFactory.get(StoreName.public_schemas); + }); + + afterEach(async () => { + await lmdbFactory.close(); + fs.rmSync(testDir, { recursive: true, force: true }); + }); + + /** + * Installs a `put` mock that always throws MDB_BAD_VALSIZE. Recovery (triggered by the + * factory's `onError` handler) replaces the underlying store handle via `updateStore`, + * so the mock must be re-applied after each such replacement to keep simulating a + * deterministic (non-transient) size error across retries. + */ + function mockPutAlwaysRejectsWithBadValSize(): void { + const internal = lmdbStore as any; + const throwBadValSize = () => { + throw new Error('MDB_BAD_VALSIZE: Unsupported size of key/DB name/data, or wrong DUPFIXED size'); + }; + internal.store.put = throwBadValSize; + + const originalUpdateStore = internal.updateStore.bind(internal); + internal.updateStore = (newStore: unknown) => { + originalUpdateStore(newStore); + internal.store.put = throwBadValSize; + }; + } + + it('should skip caching gracefully (not throw) when a value exceeds LMDB size limits', async () => { + mockPutAlwaysRejectsWithBadValSize(); + + const result = await lmdbStore.put('big-key', 'value'); + expect(result).toBe(false); + expect(lmdbStore.get('big-key')).toBeUndefined(); + }); + + it('should not produce an unhandled promise rejection for MDB_BAD_VALSIZE errors', async () => { + mockPutAlwaysRejectsWithBadValSize(); + + // If put() ever rejects without being awaited/caught here, this would surface as an + // unhandled rejection in the test process. Awaiting confirms the promise always resolves. + await expect(lmdbStore.put('key', 'value')).resolves.not.toThrow(); + }); + + it('should still throw for errors unrelated to value size', async () => { + const internal = lmdbStore as any; + + // No-op onError override so recovery doesn't replace the mocked store handle, + // letting the deterministic failure propagate as expected by the generic retry path. + (lmdbFactory as any).handleError = () => { + /* no-op: simulate recovery failure so retry hits the same mocked error */ + }; + + internal.store.put = () => { + throw new Error('MDB_PANIC: unrecoverable'); + }; + + await expect(lmdbStore.put('key', 'value')).rejects.toThrow('MDB_PANIC: unrecoverable'); + }); + + it('should put normal-sized values without any change in behavior', async () => { + const result = await lmdbStore.put('normal-key', { data: 'small value' }); + expect(result).toBe(true); + expect(lmdbStore.get('normal-key')).toEqual({ data: 'small value' }); + }); +});