Skip to content

Fix indexeddb reopen connections#6675

Draft
zzorba wants to merge 5 commits into
matrix-org:mainfrom
filament-dm:zzorba/fix-indexeddb-reopen-connections
Draft

Fix indexeddb reopen connections#6675
zzorba wants to merge 5 commits into
matrix-org:mainfrom
filament-dm:zzorba/fix-indexeddb-reopen-connections

Conversation

@zzorba

@zzorba zzorba commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

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

  • event cache — all transaction sites + reopen().
  • media — all transaction sites + reopen().
  • state — all transaction sites (sync token, room infos, room/stripped state, profiles, receipts, account data, send queue, dependent send queue, thread subscriptions, custom values) + reopen(), reopening via upgrade_inner_db using the retained store cipher and meta db. The meta db (migration bookkeeping/backups only) stays a plain handle.
  • crypto — all transaction sites (account, identity, sessions, inbound/outbound group sessions, devices, gossip/secret requests, backup keys, withheld sessions, room settings, key bundles, custom values, lease locks) + reopen(), reopening via open_and_upgrade_db with the retained serializer.

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.

  • I've documented the public API changes in the appropriate changelog files (see Writing changelog entries).
  • This PR was made with the help of AI.

Signed-off-by: Daniel Salinas

zzorba added 3 commits June 18, 2026 19:52
…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.
@zzorba zzorba changed the title Zzorba/fix indexeddb reopen connections Fix indexeddb reopen connections Jun 18, 2026
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.
@zzorba zzorba force-pushed the zzorba/fix-indexeddb-reopen-connections branch from 9488b54 to 50ac0e8 Compare June 18, 2026 21:19
@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.88%. Comparing base (d2f7ce4) to head (51bb961).
⚠️ Report is 23 commits behind head on main.
✅ All tests successful. No failed tests found.

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.
📢 Have feedback on the report? Share it here.

@codspeed-hq

codspeed-hq Bot commented Jun 18, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 50 untouched benchmarks


Comparing filament-dm:zzorba/fix-indexeddb-reopen-connections (51bb961) with main (e8673e4)

Open in CodSpeed

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.
@Hywan

Hywan commented Jun 25, 2026

Copy link
Copy Markdown
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?

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.

2 participants