Skip to content

session.js: _coldReconnect() doesn't clear paused/_sessionReleased, so resume() times out after a silent recovery #58

Description

@zoharbabin

What happened (live-reproduced)

While preparing the pause/resume recipe for issue #40 (see PR #57), a long pause (~150s, no resume() called) was left running against a live session. As expected, the server fired pauseSessionExpiredsessionReadyForResume, setting _sessionReleased = true.

Before resume() was ever called, the SDK's own transport-recovery path fired independently: a stale STV session returned a WHEP 404, which triggered _coldReconnect() (session.js line ~1858, stvSessionGone branch). _coldReconnect() rebuilt the socket/transports and successfully resumed the conversation on its own — real replies kept flowing, state went back to 'connected'.

_coldReconnect() (session.js:2057-2145) never touches this.paused or this._sessionReleased. Both stayed at their pre-reconnect values (true/true). Calling resume() afterward (session.js:1063-1076) saw _sessionReleased === true and took the "rebuild transports from a fresh stvNewSession" branch — but no fresh stvNewSession was coming, because _coldReconnect() had already rebuilt everything. That call rejected after a 30s timeout (ASRConnectionFailed: timed out waiting for the server), even though the conversation was already working correctly and kept working regardless of the rejection.

Impact

Any app that calls pause() then unconditionally calls resume() later (the normal, documented pattern — see docs/PAUSE-RESUME-RECIPE.md added in PR #57) can get a spurious rejected promise/thrown error from resume() on a long-enough pause, even though the session is actually fine. Apps that treat a resume() rejection as fatal (reasonable, since the doc for resume() implies failure = broken session) will misdiagnose a healthy session as broken.

Suggested fix

_coldReconnect() should clear this.paused = false and this._sessionReleased = false on its success path (~line 2109, alongside this._setState('connected')) when it fires while paused — it has already done the equivalent of what resume()'s expensive branch does (fresh transports, fresh session), so there's nothing left for a subsequent resume() call to do. Alternatively/additionally, resume() could tolerate hitting an already-'connected' state gracefully instead of assuming its own rebuild is still needed.

Where this was found

PR #57 (docs/PAUSE-RESUME-RECIPE.md) documents this as a caveat with an app-side mitigation (bounded pause-timeout, treat resume() rejection as non-fatal if session.state === 'connected') since the SDK itself isn't fixed yet. This issue tracks the actual source-level fix. Not blocking PR #57 — the doc's mitigation is the correct interim guidance either way, since even after this bug is fixed, apps still shouldn't assume resume() can never reject for other reasons.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions