Skip to content

fix(platform): keep the Satori heartbeat and reconnect delays from being disabled by a cleared field - #10248

Merged
Soulter merged 2 commits into
AstrBotDevs:masterfrom
Lesereingrape:fix/satori-heartbeat-floor
Sep 27, 2026
Merged

Soulter merged 2 commits into
AstrBotDevs:masterfrom
Lesereingrape:fix/satori-heartbeat-floor

Conversation

@Lesereingrape

@Lesereingrape Lesereingrape commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

satori_heartbeat_interval and satori_reconnect_delay are declared "type": "int" with no minimum in astrbot/core/config/default.py:743-752, with template defaults 10 and 5 (:533-534), and both are handed straight to asyncio.sleep — heartbeat_loop sleeps self.heartbeat_interval between two PING frames (satori_adapter.py:221), and the reconnect loop uses self.reconnect_delay as the base of its exponential backoff (:135):

self.heartbeat_interval = self.config.get("satori_heartbeat_interval", 10)
self.reconnect_delay = self.config.get("satori_reconnect_delay", 5)

dict.get only applies the default when the key is absent. A 0 reaches the constructor from the dashboard as soon as the box is cleared — toNumber() maps parseFloat('') to 0 (dashboard/src/components/shared/ConfigItemRenderer.vue:361-364) — and a string reaches it from a hand-edited data/config.json ("satori_heartbeat_interval": "30"). Both shapes are accepted today.

Measured against the real heartbeat_loop on master @ 42ad2774, with a stub WebSocket that counts frames, over a 0.2 s window:

satori_heartbeat_interval PING frames in 0.2 s heartbeat task
unset (default 10) 0 alive
0 (cleared field) 32010 alive, spinning
'10' (quoted config value) 0 dead

The 0 row is a busy loop against the upstream Satori gateway — the interval is a seconds value, so removing the pause removes the rate limit on its own protocol keep-alive. The '10' row is worse than an error: asyncio.sleep('10') raises TypeError: '<=' not supported between instances of 'str' and 'int', heartbeat_loop's own except Exception at :240-241 logs 心跳任务异常: ... and returns, so the connection stays open with no heartbeat at all and the platform only notices when the gateway drops it. A 0 satori_reconnect_delay removes the pause between reconnect attempts in the same way.

Modifications / 改动点

  • Route both fields through coerce_int_config (astrbot/core/utils/config_number.py), which is already the repo's answer for exactly this class of field: it handles bool/int/str/other, keeps a valid value, and clamps below min_value. With min_value=1 a cleared box or a negative number falls back to one second — the smallest value that is still a pause — and a quoted number like "30" is honored instead of killing the task. Valid positive values are unchanged. This mirrors what tg_adapter.py already does inline for its own restart delay (telegram_polling_restart_delay), whose guard logs the same reason: "enforcing minimum of 0.1s to avoid tight restart loops".

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

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

New tests/test_satori_adapter_heartbeat_floor.py (7 cases, following tests/test_weixin_oc_adapter_timeout_floor.py): the cleared, negative and fractional fields fall back, a non-numeric one keeps the documented default instead of reaching asyncio.sleep, a numeric string is honored, and valid tuned values plus the unset defaults pass through untouched.

On master @ 42ad2774 (before the change):

FAILED tests/test_satori_adapter_heartbeat_floor.py::test_cleared_fields_fall_back_to_a_positive_floor
FAILED tests/test_satori_adapter_heartbeat_floor.py::test_negative_fields_are_clamped
FAILED tests/test_satori_adapter_heartbeat_floor.py::test_a_fractional_interval_cannot_lose_the_pause
FAILED tests/test_satori_adapter_heartbeat_floor.py::test_non_numeric_fields_do_not_kill_the_platform_adapter
FAILED tests/test_satori_adapter_heartbeat_floor.py::test_a_numeric_string_is_respected
5 failed, 2 passed

With the change: 7 passed. The two that already pass are the controls (a tuned 60/20, and unset fields).

The gate is the one CI runs for these paths:

ruff format --check astrbot/core/platform/sources/satori/satori_adapter.py tests/test_satori_adapter_heartbeat_floor.py   # 2 files already formatted
ruff check astrbot/core/platform/sources/satori/satori_adapter.py tests/test_satori_adapter_heartbeat_floor.py           # All checks passed!

Two notes for the reviewer:

  • .github/copilot-instructions.md:27 says "Do not generate test files for now." I kept the test here because it is the measurement behind the table above, and tests/test_weixin_oc_adapter_timeout_floor.py shows this kind of guard test is wanted in the adapter layer — but say the word and I will drop the file and leave the two-line fix.
  • I noticed feat: 优化 satori 适配器 #6095 rewrites this adapter on the satori-python SDK. It is dirty against current master and has not been updated since 2026-04-28, so I did not build on it; if it lands, the same coerce_int_config call should be kept at whichever line ends up reading those two fields.

Disclosure: this PR was prepared, tested and submitted by an AI agent working on behalf of the account owner.

Summary by Sourcery

Ensure Satori heartbeat and reconnect timing always use safe positive integer delays.

Bug Fixes:

  • Prevent Satori heartbeat and reconnect loops from spinning or failing when configuration values are cleared, negative, fractional, or provided as strings.

Enhancements:

  • Normalize Satori timing configuration values while preserving valid positive settings and documented defaults.

Tests:

  • Add coverage for Satori timing configuration fallbacks, clamping, string values, invalid input, and valid defaults.

@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 found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="astrbot/core/platform/sources/satori/satori_adapter.py" line_range="61-66" />
<code_context>
         self.auto_reconnect = self.config.get("satori_auto_reconnect", True)
-        self.heartbeat_interval = self.config.get("satori_heartbeat_interval", 10)
-        self.reconnect_delay = self.config.get("satori_reconnect_delay", 5)
+        self.heartbeat_interval = coerce_int_config(
+            self.config.get("satori_heartbeat_interval", 10),
+            default=10,
+            min_value=1,
+            field_name="satori_heartbeat_interval",
+            source="Satori config",
+        )
+        self.reconnect_delay = coerce_int_config(
</code_context>
<issue_to_address>
**issue (bug_risk):** `coerce_int_config` raises `OverflowError` when the configuration value is positive or negative infinity, because it calls `int(value)` without catching `OverflowError`; the Satori adapter then fails during construction instead of falling back to its default.

**Triggers:** When a hand-edited JSON configuration contains `Infinity`, `-Infinity`, or a numeric value large enough for Python's JSON parser to produce an infinite float.

**Suggested fix:** Catch `OverflowError` alongside `TypeError` and `ValueError` in `coerce_int_config`, or reject non-finite numeric values before conversion.
</issue_to_address>

Sourcery assessment

Approval pending. 1 finding to address first.

Blocking findings: astrbot/core/platform/sources/satori/satori_adapter.py:66


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

Comment on lines +61 to +66
self.heartbeat_interval = coerce_int_config(
self.config.get("satori_heartbeat_interval", 10),
default=10,
min_value=1,
field_name="satori_heartbeat_interval",
source="Satori config",

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.

issue (bug_risk): coerce_int_config raises OverflowError when the configuration value is positive or negative infinity, because it calls int(value) without catching OverflowError; the Satori adapter then fails during construction instead of falling back to its default.

Triggers: When a hand-edited JSON configuration contains Infinity, -Infinity, or a numeric value large enough for Python's JSON parser to produce an infinite float.

Suggested fix: Catch OverflowError alongside TypeError and ValueError in coerce_int_config, or reject non-finite numeric values before conversion.

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

3 participants