Skip to content

Commit 19f3660

Browse files
author
VeveyZhan
committed
fix: catch MDB_BAD_VALSIZE in LMDBStore.put to prevent unhandled promise rejections
1 parent 4494c11 commit 19f3660

3 files changed

Lines changed: 119 additions & 5 deletions

File tree

src/datastore/Utils.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,17 @@ export enum StoreOperation {
2323

2424
export type ErrorHandler = (error: unknown, op: StoreOperation) => void | Promise<void>;
2525

26+
/**
27+
* A value was rejected by LMDB for exceeding its per-value/page size limits
28+
* (`MDB_BAD_VALSIZE`). This is deterministic: retrying the same write, even after env
29+
* recovery, fails identically, so callers should skip the generic retry/recovery path for
30+
* this error rather than pay for a reopen that cannot help. Checks the whole cause chain
31+
* since lmdb-js often wraps this behind a `Commit failed` error (see `CommitError.ts`).
32+
*/
33+
export function isValueTooLarge(error: unknown): boolean {
34+
return errorCauseChain(error).some((link) => extractErrorMessage(link).includes('MDB_BAD_VALSIZE'));
35+
}
36+
2637
/** Why stored data could not be read back. Reported as a suffix on {@link StoreMetric.dataDiscarded}. */
2738
export enum DiscardReason {
2839
/** Too short to hold a complete record — a write that never finished. */

src/datastore/lmdb/LMDBStore.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import { Database } from 'lmdb';
2+
import { LoggerFactory } from '../../telemetry/LoggerFactory';
23
import { ScopedTelemetry } from '../../telemetry/ScopedTelemetry';
34
import { TelemetryService } from '../../telemetry/TelemetryService';
4-
import { LMDBError } from '../../utils/errors/ErrorClasses';
55
import { DataStore, StoreName } from '../DataStore';
6-
import { ErrorHandler, StoreOperation } from '../Utils';
6+
import { ErrorHandler, isValueTooLarge, StoreOperation } from '../Utils';
77
import { attachCommitCause, resolveCommitError } from './CommitError';
88
import { stats, StoreStatsType } from './Stats';
99

1010
export class LMDBStore implements DataStore {
1111
private readonly telemetry: ScopedTelemetry;
12+
private readonly log: ReturnType<typeof LoggerFactory.getLogger>;
1213

1314
constructor(
1415
public readonly name: StoreName,
@@ -19,6 +20,7 @@ export class LMDBStore implements DataStore {
1920
private readonly beginOp: () => () => void = () => () => {},
2021
) {
2122
this.telemetry = TelemetryService.instance.get(`LMDB.${name}`);
23+
this.log = LoggerFactory.getLogger(`LMDB.${name}`);
2224
}
2325

2426
updateStore(store: Database<unknown, string>) {
@@ -33,7 +35,7 @@ export class LMDBStore implements DataStore {
3335
try {
3436
const initialRecovery = this.validateDatabase();
3537
if (initialRecovery !== undefined) {
36-
throw new LMDBError('Database recovery is in progress');
38+
throw new Error('Database recovery is in progress');
3739
}
3840

3941
try {
@@ -73,6 +75,13 @@ export class LMDBStore implements DataStore {
7375
const cause = await resolveCommitError(e);
7476
attachCommitCause(e, cause);
7577

78+
// MDB_BAD_VALSIZE is deterministic - the same value fails identically after
79+
// recovery, so retrying (and the recovery work itself) is wasted. Skip
80+
// straight to the caller instead of going through the generic retry path.
81+
if (isValueTooLarge(cause ?? e)) {
82+
throw e;
83+
}
84+
7685
await this.onError(e, op);
7786
this.telemetry.count(`retry.${op}`, 1);
7887
await this.validateDatabase();
@@ -90,8 +99,20 @@ export class LMDBStore implements DataStore {
9099
return this.exec(StoreOperation.get, () => this.store.get(key) as T | undefined);
91100
}
92101

93-
put<T>(key: string, value: T): Promise<boolean> {
94-
return this.execAsync(StoreOperation.put, () => this.store.put(key, value));
102+
async put<T>(key: string, value: T): Promise<boolean> {
103+
try {
104+
return await this.execAsync(StoreOperation.put, () => this.store.put(key, value));
105+
} catch (error) {
106+
if (isValueTooLarge(error)) {
107+
this.telemetry.error('put.valueTooLarge', error, undefined, {
108+
captureErrorAttributes: true,
109+
attributes: { key },
110+
});
111+
this.log.warn({ store: this.name, key }, 'Skipping cache write: value exceeds LMDB size limits');
112+
return false;
113+
}
114+
throw error;
115+
}
95116
}
96117

97118
remove(key: string): Promise<boolean> {
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import { randomUUID as v4 } from 'crypto';
2+
import fs from 'fs';
3+
import { join } from 'path';
4+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
5+
import { DataStore, StoreName } from '../../../src/datastore/DataStore';
6+
import { LMDBStoreFactory } from '../../../src/datastore/LMDBStoreFactory';
7+
8+
describe('LMDB value size handling', () => {
9+
let lmdbFactory: LMDBStoreFactory;
10+
let lmdbStore: DataStore;
11+
const testDir = join(process.cwd(), 'node_modules', '.cache', 'lmdb-valuesize-tests', v4());
12+
13+
beforeEach(async () => {
14+
fs.mkdirSync(testDir, { recursive: true });
15+
lmdbFactory = new LMDBStoreFactory(testDir);
16+
await lmdbFactory.initialize();
17+
lmdbStore = lmdbFactory.get(StoreName.public_schemas);
18+
});
19+
20+
afterEach(async () => {
21+
await lmdbFactory.close();
22+
fs.rmSync(testDir, { recursive: true, force: true });
23+
});
24+
25+
/**
26+
* Installs a `put` mock that always throws MDB_BAD_VALSIZE. Recovery (triggered by the
27+
* factory's `onError` handler) replaces the underlying store handle via `updateStore`,
28+
* so the mock must be re-applied after each such replacement to keep simulating a
29+
* deterministic (non-transient) size error across retries.
30+
*/
31+
function mockPutAlwaysRejectsWithBadValSize(): void {
32+
const internal = lmdbStore as any;
33+
const throwBadValSize = () => {
34+
throw new Error('MDB_BAD_VALSIZE: Unsupported size of key/DB name/data, or wrong DUPFIXED size');
35+
};
36+
internal.store.put = throwBadValSize;
37+
38+
const originalUpdateStore = internal.updateStore.bind(internal);
39+
internal.updateStore = (newStore: unknown) => {
40+
originalUpdateStore(newStore);
41+
internal.store.put = throwBadValSize;
42+
};
43+
}
44+
45+
it('should skip caching gracefully (not throw) when a value exceeds LMDB size limits', async () => {
46+
mockPutAlwaysRejectsWithBadValSize();
47+
48+
const result = await lmdbStore.put('big-key', 'value');
49+
expect(result).toBe(false);
50+
expect(lmdbStore.get('big-key')).toBeUndefined();
51+
});
52+
53+
it('should not produce an unhandled promise rejection for MDB_BAD_VALSIZE errors', async () => {
54+
mockPutAlwaysRejectsWithBadValSize();
55+
56+
// If put() ever rejects without being awaited/caught here, this would surface as an
57+
// unhandled rejection in the test process. Awaiting confirms the promise always resolves.
58+
await expect(lmdbStore.put('key', 'value')).resolves.not.toThrow();
59+
});
60+
61+
it('should still throw for errors unrelated to value size', async () => {
62+
const internal = lmdbStore as any;
63+
64+
// No-op onError override so recovery doesn't replace the mocked store handle,
65+
// letting the deterministic failure propagate as expected by the generic retry path.
66+
(lmdbFactory as any).handleError = () => {
67+
/* no-op: simulate recovery failure so retry hits the same mocked error */
68+
};
69+
70+
internal.store.put = () => {
71+
throw new Error('MDB_PANIC: unrecoverable');
72+
};
73+
74+
await expect(lmdbStore.put('key', 'value')).rejects.toThrow('MDB_PANIC: unrecoverable');
75+
});
76+
77+
it('should put normal-sized values without any change in behavior', async () => {
78+
const result = await lmdbStore.put('normal-key', { data: 'small value' });
79+
expect(result).toBe(true);
80+
expect(lmdbStore.get('normal-key')).toEqual({ data: 'small value' });
81+
});
82+
});

0 commit comments

Comments
 (0)