-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: webui log output incompletely #4029
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
2 tasks
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
common.js中新的processStream逻辑不再处理部分 JSON 片段,而是假设每个data:块都是完整的 JSON 字符串。这在 SSE 帧跨多次网络读取被拆分时可能出问题;建议保留或重新引入对不完整行进行增量缓冲处理的方案。- 在
ConsoleDisplayer.vue和common.js中新增了许多看起来像调试输出的console.log语句;建议删除这些语句,或通过调试标志进行控制,以避免在生产环境中产生过于噪声的日志。 - 在 store 中使用
crypto.randomUUID()假定运行环境一定提供该 API;建议增加一个小的降级方案(例如基于Math.random的 ID)或进行特性检测,以避免在旧浏览器中出现运行时错误。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- `common.js` 中新的 `processStream` 逻辑不再处理部分 JSON 片段,而是假设每个 `data:` 块都是完整的 JSON 字符串。这在 SSE 帧跨多次网络读取被拆分时可能出问题;建议保留或重新引入对不完整行进行增量缓冲处理的方案。
- 在 `ConsoleDisplayer.vue` 和 `common.js` 中新增了许多看起来像调试输出的 `console.log` 语句;建议删除这些语句,或通过调试标志进行控制,以避免在生产环境中产生过于噪声的日志。
- 在 store 中使用 `crypto.randomUUID()` 假定运行环境一定提供该 API;建议增加一个小的降级方案(例如基于 `Math.random` 的 ID)或进行特性检测,以避免在旧浏览器中出现运行时错误。
## Individual Comments
### Comment 1
<location> `dashboard/src/components/shared/ConsoleDisplayer.vue:49` </location>
<code_context>
'CRITICAL': 'purple'
- }
+ },
+ lastLogLength: 0, // 记录上次处理的日志数量
+ }
+ },
</code_context>
<issue_to_address>
**issue:** 处理日志缓存缩小的情况,避免新日志被静默忽略。
由于 watcher 只在 `newVal.length > this.lastLogLength` 时处理日志,而在 `common.js` 中通过 `splice(0, ...)` 截断缓存时,`lastLogLength` 可能保持大于当前数组长度。截断之后(例如从 200 → 100),即使后续追加了新日志,也可能永远不满足 `>` 条件,从而不会被处理。请在缓存缩小时相应调整 `lastLogLength`(例如将其钳制为 `newVal.length`),或者基于稳定标识符(例如单调递增的 ID/uuid)而不是数组长度来计算切片偏移。
</issue_to_address>
### Comment 2
<location> `dashboard/src/components/shared/ConsoleDisplayer.vue:120-127` </location>
<code_context>
+ refreshDisplay() {
</code_context>
<issue_to_address>
**question (bug_risk):** 渲染日志时已不再应用历史记录限制 (`historyNum`)。
之前 `init()` 会通过只打印最后 N 条记录(当 `historyNum_ != -1` 时)来强制执行 `historyNum_`。现在 `refreshDisplay()` 会显示所有匹配的日志并忽略 `historyNum_`,改变了历史过滤器的行为。如果这不是有意为之,请在 `refreshDisplay()` 中应用历史限制,使实际展示的条数仍然受配置的历史设置限制。
</issue_to_address>
### Comment 3
<location> `dashboard/src/components/shared/ConsoleDisplayer.vue:74-80` </location>
<code_context>
- this.printLog(lastLog.data);
+ handler(newVal) {
+ // 只处理新增的日志
+ console.log('logCache changed, length:', newVal?.length, 'lastLength:', this.lastLogLength);
+ if (newVal && newVal.length > this.lastLogLength) {
+ const newLogs = newVal.slice(this.lastLogLength);
+ console.log('Processing new logs:', newLogs);
+
+ newLogs.forEach(logItem => {
+ console.log('Log item:', logItem, 'Level selected:', this.isLevelSelected(logItem.level));
+ if (this.isLevelSelected(logItem.level)) {
+ this.printLog(logItem.data);
</code_context>
<issue_to_address>
**suggestion (performance):** 考虑移除或加以控制 watcher 中大量的 `console.log` 语句。
这些日志会在每次更新时触发,对于高吞吐量的日志流来说既噪声又影响性能。建议删除这些日志,或通过仅在开发环境启用的标志或环境检查来控制它们。
</issue_to_address>
### Comment 4
<location> `dashboard/src/stores/common.js:74-76` </location>
<code_context>
- this.log_cache.push(data_json);
- if (this.log_cache.length > this.log_cache_max_len) {
- this.log_cache.shift();
+ if (line.startsWith('data: ')) {
+ const logLine = line.replace('data: ', '').trim();
+ if (logLine) {
+ let logObject = JSON.parse(logLine);
+ // give a uuid if not exists
</code_context>
<issue_to_address>
**issue:** 放宽对 `data:` 前缀的检查,以支持没有尾随空格的 SSE 负载。
新的 `line.startsWith('data: ')` 检查会跳过只使用 `data:`(无空格)的 SSE 消息,而这种格式是允许且常见的。请使用更宽松的前缀匹配(例如 `startsWith('data:')`),然后在移除前缀后再进行 `trim`,以便同时处理这两种格式。
</issue_to_address>
### Comment 5
<location> `dashboard/src/stores/common.js:76-77` </location>
<code_context>
- this.log_cache.shift();
+ if (line.startsWith('data: ')) {
+ const logLine = line.replace('data: ', '').trim();
+ if (logLine) {
+ let logObject = JSON.parse(logLine);
+ // give a uuid if not exists
+ if (!logObject.uuid) {
</code_context>
<issue_to_address>
**issue (bug_risk):** 恢复对流式 JSON 的防御性解析,以避免中断 SSE 循环。
之前我们会对部分行做缓冲,并将 `JSON.parse` 包在 `try/catch` 中,从而保证分块的或格式不正确的 JSON 不会破坏流式传输。现在直接调用 `JSON.parse`。如果一个 JSON 对象被拆分到多个数据块,或者出现格式错误的行,就会抛出异常,很可能导致 `processStream` 终止,从而中断日志流。请重新引入缓冲和/或解析错误处理,使流在遇到坏输入时仍能安全继续。
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进 Review 质量。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- The new
processStreamlogic incommon.jsno longer handles partial JSON chunks and assumes eachdata:block is a complete JSON string, which may break when SSE frames are split across network reads; consider retaining or reintroducing an incremental buffer approach for incomplete lines. - There are many
console.logstatements added inConsoleDisplayer.vueandcommon.jsthat look like debugging output; consider removing or gating them behind a debug flag to avoid noisy logs in production. - The use of
crypto.randomUUID()in the store assumes availability of this API in the runtime environment; consider adding a small fallback (e.g., toMath.random-based IDs) or a feature check to avoid runtime errors in older browsers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `processStream` logic in `common.js` no longer handles partial JSON chunks and assumes each `data:` block is a complete JSON string, which may break when SSE frames are split across network reads; consider retaining or reintroducing an incremental buffer approach for incomplete lines.
- There are many `console.log` statements added in `ConsoleDisplayer.vue` and `common.js` that look like debugging output; consider removing or gating them behind a debug flag to avoid noisy logs in production.
- The use of `crypto.randomUUID()` in the store assumes availability of this API in the runtime environment; consider adding a small fallback (e.g., to `Math.random`-based IDs) or a feature check to avoid runtime errors in older browsers.
## Individual Comments
### Comment 1
<location> `dashboard/src/components/shared/ConsoleDisplayer.vue:49` </location>
<code_context>
'CRITICAL': 'purple'
- }
+ },
+ lastLogLength: 0, // 记录上次处理的日志数量
+ }
+ },
</code_context>
<issue_to_address>
**issue:** Handle cases where the log cache shrinks so new logs are not silently ignored.
Because the watcher only processes logs when `newVal.length > this.lastLogLength`, truncating the cache in `common.js` with `splice(0, ...)` can cause `lastLogLength` to stay larger than the current array length. After a shrink (e.g., 200 → 100), new logs can then be appended without ever satisfying the `>` condition, so they’re never processed. Please adjust `lastLogLength` when the cache shrinks (e.g., clamp it to `newVal.length`) or base the slice offset on a stable identifier (e.g., monotonic ID/uuid) instead of array length alone.
</issue_to_address>
### Comment 2
<location> `dashboard/src/components/shared/ConsoleDisplayer.vue:120-127` </location>
<code_context>
+ refreshDisplay() {
</code_context>
<issue_to_address>
**question (bug_risk):** The history limit (`historyNum`) is no longer applied when rendering logs.
Previously, `init()` enforced `historyNum_` by printing only the last N entries (when `historyNum_ != -1`). `refreshDisplay()` now shows all matching logs and ignores `historyNum_`, changing the behavior of the history filter. If this was not intentional, please apply the history limit in `refreshDisplay()` so the number of displayed entries is still bounded by the configured history setting.
</issue_to_address>
### Comment 3
<location> `dashboard/src/components/shared/ConsoleDisplayer.vue:74-80` </location>
<code_context>
- this.printLog(lastLog.data);
+ handler(newVal) {
+ // 只处理新增的日志
+ console.log('logCache changed, length:', newVal?.length, 'lastLength:', this.lastLogLength);
+ if (newVal && newVal.length > this.lastLogLength) {
+ const newLogs = newVal.slice(this.lastLogLength);
+ console.log('Processing new logs:', newLogs);
+
+ newLogs.forEach(logItem => {
+ console.log('Log item:', logItem, 'Level selected:', this.isLevelSelected(logItem.level));
+ if (this.isLevelSelected(logItem.level)) {
+ this.printLog(logItem.data);
</code_context>
<issue_to_address>
**suggestion (performance):** Consider removing or gating verbose `console.log` statements in the watcher.
These logs fire on every update and can be noisy and slow for high-volume streams. Consider removing them or guarding them with a dev-only flag or environment check.
</issue_to_address>
### Comment 4
<location> `dashboard/src/stores/common.js:74-76` </location>
<code_context>
- this.log_cache.push(data_json);
- if (this.log_cache.length > this.log_cache_max_len) {
- this.log_cache.shift();
+ if (line.startsWith('data: ')) {
+ const logLine = line.replace('data: ', '').trim();
+ if (logLine) {
+ let logObject = JSON.parse(logLine);
+ // give a uuid if not exists
</code_context>
<issue_to_address>
**issue:** Relax the `data:` prefix check to handle SSE payloads without a trailing space.
The new `line.startsWith('data: ')` check will skip SSE messages that use `data:` without a space, which is allowed and common. Please use a more permissive prefix match (e.g., `startsWith('data:')`) and then trim after removing the prefix so both formats are handled.
</issue_to_address>
### Comment 5
<location> `dashboard/src/stores/common.js:76-77` </location>
<code_context>
- this.log_cache.shift();
+ if (line.startsWith('data: ')) {
+ const logLine = line.replace('data: ', '').trim();
+ if (logLine) {
+ let logObject = JSON.parse(logLine);
+ // give a uuid if not exists
+ if (!logObject.uuid) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Restore defensive parsing for streamed JSON to avoid breaking the SSE loop.
Previously we buffered partial lines and wrapped `JSON.parse` in `try/catch` so chunked or malformed JSON wouldn’t break streaming. Now `JSON.parse` is called directly. If a JSON object is split across chunks or a malformed line appears, this will throw and likely abort `processStream`, interrupting log streaming. Please reintroduce buffering and/or parse error handling so the stream can safely continue on bad input.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
fixes: #3903 #3769
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
改进 WebUI 控制台日志流式处理,使日志能够增量追加、在组件挂载时渲染所有历史条目,并简化日志解析与缓存处理。
Bug 修复:
改进:
data:作为前缀的完整 JSON 行,并在缺失时为日志对象分配 UUID。Original summary in English
Summary by Sourcery
Improve WebUI console log streaming so logs are incrementally appended and all historical entries are rendered on mount while simplifying log parsing and cache handling.
Bug Fixes:
Enhancements:
data:and assigning UUIDs to log objects when missing.