Skip to content

Fix TypeScript React provider reconnect#5185

Open
gugahoa wants to merge 4 commits into
masterfrom
gugahoa/ts-sdk-react-reconnect
Open

Fix TypeScript React provider reconnect#5185
gugahoa wants to merge 4 commits into
masterfrom
gugahoa/ts-sdk-react-reconnect

Conversation

@gugahoa

@gugahoa gugahoa commented Jun 2, 2026

Copy link
Copy Markdown

Description of Changes

Fix TypeScript SDK React provider recovery when the underlying WebSocket closes or reports an error while a SpacetimeDBProvider is still mounted.

  • Mark DbConnection inactive before emitting disconnect / connectError, so lifecycle callbacks observe the closed state.
  • Teach the React ConnectionManager to clear closed retained provider connections and rebuild them after a short delay.
  • Cancel pending reconnects during provider release so React StrictMode cleanup does not create replacement connections.
  • Detach manager callbacks from the old connection before reconnecting, preventing stale closed connections from updating provider state.
  • Use the latest same-key retained builder when a replacement connection is needed.
  • Add focused lifecycle/reconnect regression tests and document that this recovery is provider-level only; direct DbConnection users still own reconnection.

API and ABI breaking changes

None. This does not remove or change public APIs. It changes React provider lifecycle behavior so retained provider connections recover after close/error.

Expected complexity level and risk

3/5.

The change is localized to the TypeScript SDK, but it touches async connection lifecycle behavior, timer cancellation, React provider reference counting, and callback-visible connection state. Main risks are duplicate reconnects, stale connection callbacks updating state, using an outdated retained builder for replacement connections, or StrictMode release/remount regressions. The implementation guards against stale callbacks, detaches manager callbacks from closed connections, and cancels reconnect timers during release.

Testing

  • pnpm --dir crates/bindings-typescript exec vitest run tests/db_connection.test.ts tests/connection_manager.test.ts tests/connection_manager_reconnect.test.ts
  • pnpm --dir crates/bindings-typescript lint
  • pnpm --dir crates/bindings-typescript build
  • pnpm --dir crates/bindings-typescript test

@gugahoa gugahoa self-assigned this Jun 2, 2026
@gugahoa gugahoa force-pushed the gugahoa/ts-sdk-react-reconnect branch from 792f83b to 0245d51 Compare June 3, 2026 14:18
@gugahoa gugahoa requested a review from jsdt June 3, 2026 14:22
@gugahoa gugahoa force-pushed the gugahoa/ts-sdk-react-reconnect branch from 0245d51 to bbe490e Compare June 3, 2026 14:23
@gugahoa gugahoa marked this pull request as ready for review June 3, 2026 14:25

@jsdt jsdt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! I mentioned a few potential issues, and it would be great to have test coverage for those.

Comment thread crates/bindings-typescript/src/sdk/connection_manager.ts Outdated
Comment thread crates/bindings-typescript/src/sdk/connection_manager.ts
Comment thread crates/bindings-typescript/src/sdk/connection_manager.ts
gugahoa added a commit that referenced this pull request Jun 10, 2026
Replace the fixed 1s reconnect delay in the React ConnectionManager with
exponential backoff (1s base, doubling per consecutive failed attempt,
capped at 30s). The attempt counter resets once a connection successfully
connects.

Addresses review feedback on #5185.
gugahoa added a commit that referenced this pull request Jun 10, 2026
DbConnectionImpl now records when disconnect() has been requested, and the
React ConnectionManager skips scheduling a reconnect for connections that
were intentionally disconnected. A subsequent retain() (e.g. a remount)
still builds a fresh connection.

Addresses review feedback on #5185.
gugahoa added a commit that referenced this pull request Jun 10, 2026
When the React ConnectionManager replaces the managed connection, the new
connection starts with an empty client cache, but useTable kept reporting
isReady=true from the previous subscription. Reset subscribeApplied
whenever the connection is inactive or missing, so isReady stays false
until the replacement connection's subscription is applied.

Addresses review feedback on #5185.
@gugahoa gugahoa requested a review from jsdt June 10, 2026 12:07
gugahoa added 4 commits June 10, 2026 12:56
Mark DbConnection inactive before disconnect/connectError callbacks so consumers see a closed connection.

Teach ConnectionManager to clear closed retained provider connections and rebuild them after a short delay while a provider remains mounted. Cancel pending reconnects during release to preserve StrictMode cleanup behavior.

Add lifecycle/reconnect regression tests and document that this recovery is provider-level only.
Replace the fixed 1s reconnect delay in the React ConnectionManager with
exponential backoff (1s base, doubling per consecutive failed attempt,
capped at 30s). The attempt counter resets once a connection successfully
connects.

Addresses review feedback on #5185.
DbConnectionImpl now records when disconnect() has been requested, and the
React ConnectionManager skips scheduling a reconnect for connections that
were intentionally disconnected. A subsequent retain() (e.g. a remount)
still builds a fresh connection.

Addresses review feedback on #5185.
When the React ConnectionManager replaces the managed connection, the new
connection starts with an empty client cache, but useTable kept reporting
isReady=true from the previous subscription. Reset subscribeApplied
whenever the connection is inactive or missing, so isReady stays false
until the replacement connection's subscription is applied.

Addresses review feedback on #5185.
@gugahoa gugahoa force-pushed the gugahoa/ts-sdk-react-reconnect branch from 78f60d0 to c5526ed Compare June 10, 2026 15:56
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