8000 fix(browser): an unreadable dirty page is an error, not dropped from the IndexedDB flush by adityamukho · Pull Request #470 · project-minigraf/minigraf · GitHub
[go: up one dir, main page]

Skip to content 1 8000

fix(browser): an unreadable dirty page is an error, not dropped from the IndexedDB flush - #470

Merged
adityamukho merged 1 commit into
mainfrom
fix/browserdb-dropped-dirty-pages
Oct 7, 2026
Merged

adityamukho merged 1 commit into
mainfrom
fix/browserdb-dropped-dirty-pages

Conversation

@adityamukho
Copy link
Copy Markdown
Collaborator

v2.x counterpart of the BrowserDb fix in #469 (v3).

Problem

checkpoint(), importGraph() and every write in BrowserDb collected the pages to flush to IndexedDB with:

.filter_map(|id| inner.pfs.with_backend(|b| b.read_page_raw(id).ok().map(|d| (id, d))))

A dirty page that failed to read was silently left out of the flush, and the call still reported success.

Reachability

Not reachable today. An id enters the dirty set only inside BrowserBufferBackend::write_page, right after the page is stored. load_pages_all_dirty marks only pages it holds, and the flush reads under the same single-threaded borrow. So this is a defensive fix: a future bug that left a dirty id without a page would surface as an error instead of as silent data loss.

Fix

A shared take_dirty_pages helper, the same as on v3 since #469, so the next main → v3 merge is trivial. It returns the read error, coded via MinigrafError. This PR doesn't port v3's other browser hardening (unwraps, casts); those aren't data-integrity issues.

Tests

🤖 Generated with Claude Code

https://claude.ai/code/session_015GgLEs8YozJjYhwPEYgTUn

…the IndexedDB flush

checkpoint(), importGraph() and every write collected dirty pages with
read_page_raw(id).ok() in a filter_map, so a page that failed to read
was silently left out of the flush to IndexedDB while the call reported
success. A shared take_dirty_pages helper (the same one v3 has since
#469) now returns the read error.

No current path leaves a dirty id without a page (ids enter the dirty
set only after the page is stored), so this is defensive rather than a
fix for observed loss.

The browser unit tests did not compile on wasm32 (a duplicate
page_cache_capacity from #341/#342, and dir_sync tests using tempfile);
both are fixed so the two new regression tests run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015GgLEs8YozJjYhwPEYgTUn
@codecov
codecov Bot commented Oct 7, 2026
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@adityamukho
adityamukho merged commit 596bb59 into main Oct 7, 2026
21 checks passed
@adityamukho
adityamukho deleted the fix/browserdb-dropped-dirty-pages branch October 7, 2026 07:32
adityamukho added a commit that referenced this pull request Oct 7, 2026
…-10-07

Merge main into v3 (#470 BrowserDb regression tests)
adityamukho added a commit that referenced this pull request Oct 8, 2026
Doc sync (CHANGELOG, CLAUDE.md, ROADMAP, README, TEST_COVERAGE) and
version bump to 2.0.4. v2.x patch: BrowserDb IndexedDB flush returns an
error for an unreadable dirty page (#470), plus the golden-file v7
corpus (#391) and SIGKILL committed-data crash test (#384). File format
v7 unchanged; no API changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012wXxZguv7PgKQjMMH4yJ5h
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

0