Skip to content

Start the signaling send timeout at the first write attempt - #45

Merged
zortos293 merged 1 commit into
mainfrom
capy/signaling-send-deadline
Oct 15, 2026
Merged

zortos293 merged 1 commit into
mainfrom
capy/signaling-send-deadline

Conversation

@zortos293

Copy link
Copy Markdown
Contributor

Fixes "The GeForce NOW rig could not be reached" on networks where gathering ICE candidates is slow.

Cause. The diagnostic build from #44 logged SIGNAL disconnected reason=Send timeout 2 ms after the SDP answer was queued. Each outgoing WebSocket frame got a 1 s deadline when it was queued. Creating the answer gathers STUN candidates synchronously on the signaling thread, and on this user's network that took 1.8–3.4 s (on a nearby network it takes about 0.2 s). A frame queued before that pause was already older than 1 s at the next drain, so the client closed its own signaling socket before the answer went out. The server then never received the answer, never trickled its media candidate, and all ICE pairs failed.

Fix. WriteQueue now starts a frame's deadline the first time it tries to write that frame. Time spent waiting in the queue while the thread is busy no longer counts. A socket that keeps refusing data still times out after 1 s from the first attempt. enqueue no longer takes a timestamp.

Tests. The queue test now checks two cases. Frames that wait 3 s in the queue and then each write within their own window complete. A frame whose writes keep returning Again for 1 s after the first attempt still times out. The two old assertions that expected a frame never written to time out were testing the bug itself, so this change replaces them. The 95 streaming checks pass, both normally and under ASan/UBSan, and the Switch NRO builds.

Open in Capy

@coderabbitai

coderabbitai Bot commented Oct 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 43 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a7338f8e-ada3-472b-a528-09f3c072703b
📥 Commits

Reviewing files that changed from the base of the PR and between 836e63c and 8b44d33.

📒 Files selected for processing: 3
  • app/src/WebSocketClient.cpp
  • app/src/websocket_write_queue.hpp
  • tests/websocket_write_queue_test.cpp
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zortos293
zortos293 merged commit e148ccd into main Oct 15, 2026
8 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.

1 participant