Skip to content

fix(telegram): socket-level timeouts for getUpdates to prevent TCP half-open deadlock#108874

Open
friendfish wants to merge 1 commit into
openclaw:mainfrom
friendfish:fix/fix_010_telegram_polling_deadlock
Open

fix(telegram): socket-level timeouts for getUpdates to prevent TCP half-open deadlock#108874
friendfish wants to merge 1 commit into
openclaw:mainfrom
friendfish:fix/fix_010_telegram_polling_deadlock

Conversation

@friendfish

@friendfish friendfish commented Jul 16, 2026

Copy link
Copy Markdown

What Problem This Solves

When the network path changes (proxy restart, TUN switch, VPN flap), undici keep-alive sockets used by the Telegram getUpdates long-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:

  • Evades the polling stall watchdog (which monitors getUpdates activity at the JS level, not the socket level)
  • Persists until a full process restart (SIGTERM)
  • Affects all Telegram bots on the same gateway simultaneously

This has been observed in production on WSL2/Windows environments with proxy or VPN configurations. Related reports: #86031, #108562, #56061, #57743.

Root Cause

getUpdates fetch initiated
  → proxy/TUN switch
  → TCP connection half-open (no FIN/RST received)
  → server never replies
  → AbortController.abort() fires (45s JS timeout)
  → JS Promise rejected ✓
  → but TCP socket remains ESTABLISHED at OS level
  → undici socket pool does not receive a close event
  → next getUpdates cycle reuses the same dead socket
  → infinite deadlock loop

Solution

