fix(LogMonitor): 日志文件消失时监控不再静默死亡 - #709
Merged
Merged
Conversation
monitor_file 的循环体里，mtime 检查落在读取分支的 try 之外， exists() 与随后的 stat() 之间文件被删除或重建时， FileNotFoundError 直接逃逸、监控协程带异常终止。 调用方没有 done_callback，异常无人回收，日志监控静默失效， 该趟任务再也读不到日志，只能卡到超时或误判异常。 exists() 与紧随其后的 stat() 是同一件事的两次查询，且读取分支 本就会对文件不存在给出 FileNotFoundError 并 continue，删除该冗余 检查后把 mtime 块并入已有的读取分支 try，循环体只保留一个异常出口。 净减 7 行，不新增守卫。
审阅者指南（小型 PR 中折叠显示）审阅者指南通过删除独立的文件存在性查询，并将 mtime 检查纳入已有读取 try，实现单一异常出口；文件在检查与 stat 之间被删除或重建时，监控会按既有重试流程继续运行而不会静默终止，同时补充变更日志和版本信息。 具备容错能力的日志文件监控时序图sequenceDiagram
participant Monitor as LogMonitor
participant File as LogFile
participant Callback as do_callback
loop monitor_file
Monitor->>File: stat()
alt file exists and mtime is today
Monitor->>File: read log contents
Monitor->>Monitor: process log updates
else FileNotFoundError during stat or read
Monitor->>Callback: do_callback()
Monitor->>Monitor: sleep(1)
Monitor->>Monitor: continue monitoring
else mtime is not today
Monitor->>Callback: do_callback()
Monitor->>Monitor: sleep(1)
end
end
文件级变更
可能相关的问题
提示和命令与 Sourcery 交互
自定义你的使用体验访问你的控制面板：
获取帮助Original review guide in EnglishReviewer's guide (collapsed on small PRs)Reviewer's Guide通过删除独立的文件存在性查询，并将 mtime 检查纳入已有读取 try，实现单一异常出口；文件在检查与 stat 之间被删除或重建时，监控会按既有重试流程继续运行而不会静默终止，同时补充变更日志和版本信息。 Sequence diagram for resilient log file monitoringsequenceDiagram
participant Monitor as LogMonitor
participant File as LogFile
participant Callback as do_callback
loop monitor_file
Monitor->>File: stat()
alt file exists and mtime is today
Monitor->>File: read log contents
Monitor->>Monitor: process log updates
else FileNotFoundError during stat or read
Monitor->>Callback: do_callback()
Monitor->>Monitor: sleep(1)
Monitor->>Monitor: continue monitoring
else mtime is not today
Monitor->>Callback: do_callback()
Monitor->>Monitor: sleep(1)
end
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嗨——我发现了 1 个问题
面向 AI Agent 的提示
请处理此次代码审查中的评论：
## 具体评论
### 评论 1
<location path="app/utils/LogMonitor.py" line_range="173-183" />
<code_context>
- continue
-
- if not if_mtime_checked:
- file_mtime_date = date.fromtimestamp(current_path.stat().st_mtime)
- if file_mtime_date == date.today():
- log_stat = current_path.stat()
- if_mtime_checked = True
- else:
- if warned_mtime_date != file_mtime_date:
- logger.warning(f"日志文件今天未被修改: {file_mtime_date}")
- warned_mtime_date = file_mtime_date
- await self.do_callback()
- await asyncio.sleep(1)
- continue
-
# 尝试读取文件
</code_context>
<issue_to_address>
**issue (broader_impact):** 当初始 `stat()` 执行前日志路径不存在时，新的外层异常处理路径只会记录日志并休眠五秒；它不再像已移除的 `exists()` 分支那样调用 `do_callback()`。`do_callback()` 会更新 `last_callback_time` 和 `last_callback_at`，因此在文件重新创建期间，任务会失去文件缺失状态下的心跳，并可能被视为停滞或发生超时。
**触发条件：** 受监控的文件被删除，或尚未创建时。
**建议修复：** 在 `FileNotFoundError` 处理程序中保留文件缺失时的回调语义，或者在休眠前以其他方式更新监控器的心跳。
</issue_to_address>Sourcery 评估
等待批准。 请先处理 1 个发现的问题。
阻塞性发现：app/utils/LogMonitor.py:183
Original comment in English
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="app/utils/LogMonitor.py" line_range="173-183" />
<code_context>
- continue
-
- if not if_mtime_checked:
- file_mtime_date = date.fromtimestamp(current_path.stat().st_mtime)
- if file_mtime_date == date.today():
- log_stat = current_path.stat()
- if_mtime_checked = True
- else:
- if warned_mtime_date != file_mtime_date:
- logger.warning(f"日志文件今天未被修改: {file_mtime_date}")
- warned_mtime_date = file_mtime_date
- await self.do_callback()
- await asyncio.sleep(1)
- continue
-
# 尝试读取文件
</code_context>
<issue_to_address>
**issue (broader_impact):** When the log path is absent before the initial `stat()`, the new outer exception path only logs and sleeps for five seconds; it no longer calls `do_callback()` as the removed `exists()` branch did. `do_callback()` updates `last_callback_time` and `last_callback_at`, so the task loses its missing-file heartbeat and can be treated as stalled or time out while the file is being recreated.
**Triggers:** When the monitored file is deleted or has not yet been created.
**Suggested fix:** Preserve the missing-file callback semantics in the `FileNotFoundError` handler, or otherwise update the monitor heartbeat before sleeping.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: app/utils/LogMonitor.py:183
Comment on lines
+173
to
+183
| file_mtime_date = date.fromtimestamp(current_path.stat().st_mtime) | ||
| if file_mtime_date == date.today(): | ||
| log_stat = current_path.stat() | ||
| if_mtime_checked = True | ||
| else: | ||
| if warned_mtime_date != file_mtime_date: | ||
| logger.warning(f"日志文件今天未被修改: {file_mtime_date}") | ||
| warned_mtime_date = file_mtime_date | ||
| await self.do_callback() | ||
| await asyncio.sleep(1) | ||
| continue |
There was a problem hiding this comment.
issue (broader_impact): 当初始 stat() 执行前日志路径不存在时，新的外层异常处理路径只会记录日志并休眠五秒；它不再像已移除的 exists() 分支那样调用 do_callback()。do_callback() 会更新 last_callback_time 和 last_callback_at，因此在文件重新创建期间，任务会失去文件缺失状态下的心跳，并可能被视为停滞或发生超时。
触发条件： 受监控的文件被删除，或尚未创建时。
建议修复： 在 FileNotFoundError 处理程序中保留文件缺失时的回调语义，或者在休眠前以其他方式更新监控器的心跳。
Original comment in English
issue (broader_impact): When the log path is absent before the initial stat(), the new outer exception path only logs and sleeps for five seconds; it no longer calls do_callback() as the removed exists() branch did. do_callback() updates last_callback_time and last_callback_at, so the task loses its missing-file heartbeat and can be treated as stalled or time out while the file is being recreated.
Triggers: When the monitored file is deleted or has not yet been created.
Suggested fix: Preserve the missing-file callback semantics in the FileNotFoundError handler, or otherwise update the monitor heartbeat before sleeping.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
$monitor_file 的循环体里，mtime 检查落在读取分支的 try 之外。文件在 exists() 与随后的 stat() 之间被删除或重建时，FileNotFoundError 直接逃逸、监控协程带异常终止；调用方没有 done_callback，异常无人回收，日志监控静默失效，该趟任务再也读不到日志，只能卡到超时或误判异常。
exists() 与紧随其后的 stat() 是同一件事的两次查询，且读取分支本就对文件不存在给出 FileNotFoundError 并 continue。删除该冗余检查后，把 mtime 块并入已有的读取分支 try，循环体只保留一个异常出口。净减 7 行，不新增守卫。
替换 #670：该 PR 走的是「给 stat 补 try」的加守卫路线，本 PR 直接消除冗余检查并复用已有异常出口。
Closes #666
Summary by Sourcery
Bug Fixes:
Original summary in English
Summary by Sourcery
Bug Fixes: