Skip to content

Commit 80c519a

Browse files
kriszypclaude
andcommitted
fix(storage): surface missing shared-structure decode misses (harper#1163)
A record that references a shared structure absent from this node's in-memory structures buffer (msgpackr/structon already reload from durable storage and retry, then throw) was caught, logged at error, and decoded to null — silently dropping a genuinely-existing record from query results and laundering that emptiness into caches and downstream consumers. Detect the two terminal missing-structure errors (typed: "Could not find typed structure"; classic: "Record id is not defined for") and route them to a distinct, non-fatal path: an analytics counter (decode-missing-structure) plus a dedicated warning, still returning null so internal reads (e.g. the __dbis__ metadata scan during initialization) remain tolerant. Other decode failures keep the existing error+null behavior. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent f847836 commit 80c519a

2 files changed

Lines changed: 115 additions & 2 deletions

File tree

resources/RecordEncoder.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,28 @@ import { CONFIG_PARAMS } from '../utility/hdbTerms.ts';
3434
import * as envMngr from '../utility/environment/environmentManager.js';
3535

3636
const StructonEncoder = createStructon(Encoder) as typeof Encoder;
37+
38+
// Analytics counter incremented whenever a record cannot be decoded because its shared structure is
39+
// missing on this node (see HarperFast/harper#1163). Surfaces the otherwise-silent condition in
40+
// monitoring; the store name is passed as the metric path.
41+
const MISSING_STRUCTURE_METRIC = 'decode-missing-structure';
42+
43+
// Terminal error messages msgpackr/structon throw when a record references a shared structure that
44+
// is not in this node's structures buffer. Both the typed (random-access) path (structon's
45+
// readStruct) and the classic path (msgpackr's createSecondByteReader) reload the structures from
46+
// durable storage and retry before throwing, so reaching one of these means the structure is
47+
// genuinely absent on this node — not merely stale in memory. Matched by message prefix because
48+
// neither dependency throws a typed error.
49+
const MISSING_TYPED_STRUCTURE_PREFIX = 'Could not find typed structure ';
50+
const MISSING_CLASSIC_STRUCTURE_PREFIX = 'Record id is not defined for ';
51+
52+
export function isMissingStructureError(error: any): boolean {
53+
const message = error?.message;
54+
return (
55+
typeof message === 'string' &&
56+
(message.startsWith(MISSING_TYPED_STRUCTURE_PREFIX) || message.startsWith(MISSING_CLASSIC_STRUCTURE_PREFIX))
57+
);
58+
}
3759
export type Entry = {
3860
key: any;
3961
value: any;
@@ -384,7 +406,29 @@ export class RecordEncoder extends StructonEncoder {
384406
} // else a normal entry
385407
return options?.valueAsBuffer ? buffer : decodeFromDatabase(() => super.decode(buffer, options), this.rootStore);
386408
} catch (error) {
387-
harperLogger.error('Error decoding record', error, 'data: ' + buffer.slice(0, 40).toString('hex'));
409+
const hexPreview = buffer.slice(0, 40).toString('hex');
410+
if (isMissingStructureError(error)) {
411+
// This record references a shared structure that is genuinely absent on this node — the
412+
// dependency already reloaded from durable storage and retried before throwing (typically a
413+
// replica that received the record but not the structure-buffer update; see
414+
// HarperFast/harper#1163). We still return null so internal reads (e.g. the metadata/__dbis__
415+
// scan during initialization) remain non-fatal, but surface the otherwise-silent condition
416+
// distinctly — a dedicated warning plus an analytics counter — so the dropped record is
417+
// detectable and alertable rather than laundered as legitimate emptiness into query results,
418+
// caches, and downstream consumers.
419+
// this.name is set on the RocksDB encoder; for LMDB fall back to the root store's name so
420+
// the metric/warning still attribute the dropped record to a store.
421+
const storeName = this.name ?? this.rootStore?.name;
422+
recordAction(true, MISSING_STRUCTURE_METRIC, storeName);
423+
harperLogger.warn(
424+
'Record references a shared structure missing on this node; decoded as null (see HarperFast/harper#1163)',
425+
error,
426+
'store: ' + storeName,
427+
'data: ' + hexPreview
428+
);
429+
return null;
430+
}
431+
harperLogger.error('Error decoding record', error, 'data: ' + hexPreview);
388432
return null;
389433
}
390434
}

