Fix indexeddb reopen connections#6675
Draft
zzorba wants to merge 5 commits into
Draft
Conversation
…er closes them The IndexedDB stores held a single long-lived `IDBDatabase` handle and treated `close()`/`reopen()` as no-ops, with no listener for a connection-level close. When the browser closes the connection out from under us (a transient storage backend error, eviction, or a `versionchange` from another context), every subsequent transaction fails for the lifetime of the process — the store is unable to read or write until the app restarts. For the event cache this is especially damaging: persistence runs on a decoupled task whose errors are only logged while sync keeps advancing, so dropped events are never re-requested and timelines fail to load until logout/login. Introduce a shared, reopenable connection and apply it to the event cache and media stores: - Add `connection::IndexeddbConnection`: an `Rc<RefCell<Rc<Database>>>` handle with a generic `with_transaction(stores, mode, open, body)` that builds the transaction, and on a DOM `InvalidStateError` (connection closing/closed) reopens via `open` and retries exactly once. The owning `Rc<Database>` is kept alive on the helper's stack frame for the duration of `body`, so the borrowed transaction is sound — no unsafe, no background task. - Route every event-cache and media transaction through the helper and implement their `reopen()` methods. - Add `From<indexed_db_futures::error::Error>` to the event-cache and media error types so they satisfy the helper's error bound.
Route every state-store transaction (sync token, room infos, room/stripped state, profiles, receipts, account data, send queue, dependent send queue, thread subscriptions, custom values) through `IndexeddbConnection::with_transaction`, and implement `reopen()`. The state store held a long-lived `inner: Database` handle with no-op close/reopen, so a browser-closed connection bricked the room list and the sync token until app restart. The data handle is now an `IndexeddbConnection` that reopens (via `upgrade_inner_db`, using the retained store cipher and meta db) and retries once on a DOM `InvalidStateError`. The `meta` database (migration bookkeeping/backups only) stays a plain handle.
…s it Route every crypto-store transaction (account, identity, sessions, inbound/ outbound group sessions, devices, identities, gossip/secret requests, backup keys, withheld sessions, room settings, key bundles, custom values, lease locks) through `IndexeddbConnection::with_transaction`, and implement `reopen()`. The crypto store held a long-lived `inner: Database` handle with no-op close/reopen, so a browser-closed connection bricked E2EE key access until app restart. The handle is now an `IndexeddbConnection` that reopens (via `open_and_upgrade_db` with the retained serializer) and retries once on a DOM `InvalidStateError`. The Drop impl closes the current handle via the connection. With this, all four IndexedDB stores (event cache, media, state, crypto) share one reopen-and-retry mechanism and recover from a transient connection loss instead of bricking until restart.
A connection that dies as a transaction is being built can surface as `TransactionInactiveError` or `AbortError`, not only `InvalidStateError`. Treat all three as "reopen and retry" so more dead-connection cases recover on the next operation instead of erroring once.
9488b54 to
50ac0e8
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6675 +/- ##
==========================================
- Coverage 89.89% 89.88% -0.01%
==========================================
Files 396 396
Lines 110236 110236
Branches 110236 110236
==========================================
- Hits 99098 99089 -9
- Misses 7376 7383 +7
- Partials 3762 3764 +2 ☔ View full report in Codecov by Harness. |
Adds wasm-bindgen browser tests exercising the connection-reopen path on a real headless browser: - raw_transaction_fails_after_browser_closes_connection: closes the underlying IDBDatabase (what the browser does) and asserts the next raw transaction build fails with the DOM error is_connection_closed keys on — proving the failure mode is real. - store_operation_recovers_after_browser_closes_connection: issues a store operation after that close and asserts it succeeds (reopen + retry), then survives a second close. Guards the shared IndexeddbConnection reopen mechanism against regressions.
Member
|
Thank you for your contributions. +2'312-1'777 for 5 commits. Is it possible to split the commits into smaller ones, please, to facilitate the review process? |
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
Every IndexedDB store (event_cache, media, state, crypto) holds a single long-lived IDBDatabase handle, treats close()/reopen() as no-ops, and has no connection-close listener. When the browser closes the connection out from under us — a transient storage-backend error, eviction, or a versionchange from another browsing context — every subsequent transaction fails for the lifetime of the process, so the store can neither read nor write until the page/app is restarted.
On a long-lived desktop (Electron) client this surfaced as timelines universally failing to load events and synced messages being permanently dropped (recoverable only by logout/login). For the event cache it's especially damaging: persistence runs on a decoupled task whose errors are only logged while the sync position keeps advancing, so events dropped during the dead window are never re-requested.
This makes the connection handle recoverable across all four stores.
Shared mechanism (src/connection.rs)
IndexeddbConnection wraps the handle as Rc<RefCell<Rc>> and exposes a generic with_transaction(stores, mode, open, body) that builds the transaction and, on a DOM InvalidStateError (connection closing/closed), reopens via open and retries exactly once. The owning Rc is kept alive on the helper's stack frame for the duration of body, so the borrowed transaction is sound — no unsafe, no background task (async closures / AsyncFnOnce).
Per store
Also adds From<indexed_db_futures::error::Error> to the event-cache and media error types to satisfy the shared helper's error bound.
This is the connection-lifecycle layer. Natural follow-ups (not included): typed transient-vs-logical error classification, and a recovery path that re-fetches a batch dropped during the dead window rather than relying on the next sync.
Test plan
cargo check -p matrix-sdk-indexeddb --lib --target wasm32-unknown-unknown passes for all four stores (they are #[cfg(target_family = "wasm")]).
Mechanical audit: zero remaining direct self.inner.transaction(...) sites; every operation routes through the reopen-and-retry helper.
Signed-off-by: Daniel Salinas