fix(telegram): socket-level timeouts for getUpdates to prevent TCP half-open deadlock#108874
fix(telegram): socket-level timeouts for getUpdates to prevent TCP half-open deadlock#108874friendfish wants to merge 1 commit into
Conversation
72fda3e to
c9cc364
Compare
|
Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 1:00 PM ET / 17:00 UTC. Summary PR surface: Source +76. Total +76 across 1 file. Reproducibility: no. high-confidence controlled current-main reproduction was established, but the source path and multiple production reports consistently describe getUpdates stalls after network disruption. The PR still needs an after-fix run from the reported proxy/VPN environment. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Use a getUpdates socket timeout longer than Telegram’s normal 30-second long poll but shorter than the 45-second JS abort, preserve bounded dispatcher reuse and the existing fallback state machine, add focused timeout-ordering coverage, and demonstrate redacted live recovery after a proxy/VPN/TUN flap. Do we have a high-confidence way to reproduce the issue? No high-confidence controlled current-main reproduction was established, but the source path and multiple production reports consistently describe getUpdates stalls after network disruption. The PR still needs an after-fix run from the reported proxy/VPN environment. Is this the best way to solve the issue? No. Dispatcher-level timeouts within the existing fallback loop are the right boundary, but values that fire after the 45-second abort cannot solve the claimed socket-lifecycle failure. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: unclear because the file could not be read completely. Codex review notes: model internal, reasoning high; reviewed against fe4a0aae627b. Label changesLabel justifications:
Evidence reviewedPR surface: Source +76. Total +76 across 1 file. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (4 earlier review cycles)
|
c9cc364 to
0a80a1c
Compare
|
@clawsweeper re-review Rev 2 pushed: removed the bypass dispatcher route. getUpdates timeouts are now injected via attempt.createDispatcher(timeouts) within the existing transport-attempt loop, preserving full fallback, health tracking, and sticky promotion. Added UND_ERR_BODY_TIMEOUT and UND_ERR_HEADERS_TIMEOUT to fallback retry error codes so socket-level timeouts trigger transport failover. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
…ort-attempt loop When the network path changes (proxy restart, TUN switch, VPN flap), undici keep-alive sockets can become TCP Half-Open. The existing JS-level AbortController.abort() only rejects the Promise — it does not close the OS socket. This causes the polling loop to reuse the same dead socket indefinitely, creating a silent deadlock. Fix: inject undici bodyTimeout (90s) and headersTimeout (60s) into the existing transport-attempt dispatcher chain, preserving full fallback, health tracking, and sticky promotion behavior. Timeout errors (UND_ERR_BODY_TIMEOUT, UND_ERR_HEADERS_TIMEOUT) are added to the fallback retry error codes so they trigger transport failover. Closes openclaw#86031 Related openclaw#108562 openclaw#56061 openclaw#57743 openclaw#78079
0a80a1c to
28a3bcc
Compare
What Problem This Solves
When the network path changes (proxy restart, TUN switch, VPN flap), undici keep-alive sockets used by the Telegram
getUpdateslong-polling loop can enter a TCP Half-Open state — the OS believes the connection is alive, but no data will ever arrive.The existing JS-level
AbortController.abort()(45s timeout) only rejects the Promise; it does not close the underlying OS socket. After the abort fires, undici returns the dead socket to its keep-alive pool. The next polling cycle reuses the same half-open socket and hangs again. This creates a silent deadlock that:getUpdatesactivity at the JS level, not the socket level)This has been observed in production on WSL2/Windows environments with proxy or VPN configurations. Related reports: #86031, #108562, #56061, #57743.
Root Cause
Solution
Inject undici
bodyTimeout(90s) andheadersTimeout(60s) into the existing transport-attempt dispatcher chain, preserving full fallback, health tracking, and sticky promotion behavior.Design (Rev 2 — addresses clawsweeper P1 feedback)
The timeout injection happens inside the transport-attempt loop, not as a bypass:
createTelegramDispatcher(policy, timeouts?)— extended signature spreads optionaltimeoutsinto pool options for all dispatcher modes (direct / env-proxy / explicit-proxy)TelegramTransportAttempt.createDispatcher(timeouts?)— each attempt factory accepts optional timeouts and creates a timeout-protected dispatcher viacreateTelegramDispatcher(policy, timeouts). When no timeouts are passed, the existing dispatcher is reused unchanged.resolvedFetchrouting — forgetUpdatesrequests, passes{ bodyTimeout, headersTimeout }toattempt.createDispatcher(). All other requests callattempt.createDispatcher()with no arguments. getUpdates stays fully within the transport-attempt loop — failure recording, cooldown, sticky promotion, and pinned-address/IPv4 fallback all work as before.Fallback error codes —
UND_ERR_BODY_TIMEOUTandUND_ERR_HEADERS_TIMEOUTadded toFALLBACK_RETRY_ERROR_CODES, so socket-level timeouts trigger transport failover to the next attempt.Recovery path when a timeout fires
Timeout hierarchy
Proxy Compatibility
Timeouts are applied through
createTelegramDispatcher(policy, timeouts), which routes through the correct dispatcher factory based on the resolved policy mode:new Agent(...)createHttp1EnvHttpProxyAgent(...)createHttp1ProxyAgent(...)Evidence
Production incident (2026-07-16 09:41–10:00 GMT+8, before fix)
All Telegram bots on a WSL2/Windows gateway with proxy configuration entered silent deadlock simultaneously after a network path change:
The 20-minute silent period matches TCP Half-Open behavior: OS never received FIN/RST, socket appeared healthy while no data could flow. AbortController fired but could not close the socket. Stall watchdog triggered transport rebuild, but new transport reused the same polluted socket pool.
Code analysis
request-timeouts.ts: JS-level timeout is 45s viaTELEGRAM_GET_UPDATES_REQUEST_TIMEOUT_MSpolling-liveness.ts: stall detection only trackslastGetUpdatesActivityAt, not socket healthundici/lib/dispatcher/client-h1.js:808-810:bodyTimeouttriggersutil.destroy(socket)— only path that physically closes TCP socketsfetch.tsfallback loop:shouldUseTelegramTransportFallback()checks error codes; the two new codes route into the existing recovery chainRev 2 changes (addressing clawsweeper review)
attempt.createDispatcher(timeouts)within the existing loopCloses #86031
Related: #108562 #56061 #57743 #78079