Investigate converting ShapeStream to explicit state machine

[KyleAMathews]

Summary

Recent bugs in ShapeStream have been caused by implicit state machine complexity. Both #3773 (infinite loop in replay mode) and the stale cache offset update bug share a common root cause: the client maintains many interdependent state variables that are updated across scattered code paths, making it easy to forget to update related state when taking a specific branch.

The Problem

ShapeStream currently maintains 20+ private state variables that form an implicit state machine:

Core sync state:

• #shapeHandle, #lastOffset, #liveCacheBuster, #schema

Connection state:

• #connected, #started, #state, #isUpToDate, #isMidStream, #lastSyncedAt

Stale cache handling:

• #staleCacheRetryCount, #staleCacheBuster, #lastSeenCursor

SSE handling:

• #sseFallbackToLongPolling, #consecutiveShortSseConnections, #lastSseConnectionStartTime

These variables are updated across many methods (#onInitialResponse, #onMessages, #requestShape, etc.), creating an implicit state machine where:

1. State transitions are scattered - no single place defines what variables change together
2. Valid state combinations are implicit - easy to create invalid states
3. Invariants are maintained manually - easy to update one variable but forget another
4. Testing requires mocking internals - can't test state machine logic in isolation

Bug Pattern

Both recent bugs followed the same pattern:

PR #3773 (Replay mode infinite loop):

• Code takes early return when cursor matches
• #lastSeenCursor wasn't cleared → replay mode never exits → infinite loop

Stale cache offset bug:

• Code logs warning and continues when stale response detected with existing handle
• #lastOffset was updated from stale response → handle/offset mismatch → server errors

Both bugs: early return/branch didn't handle all related state variables.

Proposed Solution

Investigate converting the implicit state to an explicit state machine:

Option A: Discriminated Union States

type SyncState = 
  | { phase: 'initial'; offset: '-1' }
  | { phase: 'syncing'; handle: string; offset: Offset; schema: Schema }
  | { phase: 'live'; handle: string; offset: Offset; schema: Schema; cursor: string }
  | { phase: 'stale-retry'; handle: string; offset: Offset; retryCount: number; cacheBuster: string }
  | { phase: 'paused'; handle: string; offset: Offset }

// Transitions are explicit functions that return new state
function handleResponse(state: SyncState, response: Response): SyncState {
  // All related state changes happen together
  // TypeScript ensures we handle all cases
}

Option B: XState or Similar

Use a formal state machine library for complex transitions:

• Visual state charts for documentation
• Built-in guards and actions
• Automatic testing of valid transitions

Option C: State Reducer Pattern

Centralize state updates through a reducer:

type StateAction = 
  | { type: 'RESPONSE_RECEIVED'; handle: string; offset: Offset; ... }
  | { type: 'STALE_RESPONSE_IGNORED' }  // No state changes!
  | { type: 'ENTER_REPLAY_MODE'; cursor: string }
  | { type: 'EXIT_REPLAY_MODE' }

function reduce(state: State, action: StateAction): State {
  // Single source of truth for state transitions
}

Benefits

1. Bugs become obvious - forgetting to handle state in a transition is a type error
2. Testable in isolation - state machine can be unit tested without network mocks
3. Self-documenting - state types and transitions document valid states
4. Easier reviews - PRs show explicit state transition changes

Scope

Start with the most bug-prone areas:

1. Stale cache detection and retry logic
2. Replay mode (cursor tracking)
3. Handle/offset consistency

Later extend to:

• SSE fallback logic
• Pause/resume
• Error recovery

Questions to Answer

1. Is the complexity worth it for a client library?
2. Which approach fits best with the existing codebase?
3. Should we use a library (XState) or roll our own?
4. Can we migrate incrementally or is it all-or-nothing?

Related

• Fix infinite loop in replay mode when CDN returns same cursor #3773 - Fix infinite loop in replay mode when CDN returns same cursor
• Current branch: claude/investigate-caching-bug-Rcwin - Fix stale cache offset update bug

[KyleAMathews]

Another bug investigation that we need to add invariants in the state machine

---

Investigation Summary: Offset Without Handle Bug
Background
This investigation stemmed from reports of client state corruption where the offset was being updated independently from the handle, leading to mismatched state.

Root Cause Analysis
The bug exists in packages/typescript-client/src/client.ts in the #onInitialResponse method, specifically in the third branch of the expired handle detection logic (lines 1099-1107).

The Three Branches of Expired Handle Detection
When the client receives a response with a handle that matches one in the expiredShapesCache:

First Branch (handle matches and is NOT expired): Normal case - accept the handle and update state.

Second Branch (handle IS expired AND client has NO handle yet): Client is starting fresh but CDN returned a stale cached response. Throws StaleCacheError to retry with a cache buster to bypass the CDN cache.

Third Branch (handle IS expired AND client already HAS a handle): The bug case. Client already has a valid handle (e.g., handle-B), but CDN returns a stale response with an expired handle (e.g., handle-A). The client correctly:

Logs a warning
Keeps its current handle (doesn't accept the expired one)
BUT incorrectly:

Continues execution past the warning
Updates this.#lastOffset from the stale response (line 1110-1113)
Updates this.#liveCacheBuster from the stale response
Updates this.#schema from the stale response
The Bug Scenario

1. Client has handle=B and offset=0_0
2. CDN returns stale cached response with expired handle=A and offset=0_inf
3. Handle correctly stays as B (expired A is ignored)
4. BUG: Offset is incorrectly updated to 0_inf from stale response
5. Client now has mismatched handle=B with offset=0_inf
6. Next request uses handle=B&offset=0_inf which is invalid state

Does This Affect Non-CDN Users?
No. This bug can only occur when:

A caching layer (CDN/proxy) serves a stale response from a previous session
The stale response contains an old handle that's been marked as expired
Without a CDN:

Requests go directly to the Electric server
The server always returns consistent handle+offset pairs
If a handle is invalid, the server returns 409 (not a 200 with mismatched data)
There's no mechanism to receive an "expired handle" in a 200 response
The Fix
The fix adds an early return in the third branch to skip updating offset/liveCacheBuster/schema when a stale response is detected:

} else {
// We already have a valid handle, so ignore the stale response entirely.
// Don't update offset or other metadata from this stale response as it
// would cause a mismatch between our current handle and the stale offset.
console.warn(...)
// Return early to avoid updating offset/liveCacheBuster/schema from stale response
return
}

Related Work
This is related to but distinct from the infinite loop fix in commit a428324 ("Fix infinite loop in replay mode when CDN returns same cursor"). That fix addressed a different CDN-related bug where the replay mode would loop forever when receiving the same cursor repeatedly.

[kevin-dp]

@KyleAMathews I agree that being more explicit about which state we're currently in and explicit state transitions would be less error-prone. Option B and C seem like quite a bit of work to rewrite, not sure if we really want to do that. The reality is, we will probably introduce new bugs. I'd rather remain close to what we have now as this has proven to work quite well and we're eliminating bugs along the way until there are no more.

Option A seems to be an improvement over what we currently have and a relatively contained change. However, my OOP background doesn't really like these discriminated union types and the handleResponse function that is likely to grow very big. I'd rather design this using the State Pattern, i.e. as an abstract class with subclasses, each of which implements its own handleResponse:

// Note: this code snippet is meant to illustrate the OOP pattern for state transitions
//           it needs to be adapted to the implementation in ts/db
abstract class SyncState {
  abstract handleResponse(response: Response): SyncState
}

class InitialSync extends SyncState {
  public readonly offset = '-1'

  handleResponse(response: Response): SyncState {
    // handle response and make state transition
    return new Syncing({
      handle: response.handle,
      offset: response.offset,
      schema: response.schema
    })
  }
}

type SyncingOpts = {
  handle: string
  offset: Offset
  schema: Schema
}

class Syncing extends SyncState {
  constructor(public readonly opts: SyncingOpts)
  
  handleResponse(response: Response): SyncState {
    // ...
  }
}
// ...

Notice that this approach allows for more controlled responses and state transitions. For example, during initial sync we may only support certain types of responses so we could use a type that is more precise than Response for the argument. And we may also know that only certain state transitions are possible so we could use a more precise return type than SyncState.

[kevin-dp]

I tasked Claude to compare the State Pattern approach from my previous comment with the other 3 approaches proposed in this issue. Here's its analysis:

Follow-up: OOP State Pattern Implementation Sketch

I've been exploring the OOP State Pattern approach (as an alternative to Options A/B/C) and I think it's a good fit for this codebase. Here's a summary of the analysis:

Why OOP State Pattern over the Other Options

Criterion	OOP State Pattern	Option A (Discriminated Unions)	Options B/C (XState/Reducer)
Closer to existing code	✅ Minimal paradigm shift	⚠️ Different style	❌ New pattern to learn
Per-state type precision	✅ Each class defines valid transitions	❌ Single handleResponse returns generic union	❌ Returns generic SyncState
Avoids giant switch/function	✅ Logic distributed across classes	❌ handleResponse grows large	❌ Single reducer can grow large
Incremental migration	✅ Convert one state at a time	⚠️ All-or-nothing for type safety	⚠️ Harder to mix approaches
No external dependencies	✅ Pure TypeScript	✅ Pure TypeScript	⚠️ XState adds dependency
Encapsulation	✅ State owns its logic	❌ Logic external to state data	❌ Logic external to state data

The main advantage over Option A specifically: with discriminated unions, you end up with a single handleResponse(state: SyncState, response: Response): SyncState function that needs a big switch statement to handle all states. With the OOP pattern, each state class implements its own handleResponse() with precise return types — the logic stays close to the data it operates on.

Key Benefit: Precise Transition Types

Each state class can declare exactly which states it can transition to:

class InitialState extends SyncState {
  // Compiler enforces: can ONLY return these types
  handleResponse(response: Response): SyncingState | LiveState | ErrorState { ... }
}

class ReplayingState extends SyncState {
  // Different valid transitions for this state
  handleResponse(response: Response): ReplayingState | LiveState { ... }
}

This means invalid transitions become compile errors, not runtime bugs.

How It Solves the Cited Bugs

PR #3773 (Replay infinite loop):

class ReplayingState extends SyncState {
  handleResponse(response: ShapeResponse): ReplayingState | LiveState {
    // If cursor unchanged, exit replay mode - ALL state cleanup happens here
    if (response.cursor === this.cursor) {
      return new LiveState({ ... });  // Clean transition, no forgotten state
    }
    // ...
  }
}

Stale cache offset bug:

class SyncingState extends SyncState {
  handleResponse(response: ShapeResponse): SyncingState | LiveState | StaleRetryState {
    if (response.handle !== this.handle) {
      // Return StaleRetryState with OUR offset, not the response's offset
      return new StaleRetryState({
        handle: this.handle,
        offset: this.offset,  // Preserve our known-good offset!
        // ...
      });
    }
    // ...
  }
}

Proposed State Classes

InitialState      → SyncingState | LiveState | ErrorState
SyncingState      → SyncingState | LiveState | StaleRetryState | ErrorState
LiveState         → LiveState | ReplayingState | StaleRetryState | PausedState | ErrorState
ReplayingState    → ReplayingState | LiveState | ErrorState
StaleRetryState   → SyncingState | StaleRetryState | ErrorState
PausedState       → (ignores responses, has resume() method)
ErrorState        → (has retry() and reset() methods)

Shared Logic

Common behavior (like handleMustRefetch() and handleError()) can live in the abstract base class with subclass overrides where needed:

abstract class SyncState {
  abstract handleResponse(response: ShapeResponse): SyncState;
  
  // Default implementations - subclasses can override
  handleError(error: Error): SyncState {
    return new ErrorState({ error, previousState: this });
  }
  
  handleMustRefetch(): SyncState {
    return new InitialState();
  }
}

Migration Path

1. Start with the most bug-prone states: StaleRetryState and ReplayingState
2. Keep existing code working alongside new state classes during transition
3. Gradually convert remaining states
4. Remove old scattered state variables once fully migrated

I've sketched out a full working implementation (~400 lines) with tests that verify both bug fixes. Happy to share if useful.

[github-actions]

The PR fixing this issue (#3816) has been released! 🚀

The following packages include the fix:

• @electric-sql/client@1.5.3

Thanks for reporting!