Repository navigation
fix(channels): ignore non-finite WeChat server longpoll timeouts - #7898
Conversation
The getupdates response's longpolling_timeout_ms was passed through float() with only TypeError/ValueError handled. A huge JSON integer raised OverflowError before the message loop and cursor write, so the same batch was re-polled forever, while Infinity/NaN were stored and turned the next request timeout into inf/nan. Apply the same positive-finite rule the channel already uses for configured timing values: catch OverflowError and reject non-finite results, keeping the previous valid timeout. Co-Authored-By: Claude Code <[email protected]>
github-actions
Bot
added
area:backend
willem-bd
left a comment
willem-bd
left a comment
There was a problem hiding this comment.
Verified against head e772ed7 — the fix is correct: float(10**400) raises OverflowError (now caught), the string forms "1e400"/"Infinity"/"-Infinity"/"NaN" parse to non-finite (now rejected by isfinite), the isfinite-before-<= 0 ordering handles NaN correctly, and valid values are untouched. The guard now exactly mirrors _coerce_float in the same file, matching the positive-finite contract pinned in backend/app/channels/AGENTS.md, and the placement in _poll_loop (line 749, before the message loop and cursor write) matches the batch-drop failure mode described. Tests are genuinely red on parent as claimed — the 3s wait_for bound beats the parent's 5s retry delay — and the integration test asserts message delivery, cursor advance, timeout retention, and the next request's timeout=47.0. Two minor non-blocking notes below.
| except (OverflowError, TypeError, ValueError): | ||
| return | ||
| if timeout_ms <= 0: | ||
| if not math.isfinite(timeout_ms) or timeout_ms <= 0: |
There was a problem hiding this comment.
Robustness gap this fix leaves open: huge-but-finite values still pass this guard. A server response of longpolling_timeout_ms: 1e308 (or a ~300-digit int) is finite, so _server_longpoll_timeout_seconds becomes ~1e305 and the read timeout in _poll_loop (max(self._current_longpoll_timeout_seconds() + 5.0, 10.0)) becomes effectively unbounded — the same "read timeout disabled" symptom this PR fixes for inf, just via a finite value. Symmetrically, tiny positive values (e.g. 1) set a near-zero longpoll and turn the no-sleep success path in _poll_loop into a tight re-poll hot loop — the failure mode backend/app/channels/AGENTS.md says configured values guard against ("prevent hot loops or endless sleeps"). Since this value is server-controlled (the PR's own stated trigger: misbehaving/changed iLink response), consider clamping to a sane range — e.g. ignore the value (falling back to the configured _polling_timeout, like _coerce_float falls back to its default) when it falls outside something like [1s, 300s]. Not blocking.
| try: | ||
| timeout_ms = float(raw_timeout) | ||
| except (TypeError, ValueError): | ||
| except (OverflowError, TypeError, ValueError): |
There was a problem hiding this comment.
Nit: this guard is now a line-for-line duplicate of _coerce_float (same exception tuple, same isfinite/> 0 rule, same file). Reusing it would keep the two rules from drifting:
timeout_ms = self._coerce_float(raw_timeout, 0.0)
if timeout_ms <= 0:
returnEither form works; not blocking.
|
Thanks — I see |
There was a problem hiding this comment.
🟢 Approval recommended
There are no active review issues for the WeChat timeout-validation changes.
No open issues 🎉
What changed in this PR
Hardens WeChat long polling so invalid server timeout hints do not interrupt message processing or replace the last valid timeout.
Changes:
- Reuses float validation and accepts server hints only within 1–300 seconds.
- Adds regression coverage for invalid values, boundary values, opt-out behavior, and continued message processing and cursor advancement.
- Documents the accepted range and fallback behavior.
| File | Description |
|---|---|
| README.md | Documents timeout limits, fallback behavior, and opt-out configuration. |
| backend/tests/test_wechat_channel.py | Tests timeout validation and poll-loop continuity after invalid hints. |
| backend/app/channels/wechat.py | Validates server hints before updating the timeout. |
| backend/app/channels/AGENTS.md | Records the server timeout validation rule. |
Review effort: Balanced
Why
WeChat's iLink
getupdatesresponse includeslongpolling_timeout_ms, which the channel honors whenrespect_server_longpoll_timeoutis on (the default)._update_longpoll_timeoutparsed it withfloat()but caught onlyTypeError/ValueError.Python's JSON decoder accepts
1e400,InfinityandNaN, and a 309+ digit integer is a valid JSON number. Two failures followed:OverflowErrorat the call site in_poll_loop, before the per-message loop and the cursor write. The broad handler logged and re-polled with the same cursor, so the whole batch was dropped andget_updates_bufnever advanced — every re-poll hit the same value.Infinity/NaNwere stored, so the next request usedtimeout=inf/naninstead of the configured value; the read timeout was effectively disabled.The channel already requires positive finite seconds for its configured timing values (
backend/app/channels/AGENTS.md:76); the server-provided value should follow the same rule. The trigger is server-controlled over trusted TLS, so this is robustness against a misbehaving or changed iLink response, not an external-attacker path.What changed
_update_longpoll_timeoutcatchesOverflowErrorand ignores non-finite values, preserving the previous valid timeout. Valid values are unchanged (42000 -> 42.0), matching the existing_coerce_floatrule in the same file.1e400,Infinity,-Infinity,NaN, zero and negative values, plus an integration test that drives the real_poll_loopthrough a valid 42s batch and a huge-int batch carrying a message, asserting the message reaches_handle_update, the cursor advances, 42.0 is retained, and the next request usestimeout=47.0.Surface area
Backend IM channel runtime only: no endpoint/SSE shape change and no behavior change for valid server responses.
Verification
From
backend/:python -m pytest tests/test_wechat_channel.py -q→ 107 passed, 2 skipped.a46d9221: 7 failed, 4 passed —OverflowError: int too large to convert to floatfor10**400, andassert inf == 42.0/assert nan == 42.0for non-finite values. The integration test fails deterministically under its 3s bound (parent's broad handler sleeps the 5s retry delay).ruff checkandruff format --checkon both files → clean. An independent reviewer reran the red/green and mutation checks in a separate worktree.AI assistance
Tool(s) used: Claude Code
How you used it: AI reproduced the overflow/non-finite path, wrote the regression tests (red on parent) and the guard, and ran the verification above. An independent review reran the red/green and mutation checks against a separate worktree.
🤖 Generated with Claude Code