fix(telegram): skip IP-rotation fallback for local socket allocation failures#94624
fix(telegram): skip IP-rotation fallback for local socket allocation failures#94624ZOOWH wants to merge 4 commits into
Conversation
…failures When connect() fails with EADDRNOTAVAIL the kernel cannot assign a source address/port for the new socket. This is a local resource failure, not a remote-reachability problem, so rotating to an alternative Telegram API IP hits the same errno on every attempt — dead code that only produces misleading "Telegram is down" log noise during a host-wide incident. Previously EADDRNOTAVAIL fell through to the code-less `fetch failed` branch whenever undici wrapped the connect failure without propagating `.code` onto the cause chain (the errno lived only in the message text), triggering the useless pinned-IP retry. Detect local-socket-failure errnos from both `.code` and the error message (in the message so detection no longer depends on undici's cause propagation), skip the pinned-IP fallback, and log the real cause. Closes openclaw#94620
openclaw-barnacle
Bot
added
channel: telegram
|
Codex review: needs real behavior proof before merge. Reviewed June 18, 2026, 9:42 PM ET / 01:42 UTC. Summary PR surface: Source +40, Tests +71. Total +111 across 2 files. Reproducibility: no. high-confidence live reproduction is available in this review. Source inspection shows current main can retry code-less Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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: Keep the EADDRNOTAVAIL fail-fast direction, remove or narrow the EAFNOSUPPORT behavior, and require live transport proof or an explicit maintainer override before landing. Do we have a high-confidence way to reproduce the issue? No high-confidence live reproduction is available in this review. Source inspection shows current main can retry code-less Is this the best way to solve the issue? No: the EADDRNOTAVAIL classifier change is a plausible narrow fix, but adding EAFNOSUPPORT is too broad because it can preempt the existing IPv4 recovery path for family-autoselection failures. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2ef0589b760d. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +40, Tests +71. Total +111 across 2 files. 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
|
clawsweeper
Bot
added
proof: sufficient
…codes only The previous ERROR_CODE_MESSAGE_PATTERN extracted all well-known errno tokens from the error message, including ECONNREFUSED. This changed the classification for message-only `ECONNREFUSED` envelopes: before the patch, `ctx.codes.size === 0` so the fallback was triggered via the code-less `fetch failed` branch; after the patch, `ctx.codes.size > 0` but `ECONNREFUSED` is not in FALLBACK_RETRY_ERROR_CODES, so the fallback was skipped for a remote-reachability problem that IP rotation can fix. Restrict the message regex to only LOCAL_SOCKET_FAILURE_ERROR_CODES (EADDRNOTAVAIL, EAFNOSUPPORT) so that ECONNREFUSED and other fallback-retry errnos remain invisible in message-only envelopes and preserve the existing IP-rotation behavior. Those errnos already match via `.code` on the cause chain; matching them from message text too would narrow the fallback classification unintentionally. Add a message-only ECONNREFUSED regression test confirming that the fallback is still triggered. Closes openclaw#94620
openclaw-barnacle
Bot
removed
the
proof: sufficient
|
@clawsweeper re-review Fixed the P2 finding: restricted the message errno parser to only extract local-socket-failure codes (EADDRNOTAVAIL, EAFNOSUPPORT) instead of all well-known errnos. This means ECONNREFUSED in a message-only envelope (no .code propagated) now remains invisible to the parser and falls through to the code-less Added a message-only ECONNREFUSED regression test confirming fallback=true (the previously broken case where the broad regex would have changed the classification). Verified locally: vitest run extensions/telegram/src/fetch.test.ts → 47 passed (5 in the shouldRetryTelegramTransportFallback block). node --import tsx proof confirms ECONNREFUSED msg-only now correctly returns true. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
… regression test LOCAL_SOCKET_FAILURE_MESSAGE_PATTERN extracts EAFNOSUPPORT from message text, but LOCAL_SOCKET_FAILURE_ERROR_CODES only contained EADDRNOTAVAIL. This mismatch meant a message-only EAFNOSUPPORT envelope would have ctx.codes non-empty without triggering the local-socket-failure guard, silently blocking the fallback without emitting the diagnostic warn — a classification gap that changes Telegram retry behavior. Add EAFNOSUPPORT to LOCAL_SOCKET_FAILURE_ERROR_CODES so the guard and message parser are aligned, and add a message-only EAFNOSUPPORT regression test asserting fallback=false and diagnostic warn emitted. Closes openclaw#94620
|
@clawsweeper re-review Fixed the P1 finding: aligned LOCAL_SOCKET_FAILURE_ERROR_CODES with LOCAL_SOCKET_FAILURE_MESSAGE_PATTERN by adding EAFNOSUPPORT to both. Previously EAFNOSUPPORT was extracted from message text but not handled by the local-socket-failure guard, creating a classification gap where it would silently block fallback without the diagnostic warn. Added a message-only EAFNOSUPPORT regression test asserting fallback=false and diagnostic warn emitted. The proof section now includes EAFNOSUPPORT verification output showing it produces the same "local socket allocation failure" diagnostic as EADDRNOTAVAIL. Verified locally: vitest run extensions/telegram/src/fetch.test.ts → 48 passed (6 in the shouldRetryTelegramTransportFallback block). node --import tsx proof confirms EAFNOSUPPORT msg-only returns false with accurate diagnostic, ECONNREFUSED msg-only still returns true. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Transport-level real behavior proof added to PR body. The previous proof only tested the exported classifier function; this updated proof verifies the complete fail-fast chain at the transport level:
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
…r recovery ClawSweeper P1 review finding: EAFNOSUPPORT was incorrectly classified as a local socket fail-fast code, but the Telegram transport fallback chain first promotes to a forced-IPv4 dispatcher (dnsResultOrder=ipv4first, autoSelectFamily=false, forceIpv4=true). A mixed-family autoselection failure producing EAFNOSUPPORT on the IPv6 path is recoverable through the IPv4 dispatcher — blocking EAFNOSUPPORT from fallback would preempt that recovery path, making a recoverable family-mismatch error unrecoverable. Changes: - Remove EAFNOSUPPORT from LOCAL_SOCKET_FAILURE_ERROR_CODES (only EADDRNOTAVAIL remains as fail-fast) - Remove EAFNOSUPPORT from LOCAL_SOCKET_FAILURE_MESSAGE_PATTERN (only EADDRNOTAVAIL is extracted from message text) - Add EAFNOSUPPORT to FALLBACK_RETRY_ERROR_CODES so it triggers fallback when exposed via .code on the cause chain - Update EAFNOSUPPORT test: message-only → true (code-less fetch failed branch triggers IP rotation) - Add new test: EAFNOSUPPORT via .code → true (FALLBACK_RETRY_ERROR_CODES match triggers fallback, IPv4 dispatcher can recover) - Add detailed comments explaining why EAFNOSUPPORT is excluded from fail-fast and included in retry codes EADDRNOTAVAIL remains fail-fast: when the kernel cannot assign a source address/port for any destination, rotating IPs cannot help. EAFNOSUPPORT is different — it signals address-family mismatch, and the sticky IPv4 dispatcher specifically addresses that scenario. Closes openclaw#94620 Co-Authored-By: Claude <[email protected]>
|
@clawsweeper re-review Addressed the P1 finding: EAFNOSUPPORT removed from the fail-fast set and added to FALLBACK_RETRY_ERROR_CODES. The Telegram transport fallback chain first promotes to a forced-IPv4 dispatcher (dnsResultOrder=ipv4first, autoSelectFamily=false, forceIpv4=true). A mixed-family autoselection failure producing EAFNOSUPPORT on the IPv6 path can recover through the IPv4 dispatcher. Blocking EAFNOSUPPORT from fallback would preempt that recovery path, making a recoverable family-mismatch error unrecoverable. Changes:
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
removed
the
status: 📣 needs proof
clawsweeper
Bot
added
the
status: 🔁 re-review loop
1 participant
Summary
When a Telegram
fetchcall fails with a local socket allocation error (EADDRNOTAVAIL or EAFNOSUPPORT), the kernel cannot assign a source address/port for the requested family. However, EAFNOSUPPORT is recoverable via the sticky IPv4 dispatcher (forceIpv4=true, dnsResultOrder=ipv4first, autoSelectFamily=false), while EADDRNOTAVAIL is not recoverable by any IP rotation (the kernel has no source address for any destination).Per the ClawSweeper P1 review finding, the original patch incorrectly classified EAFNOSUPPORT as fail-fast alongside EADDRNOTAVAIL. This would preempt the IPv4 recovery path for mixed-family autoselection failures, making a recoverable family-mismatch error unrecoverable.
Changes
extensions/telegram/src/fetch.ts:LOCAL_SOCKET_FAILURE_ERROR_CODES— fail-fast, bypassing IP-rotation fallback. The kernel cannot assign a source address/port for any destination, so rotating IPs cannot help.LOCAL_SOCKET_FAILURE_ERROR_CODESandLOCAL_SOCKET_FAILURE_MESSAGE_PATTERN, and added toFALLBACK_RETRY_ERROR_CODES. The sticky IPv4 dispatcher can recover from address-family mismatch by forcing IPv4.LOCAL_SOCKET_FAILURE_MESSAGE_PATTERNnarrowed from/\b(EADDRNOTAVAIL|EAFNOSUPPORT)\b/ito/\b(EADDRNOTAVAIL)\b/i— EAFNOSUPPORT in message text no longer extracted to codes (message-only EAFNOSUPPORT envelope falls through to code-lessfetch failedbranch, which correctly triggers IP-rotation fallback).extensions/telegram/src/fetch.test.ts:fallback=true(code-lessfetch failedbranch triggers IP rotation)..codeon cause →fallback=true(FALLBACK_RETRY_ERROR_CODESmatch triggers fallback, IPv4 dispatcher can recover).What changed
extensions/telegram/src/fetch.ts—LOCAL_SOCKET_FAILURE_ERROR_CODESnarrowed to["EADDRNOTAVAIL"]only;EAFNOSUPPORTmoved toFALLBACK_RETRY_ERROR_CODES;LOCAL_SOCKET_FAILURE_MESSAGE_PATTERNnarrowed to/\b(EADDRNOTAVAIL)\b/i; comments updated.extensions/telegram/src/fetch.test.ts— EAFNOSUPPORT message-only test now asserts fallback=true; added EAFNOSUPPORT.codetest asserting fallback=true.What did NOT change (scope boundary)
fetch failedenvelopes still trigger IP-rotation fallback.Linked context
Closes #94620
Real behavior proof
node_modules/.bin/vitest run extensions/telegram/src/fetch.test.ts.code→ false (fail-fast, diagnostic warn)fetch failedbranch).code→ true (triggers fallback viaFALLBACK_RETRY_ERROR_CODESmatch, IPv4 dispatcher can recover).code→ true (unchanged)fetch failed→ true (unchanged).codeand message-only paths. All other classifications unchanged.Tests and validation
node_modules/.bin/vitest run extensions/telegram/src/fetch.test.ts— 49 passedRisk checklist
.code+ message text) for EADDRNOTAVAIL. EAFNOSUPPORT has two recovery paths:.code→FALLBACK_RETRY_ERROR_CODESmatch, message-only → code-lessfetch failedbranch. Diagnostic warn log for EADDRNOTAVAIL preserves operator observability.