Skip to content

Log WebSocket connection-attempt timeout as a warning, not an exception#213

Merged
sierpinskid merged 1 commit into
GetStream:developfrom
harlan:fix/reduce_3sec_error_to_warning
Jun 29, 2026
Merged

Log WebSocket connection-attempt timeout as a warning, not an exception#213
sierpinskid merged 1 commit into
GetStream:developfrom
harlan:fix/reduce_3sec_error_to_warning

Conversation

@harlan

@harlan harlan commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Log a WebSocket connection-attempt timeout as a warning instead of surfacing it as an exception.

Background

WebsocketClient.ConnectAsync guards the native ClientWebSocket.ConnectAsync with a timeout (it can hang indefinitely on some Unity/Android versions, and token cancellation doesn't reliably interrupt it). When the guard trips, it throws a TimeoutException:

throw new TimeoutException($"Connection attempt timed out after {timeout} seconds.");

That exception is caught and passed to HandleConnectionFailedAsync, where it doesn't match the "expected" set (OperationCanceledException / WebSocketException / SocketException) and therefore falls through to the catch-all branch:

else
{
    _logs.Exception(exception); // -> Debug.LogException, i.e. error severity
}

Problem

A connection-attempt timeout is an expected, transient failure that the reconnect flow recovers from — but logging it via _logs.Exception (Debug.LogException) reports it at error severity. In production this floods crash/error-reporting tools (Sentry, Bugsnag, etc.) with handled, non-actionable noise, especially on flaky networks where the initial connect occasionally exceeds the timeout before reconnecting successfully.

Change

Treat TimeoutException like the other expected connection-failure types and log it as a warning instead:

else if (exception is TimeoutException)
{
    // A connection-attempt timeout (see the timeout guard in ConnectAsync)
    // is an expected, transient failure that the reconnect flow recovers
    // from. Log it as a warning rather than surfacing it as an exception,
    // so it does not flood crash reporting with handled, non-actionable noise.
    _logs.Warning(exception.ToString());
}

ConnectionFailed is still raised exactly as before, so reconnect behavior is unchanged — only the log severity of the timeout case is reduced. Genuine, unexpected connection exceptions still go through _logs.Exception.

Notes

  • Scoped to TimeoutException only; no other failure path is affected.
  • No tests required updating — the connection tests mock IWebsocketClient, and no test asserts on the log severity of HandleConnectionFailedAsync.

WebsocketClient.ConnectAsync guards the native ClientWebSocket.ConnectAsync with a timeout (it can hang on some Unity/Android versions). On timeout it throws TimeoutException, which HandleConnectionFailedAsync routes to the catch-all else branch and logs via _logs.Exception(...) (Debug.LogException) — i.e. at error severity.

A connection-attempt timeout is an expected, transient failure that the reconnect flow recovers from, so reporting it as an exception floods crash/error reporting tools (Sentry, Bugsnag, etc.) with handled, non-actionable noise. Treat TimeoutException like the other expected connection-failure types and log it as a warning instead.
@harlan

harlan commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

@sierpinskid the PR we discussed for turning 3 second timeout into a warning instead of error

@sierpinskid sierpinskid self-requested a review June 29, 2026 10:28
@sierpinskid

Copy link
Copy Markdown
Member

All tests pass:
image

@sierpinskid sierpinskid merged commit 9dccae5 into GetStream:develop Jun 29, 2026
0 of 17 checks passed
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