Conversation
Walkthrough将服务器日志缓存重构为原子快照（包含内容与元数据）、新增按区间磁盘读取，按文件大小选择内存缓存或磁盘读取；客户端统一在请求 变更详情
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Browser / Tauri)
participant Server as Server (/api/logs)
participant Disk as Disk I/O
Client->>Server: GET /api/logs (X-Log-Offset: N 或 缺省)
alt X-Log-Offset 缺省
Server->>Disk: open & stream full file
Disk-->>Server: stream bytes
Server-->>Client: 200 + Content-Disposition: attachment (stream)
else X-Log-Offset 存在
Server->>Disk: stat(file) -> (size, mtime_ns)
Server-->>Server: 构建/刷新 LogCacheSnapshot（原子 CAS）
alt snapshot.content != null (<=4MB)
Server-->>Client: 200 + X-Log-Size/X-Log-Range + body (snapshot 子串)
else snapshot.content == null (>4MB)
Server->>Disk: read_file_range(path, offset, length) 或 read_file_tail(MAX)
Disk-->>Server: bytes
Server-->>Client: 200 + X-Log-Size/X-Log-Range + body (磁盘读取)
end
end
代码审查工作量估计🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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_assets/common/assets/web/components/LogsSection.vue`:
- Around line 130-164: The current code fetches the full log into JS memory via
fetch('/api/logs') then response.text() for all branches, causing large-memory
buffering; fix by only reading the response body for the Tauri branch.
Concretely: detect window.__TAURI_INTERNALS__ first and if present call
fetch('/api/logs') then await response.text() and invoke
window.__TAURI_INTERNALS__.invoke('save_text_file', { content, defaultName:
filename, ... }); for the standard browser branch avoid response.text()/Blob
creation and instead create an anchor with href set directly to '/api/logs' (or
'/api/logs?download=1' if needed) and download: filename, append/click/remove
the link so the browser streams the file without loading it into JS memory; keep
error handling around save_text_file and the overall try/catch.
In `@src/confighttp.cpp`:
- Around line 658-667: The code currently treats a nullptr from
read_file_tail/read_file_range as an empty log and proceeds to write a 200 with
an empty body; instead, detect when new_content is nullptr after calling
read_file_tail (and similarly for read_file_range in the other block) and
immediately return a 5xx error via response->write (e.g.,
SimpleWeb::StatusCode::server_error_internal_server_error with a clear message
like "Failed to read log file") rather than creating an empty shared_ptr or
continuing to set X-Log-Size; update both the new_content handling around
read_file_tail and the analogous handling around read_file_range so I/O failures
propagate as server errors.
- Around line 600-615: The code currently calls
std::filesystem::file_size(log_path) then streams an open std::ifstream "in" to
response->write with a fixed "Content-Length" header, which creates a TOCTOU bug
for a growing/rotating log; remove the headers.emplace("Content-Length", ...)
line (and any use of fsize) so the response is sent chunked, or alternatively
implement a stable snapshot by copying the log to a temp file or reading it
fully into a buffer and then set a correct Content-Length from that snapshot
before calling response->write; update confighttp.cpp around the std::ifstream
"in" / response->write usage accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f8c5bdab-0595-406a-8f2d-0ae570bc8616
📒 Files selected for processing (4)
src/confighttp.cppsrc_assets/common/assets/web/components/LogsSection.vuesrc_assets/common/assets/web/composables/useLogs.jssrc_assets/common/assets/web/composables/useTroubleshooting.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
src_assets/**/*.{vue,js,html}
⚙️ CodeRabbit configuration file
src_assets/**/*.{vue,js,html}: 基于 Vue.js 的 Web 配置面板。审查 XSS/CSRF 安全性、 组件设计、状态管理和可访问性。
Files:
src_assets/common/assets/web/composables/useLogs.jssrc_assets/common/assets/web/composables/useTroubleshooting.jssrc_assets/common/assets/web/components/LogsSection.vue
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/confighttp.cpp
🔇 Additional comments (1)
src/confighttp.cpp (1)
624-628: C++ 标准已是 C++23，std::atomic<std::shared_ptr<>>完全支持。项目 CMake 配置明确设置了
CXX_STANDARD 23（见cmake/targets/common.cmake），而std::atomic<std::shared_ptr<T>>的特化在 C++20 中已标准化，C++23 完全支持该特性。代码无需改动。> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/confighttp.cpp (1)
654-659:⚠️ Potential issue | 🟠 Major不要把读取失败伪装成空日志。
read_file_tail()返回nullptr后，这里仍然发布content=""的快照并继续走成功分支。前端会把现有日志清空并把 offset 推进到current_size，真实的 I/O 故障会被误判成“日志已空”。这里应该直接返回 5xx，而不是构造空快照。💡 可参考的改法
- if (!new_content) { - new_content = std::make_shared<const std::string>(); - } + if (!new_content) { + response->write(SimpleWeb::StatusCode::server_error_internal_server_error, "Failed to read log file"); + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/confighttp.cpp` around lines 654 - 659, The code masks I/O failures by treating read_file_tail(log_path, MAX_LOG_CACHE_SIZE) returning nullptr as an empty log: when new_content is nullptr the handler currently creates an empty shared_ptr and proceeds; instead, detect the nullptr from read_file_tail in the request handling path (the block around read_file_tail / new_content) and return an HTTP 5xx error response (e.g., 500) immediately, logging the underlying I/O error, so you do not produce an empty snapshot or advance offsets; remove or bypass the std::make_shared<const std::string>() fallback and ensure the client receives a failure status rather than an empty content.
🤖 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/confighttp.cpp`:
- Around line 535-552: read_file_tail currently seeks from the live file end,
causing a race where size/mtime were sampled earlier (snapshot->file_size /
X-Log-Size) but the read can include new appended bytes and make current_size -
content->size() underflow; change the logic to read a fixed region based on the
sampled size/offset instead of re-reading from the current EOF: update
read_file_tail (or add an overload) to accept an explicit offset and length and
read exactly that range (compute offset = sampled_file_size - sampled_read_size)
so the body matches the previously recorded snapshot->file_size and the
X-Log-Size header, and ensure callers that use current_size - content->size()
keep using the sampled values rather than the live EOF.
- Around line 640-675: The current refresh logic unconditionally calls
log_cache.store(new_snap) which lets a slower thread overwrite a newer snapshot
and cause stale file_size/mtime to be used; change this to a CAS-style publish:
build new_snap, then attempt to atomically replace the current snapshot only if
the stored pointer still equals the earlier loaded snapshot (use
std::atomic_compare_exchange_* on log_cache or compare_exchange_strong on the
underlying atomic<std::shared_ptr<LogCacheSnapshot>>); if the CAS fails, discard
new_snap and reuse the already stored snapshot (do not call store), ensuring
subsequent logic (304 checks, try_incremental_log_read, start_offset) uses the
up-to-date snapshot.
---
Duplicate comments:
In `@src/confighttp.cpp`:
- Around line 654-659: The code masks I/O failures by treating
read_file_tail(log_path, MAX_LOG_CACHE_SIZE) returning nullptr as an empty log:
when new_content is nullptr the handler currently creates an empty shared_ptr
and proceeds; instead, detect the nullptr from read_file_tail in the request
handling path (the block around read_file_tail / new_content) and return an HTTP
5xx error response (e.g., 500) immediately, logging the underlying I/O error, so
you do not produce an empty snapshot or advance offsets; remove or bypass the
std::make_shared<const std::string>() fallback and ensure the client receives a
failure status rather than an empty content.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8becf0fa-88c2-4126-893d-b03055d1d66c
📒 Files selected for processing (2)
src/confighttp.cppsrc_assets/common/assets/web/components/LogsSection.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- src_assets/common/assets/web/components/LogsSection.vue
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/confighttp.cpp
🔇 Additional comments (1)
src/confighttp.cpp (1)
597-612: 下载分支去掉Content-Length这个改动是对的。热日志文件会持续增长，这里改成直接流式输出能避免响应长度和实际内容不一致的问题。
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/confighttp.cpp (1)
634-640:⚠️ Potential issue | 🟠 Major缓存模式下 I/O 失败时仍可能返回 200 + 空内容。
当
read_file_range返回nullptr（I/O 失败）时，代码将new_content设为空字符串，然后仅在文件不存在时返回错误。这意味着如果文件存在但因权限问题或被锁定而无法读取，会以 200 状态码返回空内容，可能导致前端误清空日志显示。对比偏移量模式（第 718-721 行）正确地在读取失败时返回 500 错误。建议在此处保持一致的错误处理逻辑。
,💡 建议改法
if (!new_content) { - new_content = std::make_shared<const std::string>(); - } - if (new_content->empty() && !std::filesystem::exists(log_path, ec)) { - response->write(SimpleWeb::StatusCode::server_error_internal_server_error, "Log file not available"); - return; + if (current_size > 0) { + response->write(SimpleWeb::StatusCode::server_error_internal_server_error, "Failed to read log file"); + return; + } + new_content = std::make_shared<const std::string>(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/confighttp.cpp` around lines 634 - 640, When read_file_range fails and returns nullptr the code currently replaces new_content with an empty string and only returns 500 when the file does not exist; instead detect the read failure (new_content == nullptr from read_file_range) and return a server_error_internal_server_error via response->write just like the offset mode handling (see the branch around lines 718-721). Concretely, do not overwrite a nullptr with an empty shared string for new_content; if new_content is nullptr, call response->write(SimpleWeb::StatusCode::server_error_internal_server_error, "Log file not available") and return, otherwise continue with the existing empty-file/file-exists check that uses log_path and ec.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/confighttp.cpp`:
- Around line 634-640: When read_file_range fails and returns nullptr the code
currently replaces new_content with an empty string and only returns 500 when
the file does not exist; instead detect the read failure (new_content == nullptr
from read_file_range) and return a server_error_internal_server_error via
response->write just like the offset mode handling (see the branch around lines
718-721). Concretely, do not overwrite a nullptr with an empty shared string for
new_content; if new_content is nullptr, call
response->write(SimpleWeb::StatusCode::server_error_internal_server_error, "Log
file not available") and return, otherwise continue with the existing
empty-file/file-exists check that uses log_path and ec.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 005481f2-1efc-4bee-b167-702cf47829db
📒 Files selected for processing (1)
src/confighttp.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/confighttp.cpp
🔇 Additional comments (5)
src/confighttp.cpp (5)
500-508: LGTM!快照结构设计合理，使用
shared_ptr<const std::string>确保内容不可变，适合原子发布模式。在偏移量模式下content为nullptr的设计也很清晰。
534-549: LGTM!
read_file_range实现简洁，错误处理正确（失败返回nullptr），ifstream使用 RAII 模式自动管理资源。
576-590: LGTM!下载模式正确省略了
Content-Length，避免了对活跃写入日志文件的 TOCTOU 风险，让响应走分块传输。
650-657: LGTM!CAS 发布模式正确实现，避免了并发线程用旧快照覆盖新快照的问题。
compare_exchange_strong失败时会自动更新snapshot为当前已发布的值，确保后续逻辑使用最新数据。
688-724: LGTM!响应处理逻辑清晰：
- 缓存模式正确计算增量偏移并返回差异内容
- 偏移量模式使用
snapshot->file_size界定读取范围，避免 TOCTOU 问题- 偏移量模式下读取失败正确返回 500 错误（第 718-721 行）
边界检查和模式切换逻辑完善。
… for display reinitialization
Yundi339
changed the title
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.
各场景与新设计的交互（偏移量模式）