feat: 无日志超时重启,多次未响应退出#49
Conversation
Agent-Logs-Url: https://github.com/123sleaf-123/OneDragon-ScriptChainer/sessions/5cac24b1-51ab-4dcd-8883-ac1058343e20 Co-authored-by: 123sleaf-123 <80097831+123sleaf-123@users.noreply.github.com>
…eval fix: read script_path from config instead of elided UI label text 测试bug修复成功
- ScriptConfig 新增 no_log_timeout_seconds / no_log_max_retries 字段 - script_runner: stdout 回调追踪最后输出时间,监控循环检测静默超时并抛出异常,run_script 实现完整重试循环 - GUI: ScriptEditDialog 新增开关+超时秒数+最大重试次数配置项 Agent-Logs-Url: https://github.com/123sleaf-123/OneDragon-ScriptChainer/sessions/d68e27fd-bfe2-4a58-a5f9-74d96e436f06 Co-authored-by: 123sleaf-123 <80097831+123sleaf-123@users.noreply.github.com>
…e with no-log timeout retry Co-authored-by: 123sleaf-123 <80097831+123sleaf-123@users.noreply.github.com>
…-check feat: 无日志超时重启——游戏未响应自动重新执行 测试测试
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough为脚本执行器添加“无日志超时重试”功能:新增配置字段、在脚本设置对话框加入相关 UI 控件、扩展运行监控以跟踪最近日志时间并在超时触发重试;增加 LogNotifier.flush() 以便在重试间推送日志通知。 Changes无日志超时重试
Sequence DiagramsequenceDiagram
participant User as 用户
participant UI as 设置对话框
participant Config as 配置对象
participant Runner as 脚本执行器
participant Process as 子进程
participant Monitor as 监控循环
participant Notifier as LogNotifier
User->>UI: 配置超时与重试次数并保存
UI->>Config: 写入 no_log_timeout_seconds / no_log_max_retries
User->>Runner: 启动脚本链
loop 重试循环(最多 max_retries 次)
Runner->>Process: 启动子进程
Runner->>Monitor: 启动监控,初始化 state.last_log_time
Process-->>Monitor: 输出日志
Monitor->>Monitor: 更新 state.last_log_time
alt 达到无日志超时
Monitor-->>Runner: 抛出 _NoLogTimeoutError
Runner->>Process: 强制清理进程
Runner->>Notifier: 调用 flush() 推送日志
Runner->>Runner: 发送重试通知并继续下一次重试
else 脚本正常完成
Monitor-->>Runner: 子进程退出,正常结束
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
总览(更新)该变更为脚本链接器添加了无日志超时重试功能,包括配置字段定义、UI 控制面板以及核心重试执行逻辑,用于在游戏未响应(无日志输出超过指定时间)时自动重新执行脚本。 庆贺诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 50 minutes and 47 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/script_chainer/win_exe/script_runner.py (3)
461-465: 异常路径清理与重试路径不一致。
- Line 463 在
_wait_for_subprocess_ready失败时使用pm.kill(),而 Line 474 在静默超时路径使用_cleanup_processes(script_config, pm)。前者不会根据kill_game_after_done处理游戏进程,若启动过程中已经拉起了游戏,这里不会关闭。- 该行为与 PR 之前一致,但现在放在重试循环内,建议统一使用
_cleanup_processes,避免半启动状态的游戏残留,特别是当后续脚本依赖"游戏已关闭"作为前置条件时。♻️ 建议统一
- if not _wait_for_subprocess_ready(pm, script_path, state, expect_target=expect_target): - print_message(f'子进程创建失败 {script_path}', level='ERROR') - pm.kill() - _active_pm = None - return + if not _wait_for_subprocess_ready(pm, script_path, state, expect_target=expect_target): + print_message(f'子进程创建失败 {script_path}', level='ERROR') + _cleanup_processes(script_config, pm) + _active_pm = None + return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/script_chainer/win_exe/script_runner.py` around lines 461 - 465, The failure branch that calls pm.kill() after _wait_for_subprocess_ready(script_path, ...) returns false should be changed to use the same cleanup used by the timeout branch: call _cleanup_processes(script_config, pm) (so game processes respect kill_game_after_done) and then set _active_pm = None and return; replace the pm.kill() call with this cleanup to ensure consistent cleanup during retries and ensure script_config is in scope when invoking _cleanup_processes.
138-160: 回调中更新时间戳在 CPython 下依赖 GIL 的原子性,建议在注释中显式说明。
last_log_time[0] = time.time()由子进程输出读取线程执行,监控线程在主线程读取;CPython 下对单个列表槽位的赋值是原子的,因此无需额外加锁,但这一假设对读者并非显而易见。可在_make_stdout_callback文档中补充"仅在 CPython/GIL 下线程安全"的说明,避免未来有人替换运行时或改为多元素结构时埋坑。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/script_chainer/win_exe/script_runner.py` around lines 138 - 160, Update the _make_stdout_callback docstring to explicitly document that the last_log_time single-slot list mutation (last_log_time[0] = time.time()) performed in _on_stdout is only thread-safe due to CPython's GIL; note that this assumption does not hold on alternative Python runtimes or if last_log_time changes to a multi-element structure, and recommend using a threading.Lock or a thread-safe container if portability or more complex state is needed.
472-488: 重试/放弃路径逻辑正确,但有两点小建议。
- Line 476 的
if attempt < max_retries: continue / else: return可扁平化为直接continue——当循环自然结束时(最后一次 attempt 仍超时)再打印"已达最大重试次数"并返回,可减少一层嵌套,但当前写法可读性尚可,保留无妨。_cleanup_processes在超时重试时会根据kill_game_after_done同时终止游戏;若用户希望保留游戏会话以便下次脚本复用,该行为可能过于激进。建议在文档字符串(Line 417–420)中明确说明"重试时游戏进程也会按kill_game_after_done被终止",便于使用者知情。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/script_chainer/win_exe/script_runner.py` around lines 472 - 488, Flatten the retry control flow and document the aggressive game-termination behavior: replace the current if attempt < max_retries ... else ... block by making the loop simply continue on timeout and then, after the retry loop completes, print the "已达最大重试次数" error and return (use the existing attempt, max_retries, and print_message symbols), and update the function/module docstring (around the existing doc area that mentions _cleanup_processes and kill_game_after_done) to explicitly state that _cleanup_processes will terminate the game process during retries according to the kill_game_after_done setting so callers know retries will also stop the game session.src/script_chainer/gui/page/script_setting_interface.py (1)
252-263: 初始化逻辑正确,但注意关闭开关后用户之前配置的 timeout 值会丢失。Line 257 在
no_log_enabled=False时将输入框显示值回退为300,而get_config_value在关闭时写回0,因此一旦用户关闭开关并保存,下次打开时无法恢复其原先配置的秒数(例如 600)。这与现有notify_log_interval的行为一致,可以接受;如需保留“上次配置值”,可考虑引入单独的 display 缓存字段。仅作提醒。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/script_chainer/gui/page/script_setting_interface.py` around lines 252 - 263, The UI currently resets no_log_timeout_input to 300 when no_log_enabled is False, causing the user's previous timeout to be lost (get_config_value writes 0 when disabled); to preserve the last configured value introduce a cache field (e.g. self._last_no_log_timeout_seconds) and update it whenever no_log_timeout_input or config is changed, keep setting no_log_timeout_input.setValue(self._last_no_log_timeout_seconds) when re-enabling the switch, and ensure saving logic writes the cached value back when the switch is enabled; update handlers for no_log_timeout_switch, no_log_timeout_input and the save/apply logic to use this cache instead of hardcoding 300.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/script_chainer/win_exe/script_runner.py`:
- Around line 434-471: last_log_time is set before launching and waiting for the
child process, so slow starts can trigger an immediate no-log timeout; to fix,
update/refresh last_log_time (e.g., set last_log_time[0] = time.time())
immediately after the call to _wait_for_subprocess_ready/_launch_script and
before calling _monitor_script_done so the silence timer begins when monitoring
actually starts, and remove the earlier attempt>0 reset of last_log_time to
avoid redundant/early resets; reference last_log_time, _launch_script,
_wait_for_subprocess_ready, _monitor_script_done and the attempt>0 reset branch
when making the change.
---
Nitpick comments:
In `@src/script_chainer/gui/page/script_setting_interface.py`:
- Around line 252-263: The UI currently resets no_log_timeout_input to 300 when
no_log_enabled is False, causing the user's previous timeout to be lost
(get_config_value writes 0 when disabled); to preserve the last configured value
introduce a cache field (e.g. self._last_no_log_timeout_seconds) and update it
whenever no_log_timeout_input or config is changed, keep setting
no_log_timeout_input.setValue(self._last_no_log_timeout_seconds) when
re-enabling the switch, and ensure saving logic writes the cached value back
when the switch is enabled; update handlers for no_log_timeout_switch,
no_log_timeout_input and the save/apply logic to use this cache instead of
hardcoding 300.
In `@src/script_chainer/win_exe/script_runner.py`:
- Around line 461-465: The failure branch that calls pm.kill() after
_wait_for_subprocess_ready(script_path, ...) returns false should be changed to
use the same cleanup used by the timeout branch: call
_cleanup_processes(script_config, pm) (so game processes respect
kill_game_after_done) and then set _active_pm = None and return; replace the
pm.kill() call with this cleanup to ensure consistent cleanup during retries and
ensure script_config is in scope when invoking _cleanup_processes.
- Around line 138-160: Update the _make_stdout_callback docstring to explicitly
document that the last_log_time single-slot list mutation (last_log_time[0] =
time.time()) performed in _on_stdout is only thread-safe due to CPython's GIL;
note that this assumption does not hold on alternative Python runtimes or if
last_log_time changes to a multi-element structure, and recommend using a
threading.Lock or a thread-safe container if portability or more complex state
is needed.
- Around line 472-488: Flatten the retry control flow and document the
aggressive game-termination behavior: replace the current if attempt <
max_retries ... else ... block by making the loop simply continue on timeout and
then, after the retry loop completes, print the "已达最大重试次数" error and return (use
the existing attempt, max_retries, and print_message symbols), and update the
function/module docstring (around the existing doc area that mentions
_cleanup_processes and kill_game_after_done) to explicitly state that
_cleanup_processes will terminate the game process during retries according to
the kill_game_after_done setting so callers know retries will also stop the game
session.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fa5d2843-981e-46ca-bd86-d83a5c63576f
📒 Files selected for processing (3)
src/script_chainer/config/script_config.pysrc/script_chainer/gui/page/script_setting_interface.pysrc/script_chainer/win_exe/script_runner.py
|
有冲突 麻烦解决下 |
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/script_chainer/win_exe/script_runner.py`:
- Around line 500-510: The except block catching _NoLogTimeoutError incorrectly
uses break which exits the retry for-loop instead of starting the next retry;
change the control flow in that except block to use continue so that after
calling _cleanup_processes(script_config, pm) the outer retry loop proceeds to
the next attempt (preserving the attempt vs max_retries logic and keeping the
existing print_message() and return behavior unchanged).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93c00a09-0206-4404-a25e-4ac595afd668
📒 Files selected for processing (3)
src/script_chainer/config/script_config.pysrc/script_chainer/gui/page/script_setting_interface.pysrc/script_chainer/win_exe/script_runner.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/script_chainer/config/script_config.py
- 将静默超时时间范围从30~86400调整为1~86400,步进从30改为1 - 修复静默超时重试时break误用为continue,确保正确进入下一轮重试
合并完成,已本地验证,功能符合预期正确运行 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/script_chainer/win_exe/script_runner.py (1)
525-538: 💤 Low value可选:收窄异常捕获范围
except Exception会静默吞掉_run_python_script和_run_external_script_with_retries中任何未预期异常(Ruff BLE001)。虽然log.error(..., exc_info=True)确保了可观察性,且此处需要一定的宽容度以避免单脚本失败阻断整个脚本链,但可以考虑至少排除SystemExit之外的关键异常(如MemoryError),或在注释中明确说明捕获所有异常的意图,方便后续维护。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/script_chainer/win_exe/script_runner.py` around lines 525 - 538, The current broad except in _run_script_in_group silently swallows critical exceptions; update _run_script_in_group (the try/except around _run_python_script and _run_external_script_with_retries) to re-raise critical exceptions (e.g., SystemExit, KeyboardInterrupt, MemoryError) while only logging non-critical Exceptions, or explicitly document why all Exceptions are caught; implement this by checking the caught exception type and re-raising for the critical ones before calling log.error(..., exc_info=True).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/script_chainer/win_exe/script_runner.py`:
- Around line 525-538: The current broad except in _run_script_in_group silently
swallows critical exceptions; update _run_script_in_group (the try/except around
_run_python_script and _run_external_script_with_retries) to re-raise critical
exceptions (e.g., SystemExit, KeyboardInterrupt, MemoryError) while only logging
non-critical Exceptions, or explicitly document why all Exceptions are caught;
implement this by checking the caught exception type and re-raising for the
critical ones before calling log.error(..., exc_info=True).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1cd3fabc-d241-4238-8d4f-72b48bba5fb2
📒 Files selected for processing (3)
src/script_chainer/gui/page/script_setting_interface.pysrc/script_chainer/services/log_notifier.pysrc/script_chainer/win_exe/script_runner.py
Closes #44
当外部脚本在运行期间超过设定时间无任何 stdout 输出时,判定为游戏/脚本未响应,自动终止并重新执行,最多重试指定次数。该功能默认关闭(
no_log_timeout_seconds=0),对现有行为零影响。配置层(
ScriptConfig)新增两个字段:
no_log_timeout_seconds: int = 0— 静默超时阈值(秒),0 = 禁用no_log_max_retries: int = 3— 最大重启次数运行层(
script_runner.py)_make_stdout_callback新增last_log_time: list[float]参数,每次收到输出更新时间戳(单元素列表实现跨闭包可变引用)_monitor_script_done每秒末检测静默超时,超时抛出内部_NoLogTimeoutErrorrun_script改为重试循环,捕获_NoLogTimeoutError后执行_cleanup_processes再重启;达到上限时记录错误并退出GUI(
ScriptEditDialog)新增两个配置项:
运行测试
python src/script_chainer/win_exe/script_runner.py --chain 01Summary by CodeRabbit