diff --git a/resources/RecordEncoder.ts b/resources/RecordEncoder.ts index d351d2f1bd..67be5e8de4 100644 --- a/resources/RecordEncoder.ts +++ b/resources/RecordEncoder.ts @@ -34,6 +34,28 @@ import { CONFIG_PARAMS } from '../utility/hdbTerms.ts'; import * as envMngr from '../utility/environment/environmentManager.js'; const StructonEncoder = createStructon(Encoder) as typeof Encoder; + +// Analytics counter incremented whenever a record cannot be decoded because its shared structure is +// missing on this node (see HarperFast/harper#1163). Surfaces the otherwise-silent condition in +// monitoring; the store name is passed as the metric path. +const MISSING_STRUCTURE_METRIC = 'decode-missing-structure'; + +// Terminal error messages msgpackr/structon throw when a record references a shared structure that +// is not in this node's structures buffer. Both the typed (random-access) path (structon's +// readStruct) and the classic path (msgpackr's createSecondByteReader) reload the structures from +// durable storage and retry before throwing, so reaching one of these means the structure is +// genuinely absent on this node — not merely stale in memory. Matched by message prefix because +// neither dependency throws a typed error. +const MISSING_TYPED_STRUCTURE_PREFIX = 'Could not find typed structure '; +const MISSING_CLASSIC_STRUCTURE_PREFIX = 'Record id is not defined for '; + +export function isMissingStructureError(error: any): boolean { + const message = error?.message; + return ( + typeof message === 'string' && + (message.startsWith(MISSING_TYPED_STRUCTURE_PREFIX) || message.startsWith(MISSING_CLASSIC_STRUCTURE_PREFIX)) + ); +} export type Entry = { key: any; value: any; @@ -384,7 +406,29 @@ export class RecordEncoder extends StructonEncoder { } // else a normal entry return options?.valueAsBuffer ? buffer : decodeFromDatabase(() => super.decode(buffer, options), this.rootStore); } catch (error) { - harperLogger.error('Error decoding record', error, 'data: ' + buffer.slice(0, 40).toString('hex')); + const hexPreview = buffer.slice(0, 40).toString('hex'); + if (isMissingStructureError(error)) { + // This record references a shared structure that is genuinely absent on this node — the + // dependency already reloaded from durable storage and retried before throwing (typically a + // replica that received the record but not the structure-buffer update; see + // HarperFast/harper#1163). We still return null so internal reads (e.g. the metadata/__dbis__ + // scan during initialization) remain non-fatal, but surface the otherwise-silent condition + // distinctly — a dedicated warning plus an analytics counter — so the dropped record is + // detectable and alertable rather than laundered as legitimate emptiness into query results, + // caches, and downstream consumers. + // this.name is set on the RocksDB encoder; for LMDB fall back to the root store's name so + // the metric/warning still attribute the dropped record to a store. + const storeName = this.name ?? this.rootStore?.name; + recordAction(true, MISSING_STRUCTURE_METRIC, storeName); + harperLogger.warn( + 'Record references a shared structure missing on this node; decoded as null (see HarperFast/harper#1163)', + error, + 'store: ' + storeName, + 'data: ' + hexPreview + ); + return null; + } + harperLogger.error('Error decoding record', error, 'data: ' + hexPreview); return null; } } diff --git a/unitTests/resources/recordEncoder.test.js b/unitTests/resources/recordEncoder.test.js index 85a69c732d..99886dc71b 100644 --- a/unitTests/resources/recordEncoder.test.js +++ b/unitTests/resources/recordEncoder.test.js @@ -1,6 +1,7 @@ require('../testUtils'); const assert = require('assert'); -const { RecordEncoder } = require('#src/resources/RecordEncoder'); +const { RecordEncoder, isMissingStructureError } = require('#src/resources/RecordEncoder'); +const harperLogger = require('#src/utility/logging/harper_logger'); const { Encoder } = require('msgpackr'); // Shared structures persisted as an encoded buffer (mirrors how a DBI stores them under @@ -83,3 +84,71 @@ describe('RecordEncoder struct-mode gating', () => { assert.strictEqual(decoded.indexed, record.indexed); }); }); + +describe('RecordEncoder missing-structure handling (harper#1163)', () => { + let warnings, errors, restoreWarn, restoreError; + beforeEach(() => { + warnings = []; + errors = []; + restoreWarn = harperLogger.warn; + restoreError = harperLogger.error; + harperLogger.warn = (...args) => warnings.push(args); + harperLogger.error = (...args) => errors.push(args); + }); + afterEach(() => { + harperLogger.warn = restoreWarn; + harperLogger.error = restoreError; + }); + + it('returns null (non-fatal) and warns distinctly when a typed structure is absent on this node', () => { + // A record references a typed (random-access) structure that this node's structures buffer does + // not contain. structon's readStruct reloads from the (still-empty) store and then throws; + // RecordEncoder must keep internal reads non-fatal (return null) while surfacing the dropped + // record distinctly rather than via the generic error path. + const writer = makeEncoder(true, sharedStore()); + const bytes = Buffer.from(writer.encode(record)); + + // Reader on a different node that never received the structure-buffer update. + const reader = makeEncoder(true, sharedStore()); + assert.strictEqual(reader.decode(bytes), null, 'missing structure should decode to null, not throw'); + assert.strictEqual(warnings.length, 1, 'should emit exactly one distinct warning'); + assert.match(warnings[0][0], /shared structure missing/); + assert.strictEqual(errors.length, 0, 'should not use the generic error path for a missing structure'); + }); + + it('recovers (decodes, no throw) once the typed structure is present on this node', () => { + // Writer and reader share the same structures store, so the reader's on-miss reload finds it. + const store = sharedStore(); + const writer = makeEncoder(true, store); + const bytes = Buffer.from(writer.encode(record)); + const reader = makeEncoder(true, store); + const decoded = reader.decode(bytes); + assert.ok(decoded, 'record should decode when the structure is available'); + assert.strictEqual(decoded.name, record.name); + }); + + it('detects both typed and classic missing-structure errors, and only those', () => { + // classic-structure miss (msgpackr createSecondByteReader) is the relevant variant on 5.1 where + // typed structs are off by default; we cannot easily manufacture a real classic shared-structure + // miss in this harness, so assert the detection contract directly against the dependency's + // terminal error messages. + assert.ok(isMissingStructureError(new Error('Could not find typed structure 1'))); + assert.ok(isMissingStructureError(new Error('Record id is not defined for 42'))); + assert.ok(!isMissingStructureError(new Error('Data read, but end of buffer not reached 64'))); + assert.ok(!isMissingStructureError(new RangeError('Offset is outside the bounds of the DataView'))); + assert.ok(!isMissingStructureError(undefined)); + }); + + it('still returns null (tolerant) for a decode failure that is not a missing structure', () => { + // Truncate a valid struct-mode record mid-body: the structure IS present, but the buffer is too + // short, so decoding throws a non-missing-structure error (e.g. out-of-bounds). That genuine + // corruption keeps the existing log-and-null behavior. + const store = sharedStore(); + const enc = makeEncoder(true, store); + const bytes = Buffer.from(enc.encode(record)); + const truncated = bytes.subarray(0, 2); + assert.strictEqual(enc.decode(truncated), null, 'corrupt (non-structure) decode should still return null'); + assert.strictEqual(errors.length, 1, 'genuine corruption should use the generic error path'); + assert.strictEqual(warnings.length, 0, 'genuine corruption should not use the missing-structure warning'); + }); +});