Skip to content

fix(weixin_oc): keep a cleared timeout field from disabling the HTTP timeout - #10229

Merged
Soulter merged 1 commit into
AstrBotDevs:masterfrom
Lesereingrape:fix/weixin-oc-timeout-minimum
Sep 27, 2026
Merged

Soulter merged 1 commit into
AstrBotDevs:masterfrom
Lesereingrape:fix/weixin-oc-timeout-minimum

Conversation

@Lesereingrape

@Lesereingrape Lesereingrape commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

weixin_oc_long_poll_timeout_ms and weixin_oc_api_timeout_ms are read with a bare int() in WeixinOCAdapter.__init__, with no lower bound. Both are user-tunable fields — tests/test_weixin_oc_config_metadata.py asserts they are exactly the two personal-WeChat fields the dashboard exposes — and the dashboard's numeric input writes 0 when the box is cleared (dashboard/src/components/shared/ConfigItemRenderer.vue: toNumber() maps parseFloat('') → 0).

Aiohttp reads ClientTimeout(total=0) as no timeout at all, so a cleared field silently removes the deadline from every WeChat API call:

total=0   -> 200 {"ok": true}     # handler slept 0.4s, no timeout enforced
total=-5  -> 200 {"ok": true}
total=0.001 -> raised TimeoutError

The consequence is in the receive loop: _poll_inbound_updates() (astrbot/core/platform/sources/weixin_oc/weixin_oc_adapter.py:1579) issues getupdates with timeout_ms=self.long_poll_timeout_ms, and the loop is written to survive exactly this case (:1763-1769, except asyncio.TimeoutError: ... retry). With the deadline disabled that branch never fires — one stalled connection parks the coroutine forever and the bot goes quiet without a single log line. Same for _poll_qr_status (:1118).

A hand-edited non-numeric value is worse: int('') raises ValueError: invalid literal for int() with base 10: '' inside the constructor, and PlatformManager.initialize (astrbot/core/platform/manager.py:96-97) flattens that to one Failed to initialize platform adapter line, so the platform never starts.

The repository already refuses both shapes for these very keys — login_registration.py:32-37 routes weixin_oc_api_timeout_ms / weixin_oc_long_poll_timeout_ms through _int_config(value, default, 1_000) on the QR-login path, and this adapter has its own _get_int_config(key, default, minimum) for its other numeric knobs. Only the constructor reads were left unguarded.

Modifications / 改动点

  • Route the three numeric constructor fields (weixin_oc_qr_poll_interval, weixin_oc_long_poll_timeout_ms, weixin_oc_api_timeout_ms) through the adapter's existing _get_int_config, with a MIN_TIMEOUT_MS = 1_000 floor for the two millisecond fields — the same floor login_registration._int_config already applies to the same keys. Defaults and valid values are unchanged; a 0, a negative, or a non-numeric value now falls back instead of disabling the timeout or killing the adapter.

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

New tests/test_weixin_oc_adapter_timeout_floor.py (7 cases: cleared field → 1000, value below the floor → 1000, tuned value kept, unset → documented default, non-numeric → default instead of raising, and the value that actually reaches self.client.api_timeout_ms).

On master @ 69ae35a3 (fails before the change):

FAILED tests/test_weixin_oc_adapter_timeout_floor.py::test_cleared_api_timeout_falls_back_to_a_positive_floor
FAILED tests/test_weixin_oc_adapter_timeout_floor.py::test_cleared_long_poll_timeout_falls_back_to_a_positive_floor
FAILED tests/test_weixin_oc_adapter_timeout_floor.py::test_negative_timeouts_are_clamped_like_the_login_flow
FAILED tests/test_weixin_oc_adapter_timeout_floor.py::test_config_value_below_the_floor_still_clamped
FAILED tests/test_weixin_oc_adapter_timeout_floor.py::test_a_non_numeric_field_does_not_kill_the_platform_adapter
5 failed, 2 passed in 8.46s

The fifth failure is the crash itself:

>           int(platform_config.get("weixin_oc_qr_poll_interval", 1)),
E           ValueError: invalid literal for int() with base 10: ''
astrbot/core/platform/sources/weixin_oc/weixin_oc_adapter.py:132: ValueError

After the change:

7 passed in 6.18s

Adjacent and whole-suite regression, Windows / Python 3.12, TESTING=true:

tests/test_weixin_oc_adapter_timeout_floor.py tests/test_weixin_oc_login_registration.py tests/test_weixin_oc_config_metadata.py
12 passed in 6.52s

pytest tests -q
3526 passed, 82 skipped in 338.62s (0:05:38)

Lint gate (repo-wide job, pinned version):

ruff 0.15.22 check  astrbot/core/platform/sources/weixin_oc/ tests/test_weixin_oc_adapter_timeout_floor.py -> All checks passed!
ruff 0.15.22 format --check  ...  -> 5 files already formatted

Docs

No docs/zh or docs/en update: the two fields keep the same meaning, defaults and unit. This change only rejects the values that the login path already rejects, so no documented behavior moves.

Checklist / 检查清单

The boxes below are left unticked on purpose: this patch was prepared by an automated agent session working for the account owner, so the first-person statements are the owner's to confirm, not the agent's. The measured evidence each item would attest to is inline above.


📋 查看中文说明

WeixinOCAdapter.__init__ 用裸 int() 读取 weixin_oc_long_poll_timeout_ms 与 weixin_oc_api_timeout_ms,且没有下限。这两项恰好是 Dashboard 对个人微信开放调参的字段(tests/test_weixin_oc_config_metadata.py 对此有断言),而 Dashboard 的数字输入框被清空时 toNumber() 会写入 0。aiohttp 把 ClientTimeout(total=0) 解释为「完全不设超时」,于是长轮询 getupdates 的超时保护失效:接收循环里 except asyncio.TimeoutError 的重试分支永远不会触发,一条卡住的连接就能让机器人静默且没有任何日志。若配置被手工写成非数字,int('') 会在构造期抛 ValueError,PlatformManager.initialize 只留一行日志,该平台直接起不来。仓库在扫码登录路径上(login_registration.py:32-37)早已用 _int_config(value, default, 1_000) 对同样的 key 做了下限保护,本适配器的 _get_int_config 也给它自己的其他数值项设了下限——只有构造函数这三行漏了。本 PR 让这三行改走同一个 helper,默认值与合法取值均不变。新增 7 条用例:修改前 5 failed, 2 passed,修改后 7 passed;相关三文件 12 passed;Windows 全量 3526 passed, 82 skipped;ruff 0.15.22 check/format 均通过。检查清单未勾选是因为该补丁由自动化 agent 会话代为准备,第一人称声明需由账号本人确认,实测证据已附于上文。

Summary by Sourcery

Keep Weixin OC polling and API timeouts positive and robust against invalid dashboard configuration values.

Bug Fixes:

  • Prevent invalid or cleared Weixin OC timeout settings from disabling HTTP timeouts or preventing the platform adapter from starting.

Enhancements:

  • Apply consistent minimum-value and fallback handling to Weixin OC QR polling and HTTP timeout configuration.

Tests:

  • Add coverage for timeout floors, fallback defaults, preserved valid values, and non-numeric configuration handling.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Approved.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@Soulter
Soulter merged commit 559b171 into AstrBotDevs:master Sep 27, 2026
23 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.

2 participants