Inject undici bodyTimeout (90s) and headersTimeout (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:

  1. createTelegramDispatcher(policy, timeouts?) — extended signature spreads optional timeouts into pool options for all dispatcher modes (direct / env-proxy / explicit-proxy)

  2. TelegramTransportAttempt.createDispatcher(timeouts?) — each attempt factory accepts optional timeouts and creates a timeout-protected dispatcher via createTelegramDispatcher(policy, timeouts). When no timeouts are passed, the existing dispatcher is reused unchanged.

  3. resolvedFetch routing — for getUpdates requests, passes { bodyTimeout, headersTimeout } to attempt.createDispatcher(). All other requests call attempt.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.

  4. Fallback error codesUND_ERR_BODY_TIMEOUT and UND_ERR_HEADERS_TIMEOUT added to FALLBACK_RETRY_ERROR_CODES, so socket-level timeouts trigger transport failover to the next attempt.

Recovery path when a timeout fires

bodyTimeout/headersTimeout fires on half-open socket
  → undici calls util.destroy(socket) — TCP connection physically closed
  → undici throws error with code UND_ERR_BODY_TIMEOUT or UND_ERR_HEADERS_TIMEOUT
  → resolvedFetch catch block: shouldUseTelegramTransportFallback() → true (error code in whitelist)
  → recordAttemptFailure() increments failure count
  → after threshold: promoteStickyAttempt() → next attempt (IPv4-only or pinned-IP fallback)
  → new attempt creates fresh dispatcher with timeouts
  → getUpdates resumes on healthy connection

Timeout hierarchy

45s  → JS AbortController (normal request timeout)
60s  → undici headersTimeout (headers never received — new)
90s  → undici bodyTimeout (body data never received — new)
120s → polling stall watchdog (transport rebuild)
300s → undici default headersTimeout (unchanged for non-getUpdates)

Proxy Compatibility

Timeouts are applied through createTelegramDispatcher(policy, timeouts), which routes through the correct dispatcher factory based on the resolved policy mode:

Mode Dispatcher Factory Timeouts Applied
direct new Agent(...)
env-proxy createHttp1EnvHttpProxyAgent(...)
explicit-proxy 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:

  • 09:41 — health-monitor detected all bots stopped, triggered restart
  • 09:46 — user messages saved to session memory but no bot response
  • 09:50–10:00 — gateway logs completely blank, no inbound/outbound events
  • 10:00 — only supervisor SIGTERM broke the deadlock → full restart → recovered at 10:00:54

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 via TELEGRAM_GET_UPDATES_REQUEST_TIMEOUT_MS
  • polling-liveness.ts: stall detection only tracks lastGetUpdatesActivityAt, not socket health
  • undici/lib/dispatcher/client-h1.js:808-810: bodyTimeout triggers util.destroy(socket) — only path that physically closes TCP sockets
  • fetch.ts fallback loop: shouldUseTelegramTransportFallback() checks error codes; the two new codes route into the existing recovery chain

Rev 2 changes (addressing clawsweeper review)

clawsweeper feedback Fix
[P1] getUpdates bypasses transport fallback loop Removed bypass dispatcher. Timeouts now injected via attempt.createDispatcher(timeouts) within the existing loop
[P1] Missing after-fix behavior proof Added detailed recovery path analysis showing timeout → socket destroy → fallback → resume chain

Closes #86031
Related: #108562 #56061 #57743 #78079

@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 16, 2026
@friendfish
friendfish force-pushed the fix/fix_010_telegram_polling_deadlock branch from 72fda3e to c9cc364 Compare July 16, 2026 10:14
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 16, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jul 16, 2026
@clawsweeper

clawsweeper Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 1:00 PM ET / 17:00 UTC.

Summary
Adds undici header/body timeouts for Telegram getUpdates inside the existing transport-attempt loop and classifies timeout errors for dispatcher fallback.

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.

  • Timeout hierarchy: 2 added; 60s and 90s after a 45s abort. Both new socket timers are currently ordered too late to perform the recovery claimed by the patch.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Set the socket timeout to fire before the existing 45-second request abort and add focused ordering coverage.
  • [P2] Post redacted after-fix logs or live output from a proxy, VPN, or TUN network-path change showing timeout, fallback, and resumed polling.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR has before-fix incident context and code analysis but no after-fix network-flap evidence showing timeout, socket recovery or fallback, and resumed getUpdates polling; add redacted logs, terminal output, or a linked runtime artifact, update the PR body, and request @clawsweeper re-review if a fresh review does not start automatically.

Risk before merge

  • [P1] Even after correcting the ordering, recovery remains unproven across direct, environment-proxy, and explicit-proxy dispatchers under an actual network-path change.
  • [P2] Once reachable, the new timeout error codes trigger transport failure accounting and fallback promotion for Telegram polling, so incorrect thresholds could cause unnecessary failover or ingress interruptions.

Maintainer options:

  1. Correct and prove the timeout path (recommended)
    Move the socket timeout ahead of the 45-second abort, add focused regression coverage, and provide redacted live network-flap output before merge.
  2. Pause the transport change
    Leave current polling recovery unchanged if the contributor cannot demonstrate that the new dispatcher timeout reliably closes the affected socket across supported proxy modes.

Next step before merge

  • [P2] The contributor must correct the timeout ordering and provide real behavior proof from the affected network setup; automation cannot supply that contributor-side evidence.

Security
Cleared: The one-file runtime patch adds no dependencies, downloaded code, secret access, workflow permissions, or other concrete security or supply-chain concern.

Review findings

  • [P1] Run the socket timeout before the 45-second abort — extensions/telegram/src/fetch.ts:65-71
Review details

Best 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:

  • [P1] Run the socket timeout before the 45-second abort — extensions/telegram/src/fetch.ts:65-71
    getUpdates is already aborted after 45 seconds, but these dispatcher timeouts are 60 and 90 seconds. The abort therefore wins before either undici timer can close the half-open socket, leaving the claimed recovery inactive. Set the relevant socket timeout above Telegram’s normal 30-second poll but below 45 seconds and cover that ordering. This is the same blocker raised on the prior reviewed head and remains present here.
    Confidence: 0.97

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against fe4a0aae627b.

Label changes

Label justifications:

  • P1: The PR addresses a Telegram ingress deadlock affecting real users, but its current timeout ordering leaves the reported workflow broken.
  • merge-risk: 🚨 message-delivery: Changes to getUpdates timeout and fallback behavior can suppress or delay all inbound Telegram messages when recovery is incorrect.
  • merge-risk: 🚨 availability: The patch changes whether Telegram polling dispatchers recover, fail over, or remain stalled after network-path failures.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR has before-fix incident context and code analysis but no after-fix network-flap evidence showing timeout, socket recovery or fallback, and resumed getUpdates polling; add redacted logs, terminal output, or a linked runtime artifact, update the PR body, and request @clawsweeper re-review if a fresh review does not start automatically.
Evidence reviewed

PR surface:

Source +76. Total +76 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 84 8 +76
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 84 8 +76

What I checked:

  • Current-main transport behavior: Current main routes requests through reusable direct, IPv4, and pinned-IP dispatcher attempts, records failures, and promotes sticky fallbacks; the PR correctly targets this existing owner boundary rather than bypassing it. (extensions/telegram/src/fetch.ts:789, fe4a0aae627b)
  • Timeout ordering remains incorrect: The current PR head sets bodyTimeout to 90,000 ms and headersTimeout to 60,000 ms, while the established getUpdates request abort occurs after 45,000 ms, so neither new socket timer can be the first recovery mechanism. (extensions/telegram/src/fetch.ts:65, 28a3bcc7ddb6)
  • Prior finding continuity: The previous review of 0a80a1c raised “Run the socket timeout before the 45-second abort”; the supplied current-head patch still contains the same 60-second and 90-second constants. (extensions/telegram/src/fetch.ts:65, 28a3bcc7ddb6)
  • Undici timeout contract: Undici documents headersTimeout as the wait for complete response headers and bodyTimeout as the maximum interval between response body chunks; a separately supplied AbortSignal can terminate the request first.
  • Existing liveness recovery: Current main already detects an active getUpdates request exceeding the configured stall threshold and forces a polling restart, so the socket timeout must fire earlier to add the distinct physical-socket recovery claimed by this PR. (extensions/telegram/src/polling-liveness.ts:82, fe4a0aae627b)
  • Real behavior proof is absent: The PR body contains a before-fix incident description and code-path analysis, but no after-fix proxy, VPN, or TUN-flap run showing a timeout error, fallback, and resumed polling. (28a3bcc7ddb6)

Likely related people:

  • Tak Hoffman: Recent merged work restored and tightened Telegram fallback-probe coverage, making this a strong routing candidate for transport-attempt and recovery behavior. (role: fallback-path contributor; confidence: medium; commits: 9446ee8ea3, fc570934de, 366c1d6b9e; files: extensions/telegram/src/fetch.ts, extensions/telegram/src/fetch.test.ts)
  • Julia Bush: Introduced merged Telegram polling-fetch abort behavior during gateway shutdown, which is closely related to abort and socket-lifecycle semantics reviewed here. (role: polling-abort contributor; confidence: medium; commits: e94ebfa084; files: extensions/telegram/src/request-timeouts.ts, extensions/telegram/src/polling-liveness.ts)
  • Peter Steinberger: Carried Telegram fetch-test relocation and several recent Telegram extension test/runtime refactors around the affected transport surface. (role: recent area contributor; confidence: medium; commits: bb3ea2137b, 992b30604d, 86921b624c; files: extensions/telegram/src/fetch.test.ts, extensions/telegram/src/fetch.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (4 earlier review cycles)
  • reviewed 2026-07-16T10:21:36.977Z sha c9cc364 :: needs real behavior proof before merge. :: [P1] Preserve transport fallbacks for getUpdates
  • reviewed 2026-07-16T11:08:59.406Z sha c9cc364 :: needs real behavior proof before merge. :: [P1] Keep getUpdates in the transport fallback loop
  • reviewed 2026-07-16T13:52:38.464Z sha 0a80a1c :: needs real behavior proof before merge. :: [P1] Run the socket timeout before the 45-second abort
  • reviewed 2026-07-16T16:20:41.093Z sha 28a3bcc :: needs real behavior proof before merge. :: [P1] Make the socket timeout precede the request abort

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 16, 2026
@friendfish
friendfish force-pushed the fix/fix_010_telegram_polling_deadlock branch from c9cc364 to 0a80a1c Compare July 16, 2026 13:44
@friendfish

Copy link
Copy Markdown
Author

@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.

@clawsweeper

clawsweeper Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 16, 2026
…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
@friendfish
friendfish force-pushed the fix/fix_010_telegram_polling_deadlock branch from 0a80a1c to 28a3bcc Compare July 16, 2026 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: telegram Channel integration: telegram merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Windows gateway listens but local health/status time out after Telegram polling stall (v2026.5.20)

1 participant