fix(replication): classify undecodable records + hold on unknown table id (#537)

[kriszyp]

Fixes #537 (root-cause + observability). Supersedes the approach in #538 — see Relationship to #538 below.

Problem

On the replication receive-apply path, record decode was wrapped in a bare catch that logged and skipped every decode failure, letting the resume cursor advance past the dropped record: silent (no metric), undiscriminating, and — for an unknown table id — actually a double-throw wedge (the catch re-threw on tableDecoder.decoder and escaped). This is the mechanism behind #537's "laggards plateaued thousands of rows below the source … believing themselves current."

Fix — handle each receive-path failure by its true nature

1. Decode failure = permanent → skip + surface. Transient conditions are handled upstream (blob-gap hold, backpressure), and every known decode source has been root-caused, so one reaching this catch is unrecoverable old-version/corrupt data no re-copy heals (re-fetching re-ships the same undecodable bytes; holding would wedge the leg). classifyReplicationDecodeError chooses how loudly to surface the drop:

• skip-missing-structure (isMissingStructureError): fires the same decode-missing-structure metric core's local-read path already uses — alertable instead of silent. Kept concise (no per-record struct-dictionary dump) because Interrupted bulk copies can persist a resume cursor over an undelivered range #537's trigger replays thousands of these.
• skip: any other (unexpected, post-root-cause) failure — loud error log, decoder dictionaries passed as objects (lazy) not eagerly JSON.stringify'd per record.

Both verdicts skip + advance — correct, because the record is genuinely undeliverable.

2. Unknown table id = transient → hold, don't seal. tableDecoders is populated only by TABLE_FIXED_STRUCTURE, which the sender always emits before the first record of a table (length-gated send at ~2882) over an ordered WS. So a missing decoder means a missed structure sync (reset/edge case) or un-propagated schema (#1497 family) — recoverable on reconnect, categorically unlike an undecodable record. The old code logged, fell through, and double-threw on tableDecoder.decoder inside the catch → escaped → wedged the leg. Now it holds: close(1011) → reconnect → resume from the durable cursor (which re-sends TABLE_FIXED_STRUCTURE), without advancing past the record, so nothing is lost. Also makes tableDecoder provably non-null in the decode catch.

Adds classifyReplicationDecodeError + unit coverage. harper-pro-only (isMissingStructureError already exported from core RecordEncoder.ts).

Relationship to #538

#538 detects a post-copy row-count shortfall and forces a fresh full copy. For this failure class that is counterproductive: the shortfall is permanently undecodable records, so a forced re-copy re-ships the same bytes, re-skips them, and re-fires on the next reconnect — a loop that never converges (and its fuzzy max(100, 10%) estimate-vs-estimate count excludes TTL tables). This PR fixes the root cause at the layer where it happens. Recommendation: close #538 (or repurpose its count check as observability, not a re-copy trigger). Discussion posted on #538.

Reproduction / validation

Ran the harper#1163 divergence cluster harness + probed the decode path:

• integrationTests/cluster/typedStructReplicationDivergence.test.mjs → 4/4 pass; the new unknown-table hold path does not trigger in normal operation.
• Mode confirmed: replication decoder is a raw StructonPackr → absent structure throws ("Could not find typed structure N") → isMissingStructureError → skip-missing-structure. (Core RecordEncoder's "returns null" is a different call site — #1163 local reads.)
• Permanent confirmed for the right reason: sender sends TABLE_FIXED_STRUCTURE before dependent records, so a throw = structure genuinely absent from the sender's synced binary = corrupt/old-version = unrecoverable.
• Interrupted bulk copies can persist a resume cursor over an undelivered range #537 signature match: a count gap (not values corruption) → mode A.
• Latent, out of scope: mode B (diverged same-id structure) would silently misdecode if TABLE_FIXED_STRUCTURE seeding ever failed — separate guard.

Cross-model review (thorough: Codex + Gemini + Harper-domain adjudication)

• No blocker introduced.
• Codex's unknown-tableId double-throw and Gemini's per-record JSON.stringify hot-path cost: both addressed in this revision (hold at the source; concise/lazy logging).
• Rejected false positive: Gemini's "missing file extension" on the test import (resolved by the #src/* subpath mapping; 10 existing tests use it).

Test

npm run test:unit — classifier suite 5/5; related replication suites green. Divergence integration 4/4. Build emits.

KrAIs (Claude Opus 4.8), with Kris

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a classification system for replication decode errors, distinguishing between missing shared structures (which trigger a specific metric and warning) and other unexpected decode failures (which are logged as errors). It also adds unit tests to verify this classification logic. The reviewer pointed out a potential issue where a secondary TypeError could be thrown if tableDecoder is undefined inside the catch block, and suggested using optional chaining to prevent masking the original error.

[gemini-code-assist]

If tableDecoder is undefined (e.g., due to an unknown tableId or transient schema-propagation lag), attempting to access properties on it directly inside the catch block will throw a secondary TypeError. This secondary error will escape the catch block, masking the original error and potentially crashing the connection or process.

Using optional chaining (tableDecoder?.name and tableDecoder?.decoder) prevents this double-throw and ensures that the original error is logged correctly.

					if (classifyReplicationDecodeError(error) === 'skip-missing-structure') {
						// Missing shared structure (harper#1163): genuinely absent on this node, re-copy re-ships the
						// same undecodable bytes. Surface it through the same metric core's local-read path fires so
						// the drop is alertable rather than laundered as emptiness. Kept concise: this is the #537
						// flood path (an interrupted copy can replay thousands of these), so we do NOT stringify the
						// decoder's full struct dictionaries per record — the metric carries the volume and error
						// already names the specific missing structure.
						recordAction(true, DECODE_MISSING_STRUCTURE_METRIC, databaseName + '.' + (tableDecoder?.name ?? 'unknown'));
						logger.warn?.(
							connectionId,
							"Skipping replication record referencing a shared structure missing on this node (permanent; see harper#1163), table: " + (tableDecoder?.name ?? "unknown") + ", record id: " + id,
							error
						);
					} else {
						// Unexpected post-root-cause decode failure — log loudly for investigation. Still skipped (the
						// record is undecodable on this node); the cursor advances past it below. Pass the decoder
						// dictionaries as objects (not eager JSON.stringify) so the formatting cost is paid only when
						// the log actually emits, not on every dropped record.
						logger.error?.(
							connectionId,
							'Error decoding replication message, record id: ' + id,
							'typed structures for current decoder',
							tableDecoder?.decoder?.typedStructs,
							'structures for current decoder',
							tableDecoder?.decoder?.structures,
							'encoded message',
							auditRecord.encoded.subarray(0, 1000),
							auditRecord,
							error
						);
					}

[claude]

Reviewed; no blockers found.

[kriszyp]

Reproduction / empirical validation

Ran the harper#1163 structure-divergence cluster harness and probed the actual decode path to confirm this fix targets the real failure mode.

Harness baseline: HARPER_RUN_STRESS_TESTS=1 npm run test:integration -- integrationTests/cluster/typedStructReplicationDivergence.test.mjs → 4/4 pass, ~58s on this branch (pre-diverged full copy, late-joiner, restart-reload).

Which decode-failure mode does the replication receive path hit? The replication decoder is a raw StructonPackr (not core's RecordEncoder), so an absent structure throws ("Could not find typed structure N", structon struct.js) rather than returning null:

• absent structure → throws → isMissingStructureError true → classifyReplicationDecodeError → skip-missing-structure (the path this PR covers). ✅
• diverged same-id structure → silently misdecodes at the StructonPackr level, but the TABLE_FIXED_STRUCTURE handshake seeds the decoder from the sender's dict before the dependent record, so this doesn't reach the catch in normal operation (the passing Attempt-A confirms). The test header's "returns null" language describes core RecordEncoder's local-read path (#1163), a different call site.

Permanent vs recoverable: confirmed permanent, for the right reason. The sender emits TABLE_FIXED_STRUCTURE before any record whose encoding introduced a new structure (per-record, gated on the encoder's append-only structure-array length growing — replicationConnection.ts:2882-2908). So a receiver throw means the referenced structure is not in the sender's synced structures at all → genuinely absent (old-version/corrupt bytes) → no re-copy can heal it, and holding would wedge the leg. Skip + advance is correct; this PR's only change is making the drop observable (the decode-missing-structure metric) instead of silent.

#537 signature match: #537 is a count gap ("thousands of rows below source"), not a values-corruption gap — consistent with mode A (records thrown + skipped, cursor advancing), not mode B (records applied with wrong values, count intact).

Latent, out of scope: mode B (diverged same-id structure) would silently corrupt if the TABLE_FIXED_STRUCTURE seeding ever failed — worth a separate guard, but not reachable via this path in normal cluster operation.

— KrAIs (Claude Opus 4.8), with Kris

[kriszyp]

Test coverage added (and an honest note on the gap)

Two tests landed (commit 85019093), plus a small env-gated test hook (maybeInjectCopyCursorForTest, same pattern as the existing maybeStallCopyForTest / armReplicationWedgeForTest):

1. integrationTests/cluster/poisonedCopyCursorDataLoss.test.mjs — reproduces #537's symptom (Lavinia's Part-1 recipe). A receiver presents a copyCursor with afterKey='row-0500' against a 1000-row source; the leader trusts it, skips row-0001..0500, delivers only the tail; B ends with exactly 500 rows, cluster_status connected, row-0001 absent — a permanent, unsurfaced gap. Passes 1/1 (~24s).

• Surfaced a useful nuance: the gap is only permanent for rows older than copyStartTime (base-copy data) — rows newer are healed by the post-copy audit replay. The test future-dates copyStartTime to faithfully simulate the base-data condition (which is exactly Interrupted bulk copies can persist a resume cursor over an undelivered range #537's — copying pre-existing rows). This is a characterization test (the cursor-trust behavior is correct for a legitimate interrupted copy; it's only a loss vector when the cursor is wrong), so it guards the symptom, not the fix.

2. unitTests/replication/decodeDropMissingStructure.test.mjs — drives the real StructonPackr encode/decode (encode learns struct 0 → decode without it → genuine "Could not find typed structure 0"), then asserts isMissingStructureError → classifyReplicationDecodeError → skip-missing-structure. Passes 6/6. Upgrades the isolated-Error unit test to real structon behavior.

Honest gap: we do not yet have a full end-to-end test that induces a decode-drop during a live cluster copy and asserts the decode-missing-structure metric fires + the row goes missing. The TABLE_FIXED_STRUCTURE ordering makes that hard to synthesize; the cleanest path would be a sender-side hook that withholds one TABLE_FIXED_STRUCTURE so the receiver's decoder genuinely lacks a structure. Test 2 covers the classifier on a real throw; the metric-firing in the live path is currently verified by inspection, not by test.

— KrAIs (Claude Opus 4.8), with Kris