unitTests/resources/recordEncoder.test.js

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
require('../testUtils');
22
const assert = require('assert');
3-
const { RecordEncoder } = require('#src/resources/RecordEncoder');
3+
const { RecordEncoder, isMissingStructureError } = require('#src/resources/RecordEncoder');
4+
const harperLogger = require('#src/utility/logging/harper_logger');
45
const { Encoder } = require('msgpackr');
56

67
// Shared structures persisted as an encoded buffer (mirrors how a DBI stores them under
@@ -83,3 +84,71 @@ describe('RecordEncoder struct-mode gating', () => {
8384
assert.strictEqual(decoded.indexed, record.indexed);
8485
});
8586
});
87+
88+
describe('RecordEncoder missing-structure handling (harper#1163)', () => {
89+
let warnings, errors, restoreWarn, restoreError;
90+
beforeEach(() => {
91+
warnings = [];
92+
errors = [];
93+
restoreWarn = harperLogger.warn;
94+
restoreError = harperLogger.error;
95+
harperLogger.warn = (...args) => warnings.push(args);
96+
harperLogger.error = (...args) => errors.push(args);
97+
});
98+
afterEach(() => {
99+
harperLogger.warn = restoreWarn;
100+
harperLogger.error = restoreError;
101+
});
102+
103+
it('returns null (non-fatal) and warns distinctly when a typed structure is absent on this node', () => {
104+
// A record references a typed (random-access) structure that this node's structures buffer does
105+
// not contain. structon's readStruct reloads from the (still-empty) store and then throws;
106+
// RecordEncoder must keep internal reads non-fatal (return null) while surfacing the dropped
107+
// record distinctly rather than via the generic error path.
108+
const writer = makeEncoder(true, sharedStore());
109+
const bytes = Buffer.from(writer.encode(record));
110+
111+
// Reader on a different node that never received the structure-buffer update.
112+
const reader = makeEncoder(true, sharedStore());
113+
assert.strictEqual(reader.decode(bytes), null, 'missing structure should decode to null, not throw');
114+
assert.strictEqual(warnings.length, 1, 'should emit exactly one distinct warning');
115+
assert.match(warnings[0][0], /shared structure missing/);
116+
assert.strictEqual(errors.length, 0, 'should not use the generic error path for a missing structure');
117+
});
118+
119+
it('recovers (decodes, no throw) once the typed structure is present on this node', () => {
120+
// Writer and reader share the same structures store, so the reader's on-miss reload finds it.
121+
const store = sharedStore();
122+
const writer = makeEncoder(true, store);
123+
const bytes = Buffer.from(writer.encode(record));
124+
const reader = makeEncoder(true, store);
125+
const decoded = reader.decode(bytes);
126+
assert.ok(decoded, 'record should decode when the structure is available');
127+
assert.strictEqual(decoded.name, record.name);
128+
});
129+
130+
it('detects both typed and classic missing-structure errors, and only those', () => {
131+
// classic-structure miss (msgpackr createSecondByteReader) is the relevant variant on 5.1 where
132+
// typed structs are off by default; we cannot easily manufacture a real classic shared-structure
133+
// miss in this harness, so assert the detection contract directly against the dependency's
134+
// terminal error messages.
135+
assert.ok(isMissingStructureError(new Error('Could not find typed structure 1')));
136+
assert.ok(isMissingStructureError(new Error('Record id is not defined for 42')));
137+
assert.ok(!isMissingStructureError(new Error('Data read, but end of buffer not reached 64')));
138+
assert.ok(!isMissingStructureError(new RangeError('Offset is outside the bounds of the DataView')));
139+
assert.ok(!isMissingStructureError(undefined));
140+
});
141+
142+
it('still returns null (tolerant) for a decode failure that is not a missing structure', () => {
143+
// Truncate a valid struct-mode record mid-body: the structure IS present, but the buffer is too
144+
// short, so decoding throws a non-missing-structure error (e.g. out-of-bounds). That genuine
145+
// corruption keeps the existing log-and-null behavior.
146+
const store = sharedStore();
147+
const enc = makeEncoder(true, store);
148+
const bytes = Buffer.from(enc.encode(record));
149+
const truncated = bytes.subarray(0, 2);
150+
assert.strictEqual(enc.decode(truncated), null, 'corrupt (non-structure) decode should still return null');
151+
assert.strictEqual(errors.length, 1, 'genuine corruption should use the generic error path');
152+
assert.strictEqual(warnings.length, 0, 'genuine corruption should not use the missing-structure warning');
153+
});
154+
});

0 commit comments

Comments
 (0)