Repository navigation
fix(core): keep a one-item segmented-reply interval from dropping the whole reply - #10254
Lesereingrape wants to merge 1 commit into
Conversation
… reply _Calc_comp_interval reads self.interval[1], but the "interval" string is parsed into a list of any length, so interval="3" left a single element and raised IndexError before the first segment was sent. The delay is computed outside the send try/except, so the exception escaped process() into the dispatcher task callback and the whole reply was dropped. A non-finite bound fails one step later: random.uniform() yields nan and asyncio.sleep(nan) never resumes on the supported Python 3.12. Validate the parsed pair where the existing unparseable-text fallback already lives, mirroring _resolve_log_base, and cover both shapes with a focused test module.
There was a problem hiding this comment.
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/pipeline/respond/stage.py" line_range="52" />
<code_context>
+ f"using the default {list(DEFAULT_INTERVAL)} instead.",
+ )
+ return list(DEFAULT_INTERVAL)
+ if not all(math.isfinite(value) for value in values):
+ # random.uniform() returns nan for a nan or inf bound, and on the
+ # supported Python 3.12 asyncio.sleep(nan) never resumes, so the
</code_context>
<issue_to_address>
**issue (bug_risk):** Checking that both configured bounds are finite does not ensure that the value returned by `random.uniform()` is a usable finite sleep delay. For example, `interval: "1e308,1e308"` passes validation but causes `asyncio.sleep(1e308)` to remain pending for an effectively unbounded time, while ranges such as `-1e308,1e308` can overflow inside `random.uniform()` and produce an infinite delay.
**Triggers:** When a user supplies extremely large finite interval bounds.
**Suggested fix:** Validate the resulting delay range against the supported sleep semantics, such as requiring non-negative bounds below an explicit maximum, or clamp/reject values that can produce an unusable delay.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: astrbot/core/pipeline/respond/stage.py:52
| f"using the default {list(DEFAULT_INTERVAL)} instead.", | ||
| ) | ||
| return list(DEFAULT_INTERVAL) | ||
| if not all(math.isfinite(value) for value in values): |
There was a problem hiding this comment.
issue (bug_risk): Checking that both configured bounds are finite does not ensure that the value returned by random.uniform() is a usable finite sleep delay. For example, interval: "1e308,1e308" passes validation but causes asyncio.sleep(1e308) to remain pending for an effectively unbounded time, while ranges such as -1e308,1e308 can overflow inside random.uniform() and produce an infinite delay.
Triggers: When a user supplies extremely large finite interval bounds.
Suggested fix: Validate the resulting delay range against the supported sleep semantics, such as requiring non-negative bounds below an explicit maximum, or clamp/reject values that can produce an unusable delay.
platform_settings.segmented_reply.intervalis a free-text string in the dashboard config (astrbot/core/config/default.pydeclares it as"interval": {"type": "string"}, hint格式:最小值,最大值(如:1.5,3.5)), andRespondStage.initializeparsed it into a list of any length:_calc_comp_intervalthen readsself.interval[1]:So a one-item value —
interval: "3", which is what a user writes when they want a flat 3-second pause — leaves a single-element list and raisesIndexErroron the first component:That call site is outside the
trythat wraps the send (astrbot/core/pipeline/respond/stage.py:298-300), and the pipeline scheduler has no handler either, so the exception travels toEventDispatcher._on_task_done(astrbot/core/event_bus.py:56-63) and surfaces as one line,Pipeline task failed.— no segment of the reply is ever sent. This is asymmetric with an unparseable value such as"abc", which the existingexcept BaseExceptionalready degrades to the default pair, and withlog_base, which since #10231 goes through_resolve_log_basefor exactly this reason.A non-finite bound fails one step later and even less visibly:
float("nan")parses fine,random.uniform(nan, 3.5)returnsnan, and on the supported floor (requires-python = ">=3.12")asyncio.sleep(nan)never resumes — measured on Python 3.12.11,sleep(nan)was still pending after a 6 s deadline whilesleep(0.2)andsleep(3.5)both finished, whereas on 3.13 it raisesValueError: Invalid delay: NaN (not a number). Either way the user gets no reply, one of which also leaks a task.Modifications / 改动点
This is NOT a breaking change. / 这不是一个破坏性变更。
Add
DEFAULT_INTERVAL = (1.5, 3.5)and a module-level_resolve_interval()next to the existing_resolve_log_base(), and route the parsed list through it at the place where the unparseable-text fallback already lives. A list that is not exactly two finite numbers now logs an error and falls back to the documented default instead of reachingself.interval[1]/asyncio.sleep(). Defaults, valid two-value settings and the"abc"path are unchanged.Screenshots or Test Results / 运行截图或测试结果
New
tests/test_respond_stage_interval_shape.py(5 cases: one-item string, extra values, non-finite bounds, a valid pair still honoured, unparseable text still falls back).Before the change, on
master@1ef16908(test file only, source unmodified):After the change:
Adjacent suites (respond stage, smoke, message tools, aiocqhttp reply), Windows / Python 3.12:
Whole suite, Windows / Python 3.12,
TESTING=true:Lint gate (repo-wide job, pinned version):
Docs
No
docs/zhordocs/enupdate: the documented format stays最小值,最大值and no config label, entry point or default moves. This change only rejects the shapes the same helper already rejects forlog_base, so no documented behavior changes.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.
📋 查看中文说明
platform_settings.segmented_reply.interval在 Dashboard 里是自由文本(default.py声明为{"type": "string"},提示「格式:最小值,最大值(如:1.5,3.5)」)。RespondStage.initialize用[float(t) for t in ...]把它解析成任意长度的列表,而_calc_comp_interval固定读self.interval[1]:用户想要「固定 3 秒间隔」而写成interval: "3"时,解析结果是[3.0],第一段消息就抛IndexError: list index out of range。这个调用点在包裹发送逻辑的try之外,pipeline scheduler 也不捕获,异常最终落到EventDispatcher._on_task_done,只留下一行Pipeline task failed.,整条回复一个字都发不出去。相比之下"abc"这种解析失败的值早就被except兜底回默认区间,log_base也自 #10231(本仓库已合并)起走_resolve_log_base——只有 interval 的「形状」漏了校验。另外float("nan")能解析成功,random.uniform(nan, 3.5)返回nan,而在项目支持下限 Python 3.12 上asyncio.sleep(nan)永不返回(实测 6 秒仍在挂起,对照组sleep(0.2)/sleep(3.5)均正常结束),3.13 上则抛ValueError;两种结果都是用户收不到回复,前者还会泄漏一个 task。本 PR 在原有兜底位置新增DEFAULT_INTERVAL与_resolve_interval(),要求「恰好两个有限数值」,否则记一条 error 并回退到文档默认值;合法配置、默认值与"abc"路径行为不变。新增 5 条用例:修改前3 failed, 2 passed,修改后5 passed;相关四文件96 passed;Windows 全量见上方英文段落;ruff 0.15.22check/format 均通过。检查清单未勾选是因为该补丁由自动化 agent 会话代为准备,第一人称声明需由账号本人确认,实测证据已附于上文。Summary by Sourcery
Validate segmented-reply intervals and safely fall back to the default when the configured value is unusable.
Bug Fixes:
Enhancements:
Tests: