Skip to content

fix: Return decode errors from lazy history reads - #1177

Open
zxch3n wants to merge 2 commits into
mainfrom
fix/dag-unparsable-block-error
Open

zxch3n wants to merge 2 commits into
mainfrom
fix/dag-unparsable-block-error

Conversation

@zxch3n

@zxch3n zxch3n commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Summary

A snapshot with a forged/truncated change block and repaired checksums can import lazily. A first history read now returns DecodeError("cannot parse change block ...") instead of panicking in the DAG loader. An attached import also rolls back if a block's header is readable but its body fails later, keeping op log and state versions aligned.

  • Propagate fallible DAG reads through version conversion, replay-base selection, causal iteration, diff, checkout, revert, import, fork-at, and shallow/state-only/snapshot-at export. Keep the original assertion for a missing id without a parse failure.
  • Guard attached imports using the change store's conservative unparsed-body flag. Loading all DAG headers cannot clear it; a successful full body walk can, and arena rollback that evicts bodies restores it. This includes register-only imports after header-only queries. Share the import's existing Arc<VersionVector> checkpoint with both rollback journals, removing two extra full-vector copies.
  • Fallible range reads reject a damaged range before supplying changes to an export's scratch history. Legacy infallible iter_blocks/iter_changes record and skip only the bad block, retaining the healthy portions. Document the causal iterator invariant: try_new loads every target node before iteration.
  • Additive Rust try_* queries preserve existing query signatures. Existing fallible WASM queries use them. Update the context article and patch changeset, including the Rust error-type compatibility details below.

Rationale and compatibility

Follow-up to #1172. An earlier reader must not be required to record a failure before a fallible operation can handle it. DAG nodes come from block headers: an empty DAG unparsed_vv does not establish that all bodies are readable. The attached-import journal stays conservative until a full body walk validates them, avoiding a history scan on each import.

ChangeTravelError gains HistoryUnreadable(LoroError) and is now #[non_exhaustive]. This is an explicit Rust source compatibility change: downstream exhaustive matches need a wildcard arm. FrontiersNotIncluded changes from a unit struct to a private-field struct with a same-named constant; construction and constant patterns still compile, but an unreadable-history error does not equal that missing-frontiers constant. Both choices are stated in the changeset.

Undo, checkout_to_latest, and fork retain their infallible boundaries. Reads record actual corruption before the existing panic boundary; a record alone does not break a healthy shared internal read. Snapshot history remains lazy, so operations that do not read a damaged block can still succeed before a failure is recorded.

Regression coverage includes 25 native first readers on separate documents with first/middle/last truncated blocks (75 cases); a second public-API snapshot rewrite XORs byte -3 of each of those three block positions, repairs both checksum layers, warms both peers' DAG headers successfully, then imports a concurrent map update. It asserts a decode error, unchanged deep value/version vector/status, and unchanged, equal op log/state frontiers. Store tests check skip-only-bad-block behavior in all three positions and body validation after header reads/arena rollback. WASM has 20 cases, including the header-only map-import rollback and a usable instance afterwards.

Performance

The successful lazy DAG loader still does the same store lookup and node insertion. It performs no additional block read, parse, history scan, or parse-failure lock; the record lookup remains confined to the missing-node branch. Body tracking is conservative and does not add accounting to each successful parse.

Release measurements on this macOS arm64 host used setup outside timers and no concurrent validation. Three alternating before/after pairs, each with three repeats of 30 operations, give nine samples per revision. Values below are median totals in microseconds; ratio is after/before.

The many-peer fixture measures a one-op causal map import into a snapshot-loaded doc. It has 1k/10k peers with one existing map op each. The header-warmed case loads every DAG header without parsing old bodies: pre-review be17aed3dca7413cbd2ecbb93b2f1b633102c03c skips the journal there, while the revised implementation opens it. Sharing the existing version checkpoint avoids the two extra O(peers) vector copies. The separate DAG unparsed_vv checkpoint remains; it is empty after warming.

Peers / history Before review fixes Revised fix Ratio
1k / cold 42107 42136 1.001
1k / headers warmed 13532 13485 0.997
10k / cold 462881 467760 1.011
10k / headers warmed 171435 170691 0.996

Also remeasured the original two-peer case against base main c00c9fa501f8d32f68d6255eacb7035a67fb6ab6 (600 peer-1 and 300 peer-2 text commits plus a map entry):

Operation Main Revised fix Ratio
Snapshot import 349 351 1.006
First historical checkout 28638 29247 1.021
Concurrent text import 26411 26230 0.993
Concurrent map import 2582 2599 1.007

No material increase appeared in these runs; they do not establish a bound for every workload. Reproduce each case from crates/loro/tests/perf_history_lazy_load.rs on both revisions with cargo test --release -p loro --test perf_history_lazy_load -- --ignored --exact perf_map_import_many_peers --nocapture --test-threads=1 (replace the exact name with perf_history_lazy_load for the two-peer case).

Validation

All final checks passed:

  • cargo test -p loro-internal --lib change_store: 24 passed, 0 failed, 392 filtered.
  • cargo test -p loro-internal --lib loro_dag: 6 passed, 0 failed, 410 filtered.
  • cargo test -p loro --test unregistered_container_parent: 15 passed, 0 failed, including all three header-only corruption positions.
  • Release truncated-block first-reader matrix: 1 passed, 0 failed, 14 filtered (75 cases plus shallow-import rollback). Release header-only corruption regression: 1 passed, 0 failed, 14 filtered.
  • pnpm test: 1751 passed, 39 skipped; 62 doctests passed, 0 failed.
  • pnpm check: cargo clippy --all-features -- -Dwarnings passed.
  • pnpm test-loom: 9 passed, 0 failed (LOOM_MAX_PREEMPTIONS=2, release).
  • pnpm release-wasm: four targets regenerated; CommonJS smoke and TypeScript check passed; Vitest 44 files passed, 479 tests passed, 2 skipped, 2 todo; Deno 4 passed; Bun 4 passed. The unreadable-history file has 20 passing tests.
  • git diff --check: passed.

Cargo rejects cargo test -p loro-internal --lib change_store loro_dag because it accepts one test filter; the two equivalent commands above ran separately. The new regression was first run against the pre-review implementation and failed with op log frontiers ahead of state, then passed after the fix. Scripts use the locally cached pnpm 10.33.0. No unrelated Cargo.lock version change is included; the original PR's xxhash-rust dev dependency remains necessary for checksum repair in the public regressions.

See context/arena-parent-links.md for first-reader errors and infallible compatibility.

Co-Authored-By: GPT-6 <noreply@openai.com>
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

WASM Size Report

  • Original size: 3338.09 KB
  • Gzipped size: 1111.43 KB
  • Brotli size: 778.71 KB

See context/arena-parent-links.md for body validation and compatibility.

Co-Authored-By: GPT-6 <noreply@openai.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant