Skip to content

feat(asr): enhance Soniox ASR integration with reconnect metrics and … - #2346

Open
diyuyi-agora wants to merge 2 commits into
mainfrom
bugfix/yuyidi/soniox_asr_0929
Open

diyuyi-agora wants to merge 2 commits into
mainfrom
bugfix/yuyidi/soniox_asr_0929

Conversation

@diyuyi-agora

Copy link
Copy Markdown
Contributor

…error handling

@diyuyi-agora

Copy link
Copy Markdown
Contributor Author
image

@diyuyi-agora
diyuyi-agora force-pushed the bugfix/yuyidi/soniox_asr_0929 branch from adc0571 to 4c39e8d Compare September 29, 2026 11:37
@github-actions

Copy link
Copy Markdown

Finding (current head 4c39e8d5)

  • [P1, merge blocker] Do not suppress service-going-away close code 1001 (websocket.py:134). websockets classifies both 1000 and 1001 (going away) as ConnectionClosedOK (OK_CLOSE_CODES includes 1001). Therefore a Soniox server restart or service departure during active audio now skips EXCEPTION and send_asr_error; _handle_close only emits connection_status_changed and retries. Before this change, the exception emitted a non-fatal ASR error. Keep 1000 silent but report 1001 as a transient non-fatal failure, and test an actual connect()/receive close with 1001. The ASR design guide requires transient service failures to be reported as NON_FATAL_ERROR (section 9.2); this change violates that MUST rule without a documented contract migration.

ASR design review

  • Lifecycle: N/A (no lifecycle override changed).
  • Connection state: pass for status transitions; fail for classifying 1001 as an error-free close.
  • Buffering: N/A (no buffering change).
  • Finalize: N/A (no finalize change).
  • Reconnect: pass (1001 still enters the existing backoff path).
  • Result shape: N/A (no ASR result change).
  • Metrics: N/A (no metric change).
  • Tests: fail (new close tests check 1000 and 1006, but not the 1001 case; the WebSocket unit test mirrors the branch rather than invoking connect()).

Static review only: the checked-out worktree is the base branch, with neither ten_runtime nor ten_ai_base installed; no PR-head scripts or dependencies were run.

@TEN-framework TEN-framework deleted a comment from github-actions Bot Sep 29, 2026
@TEN-framework TEN-framework deleted a comment from github-actions Bot Sep 29, 2026
@TEN-framework TEN-framework deleted a comment from github-actions Bot Sep 29, 2026
@github-actions

Copy link
Copy Markdown

Findings (current head f0a19faf)

  • [P1, merge blocker] Do not suppress an incomplete 1000 close handshake (websocket.py:153). is_normal_close() checks only ConnectionClosed.code, not whether the exception is ConnectionClosedOK. In websockets, ConnectionClosedError can have received code 1000 when the close handshake fails (for example, the server sends 1000 and the connection drops before the reply); its .code is still 1000. The new connect() branch then skips EXCEPTION, and _handle_close emits only a disconnect before retrying, so a transient network failure is no longer reported as NON_FATAL_ERROR. This violates the ASR design guide section 9.2 MUST rule without a documented contract migration. Restrict the silent branch to a completed ConnectionClosedOK with code 1000, and test ConnectionClosedError(Close(1000, "OK"), None) through connect() and the emitted error path. The earlier 1001 finding is addressed by this head.

  • [P1, CI blocker] Shorten the second commit header (commit f0a19faf). The current commitlint check fails header-max-length: this commit header is 103 characters, above the repository limit of 100. A later commit does not fix a per-commit check; the offending header needs rewording.

ASR design review

  • Lifecycle: N/A (no lifecycle override changed).
  • Connection state: fail for incomplete code-1000 close handshakes; complete 1000 and 1001 classification otherwise covered.
  • Buffering: N/A (no buffering change).
  • Finalize: N/A (no finalize change).
  • Reconnect: pass for the changed 1001 path; existing backoff still applies.
  • Result shape: N/A (no ASR result change).
  • Metrics: N/A (no metrics change).
  • Tests: fail (no incomplete-handshake/error-reporting case; the 1001 connect() path is covered).

Static review only: the checkout is the base branch, and the ASR runtime dependencies are unavailable locally; no PR-head code or dependency installation was run.

This branch has not been deployed

No deployments
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