Repository navigation
fix: keep shallow re-export and parent lookups working on inconsistent stored data - #1172
Merged
Merged
Conversation
Re-exporting a shallow doc at its own root filtered the cached root state and returned an error when the state was inconsistent. The error was not cached, so every export failed again and only fork() worked. Fall back to the verbatim root, as exporters before #1123 did: nothing referenced is dropped. See context/internal-encoding.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…eator resolver The resolver runs under the state lock from queries that cannot return an error. It now records the block and answers CreatorOp::Corrupt; import, export and checkout then return a DecodeError. See context/arena-parent-links.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
WASM Size Report
|
…rned The check sat in _checkout_without_emitting, so once a block was recorded, undo, checkout_to_latest and a detached fork panicked on the new error (they unwrap it). Check it in checkout, diff, revert_to, import and export instead; merge propagates the export error. Every reader of the change store now records a block it cannot parse, not only the creator resolver, and export checks again when it is done, so it cannot return updates that silently miss the block. See context/arena-parent-links.md, "A block that cannot be parsed". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two follow-ups from reviewing the 1.16.4 release PR (#1121). Both turn a hard failure on bad stored data into a recoverable one.
1. Export an unprunable shallow root verbatim (
1febed11)Re-exporting a shallow doc at its own root (#1123) filters the cached root state. When the state was inconsistent (a container's header parent does not match the container that references it), the export returned an error. The error was not cached, so every
export({ mode: "snapshot" })redid the walk and failed again, and onlyfork()still worked. Documents already hit by the #1161 family can hold such a root.Now the error is logged and the root is exported verbatim, as before #1123. Nothing referenced is dropped, and the fallback is memoized like a successful check.
2. Report an unparsable change block instead of panicking (
85e8dabb,fc2a196e)ChangeStore::creator_resolver(#1159) panicked when a lazily loaded block could not be parsed. It runs under the state lock fromis_deleted/has_container/get_path, which returnbool/Option, so there is noErrto return from them.ChangeStore::parse_failures), not only the resolver. The resolver answersCreatorOp::Corrupt, treated likeAbsentfor that lookup.checkout,diff,revert_to,importandexport(so alsofork_atandmerge) returnDecodeError("cannot parse change block ...")once a block is recorded.exportchecks again when it is done, so it cannot return bytes that silently miss the block._checkout_without_emittingor the exporters:undo,checkout_to_latestand a detachedforkcall those andunwrap. The first version of this PR had it there and moved the panic to those three;fc2a196efixes that (review feedback).Snapshot import already validates KV checksums, so only a forged or truncated-and-rechecksummed block reaches this path.
Known limits (documented in
context/arena-parent-links.md)undo,checkout_to_latestandforkbehave as before this PR on such a doc. They can still panic when they need the broken block itself:AppDag::ensure_lazy_load_nodepanics with "unparsed vv don't match with change store".importorcheckoutthat is the first to read the block finishes on the partial history; onlyexportre-checks at the end.get_deep_value) is what can be salvaged.Behavior change to review
LoroDoc::mergereturns the export error instead of unwrapping it.Validation
pnpm test: 1744 passed, 37 skipped; doctests pass.pnpm test-loom: 9 passed.pnpm check: clean.change_store.rs: the resolver answer, every reader recording, a doc with a truncated block (using a container that a healthy history does find), an export that is the first to read the block, anda_recorded_parse_failure_does_not_panic_where_no_error_can_be_returned. The last one fails with the panic from the review when the check is put back into the internal checkout.🤖 Generated with Claude Code