Open
Conversation
Contributor
演练此 PR 引入一个新的 HTTP 日志服务器，用于通过浏览器实时查看应用日志和游戏截图。包括新的服务类实现、配置选项、UI 绑定和国际化支持，完整集成到依赖注入容器中。 变更
序列图sequenceDiagram
participant Browser as 浏览器客户端
participant Server as HttpLogServerService
participant Config as ConfigService
participant Sink as ILogEventSink
participant Capture as GameCapture
Browser->>Server: GET /
Server-->>Browser: 返回嵌入式 HTML UI
Note over Server: 应用启动
Config->>Server: 订阅 HttpLogServer 配置变更
Server->>Server: StartServer()
Sink->>Server: Emit(logEvent)
Server->>Server: 格式化并序列化日志为 JSON
Server->>Server: 缓冲日志（最大 N 条）
Browser->>Server: GET /logs (SSE)
Server->>Server: ServeLogsSseAsync()
Server->>Browser: 重放缓冲日志
Server-->>Browser: 持续推送新日志事件
Browser->>Server: GET /screenshot
Server->>Capture: CaptureGameScreenshot()
Capture-->>Server: 返回截图 JPEG
Server-->>Browser: 返回图像数据
Config->>Server: 配置变更（地址/端口）
Server->>Server: RestartServer()
代码审查工作量评估🎯 4 (复杂) | ⏱️ ~45 分钟 诗
🚥 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 unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
BetterGenshinImpact/Service/HttpLogServerService.cs (1)
126-133: 停止服务时应无条件释放_listener与_cts。当前仅在
IsListening为 true 时关闭 listener，启动失败路径可能遗漏释放；_cts也未Dispose。建议统一清理。♻️ 建议清理逻辑
private void StopServer() { _cts?.Cancel(); - if (_listener != null && _listener.IsListening) + _cts?.Dispose(); + if (_listener != null) { - _listener.Stop(); + try + { + if (_listener.IsListening) _listener.Stop(); + } + catch { } _listener.Close(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BetterGenshinImpact/Service/HttpLogServerService.cs` around lines 126 - 133, StopServer currently only stops and closes _listener when _listener.IsListening is true and only cancels _cts without disposing it, which can leak resources if startup failed; update StopServer to unconditionally Cancel and Dispose _cts (call _cts?.Cancel(); _cts?.Dispose();) and to unconditionally Close/Stop/Dispose the listener regardless of IsListening (check _listener != null then call _listener.Stop(); _listener.Close(); and set or dispose as appropriate), ensuring _listener and _cts are always released; refer to the StopServer method and the fields _listener and _cts when making the changes.BetterGenshinImpact/View/Pages/CommonSettingsPage.xaml (1)
500-504: 建议在监听地址说明中补充安全提示。当前文案强调“允许所有设备访问”，但该功能会暴露实时日志与截图。建议在 Line 503 增加“仅建议可信局域网使用”的提示，降低误暴露风险。
✏️ 建议文案调整
- Text="默认 0.0.0.0 (允许所有设备访问)" + Text="默认 0.0.0.0（仅建议在可信局域网使用）"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BetterGenshinImpact/View/Pages/CommonSettingsPage.xaml` around lines 500 - 504, Update the UI text to include a security reminder that binding the listener to 0.0.0.0 exposes real-time logs and screenshots and should only be used on trusted LANs: locate the ui:TextBlock showing "默认 0.0.0.0 (允许所有设备访问)" and either append a short caution like "仅建议在可信局域网中使用" to its Text or add a nearby smaller/tertiary ui:TextBlock (same Grid row/column or Grid.Row="2") with that warning; ensure Foreground uses ThemeResource TextFillColorTertiaryBrush and TextWrapping="Wrap" so styling and layout match existing elements.BetterGenshinImpact/Core/Config/OtherConfig.cs (1)
42-46: 建议在配置模型层增加端口与地址归一化校验。当前
Port和ListenAddress没有模型级约束，手工改配置文件时容易落入非法值，问题会延迟到运行期。建议在该配置类里直接做 clamp/trim。🛠️ 建议补充校验
public partial class HttpLogServer : ObservableObject { [ObservableProperty] private bool _enabled = false; [ObservableProperty] private int _port = 8080; [ObservableProperty] private string _listenAddress = "0.0.0.0"; + + partial void OnPortChanged(int value) + { + if (value is < 1 or > 65535) + { + Port = Math.Clamp(value, 1, 65535); + } + } + + partial void OnListenAddressChanged(string value) + { + ListenAddress = string.IsNullOrWhiteSpace(value) ? "0.0.0.0" : value.Trim(); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BetterGenshinImpact/Core/Config/OtherConfig.cs` around lines 42 - 46, The Port and ListenAddress fields lack model-level normalization; implement normalization using the ObservableProperty partial-change hooks so values are clamped/cleaned immediately when set: add partial void OnPortChanged(int value) in OtherConfig to clamp to a valid TCP port range (1–65535, default to 8080 if out of range) and add partial void OnListenAddressChanged(string value) to Trim() the string, replace empty/whitespace with "0.0.0.0", and optionally perform a simple format check (fallback to default on failure) so invalid values are rejected or normalized at assignment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@BetterGenshinImpact/Service/HttpLogServerService.cs`:
- Around line 75-91: BroadcastLog is doing synchronous Write/Flush to each SSE
client (client.OutputStream.Write/Flush) which can block the logging thread;
change it to perform non-blocking background sends using asynchronous APIs
(e.g., WriteAsync/FlushAsync) and avoid iterating _clients.Keys directly by
snapshotting the client list; run each send in a Task with a
CancellationToken/timeout and remove the client via _clients.TryRemove(client,
out _) on exception or timeout to implement eviction. Locate BroadcastLog and
replace sync calls to client.OutputStream.Write/Flush with asynchronous send
tasks, apply a per-client timeout/ cancellation policy, and ensure failed or
slow clients are removed from _clients.
- Around line 168-186: The endpoints ServeLogsSseAsync and ServeScreenshot are
exposed without auth inside HandleRequestAsync; add an access check early in
HandleRequestAsync (e.g., call a new ValidateAccess(HttpListenerRequest) helper)
that verifies a configured bearer token from a header or query parameter and/or
checks context.Request.RemoteEndPoint against an IP whitelist, and return a
401/403 response if validation fails; update calls to ServeLogsSseAsync and
ServeScreenshot to only execute after ValidateAccess passes and keep ServeHtml
publicly accessible or similarly protect it if desired.
- Around line 94-123: In StartServer(), the call to _listener.Prefixes.Add is
currently outside the try/catch so any exceptions escape your handler; move the
Prefixes.Add call into the same try block that calls _listener.Start() and
Task.Run(()=>ListenAsync(...)) and perform port validation (e.g., ensure
config.Port is within valid TCP port range) before constructing the prefix, so
any prefix/port-related exceptions are caught by the existing catch; update
references to prefix/address construction and keep ListenAsync, _listener.Start
and the catch block unchanged.
---
Nitpick comments:
In `@BetterGenshinImpact/Core/Config/OtherConfig.cs`:
- Around line 42-46: The Port and ListenAddress fields lack model-level
normalization; implement normalization using the ObservableProperty
partial-change hooks so values are clamped/cleaned immediately when set: add
partial void OnPortChanged(int value) in OtherConfig to clamp to a valid TCP
port range (1–65535, default to 8080 if out of range) and add partial void
OnListenAddressChanged(string value) to Trim() the string, replace
empty/whitespace with "0.0.0.0", and optionally perform a simple format check
(fallback to default on failure) so invalid values are rejected or normalized at
assignment.
In `@BetterGenshinImpact/Service/HttpLogServerService.cs`:
- Around line 126-133: StopServer currently only stops and closes _listener when
_listener.IsListening is true and only cancels _cts without disposing it, which
can leak resources if startup failed; update StopServer to unconditionally
Cancel and Dispose _cts (call _cts?.Cancel(); _cts?.Dispose();) and to
unconditionally Close/Stop/Dispose the listener regardless of IsListening (check
_listener != null then call _listener.Stop(); _listener.Close(); and set or
dispose as appropriate), ensuring _listener and _cts are always released; refer
to the StopServer method and the fields _listener and _cts when making the
changes.
In `@BetterGenshinImpact/View/Pages/CommonSettingsPage.xaml`:
- Around line 500-504: Update the UI text to include a security reminder that
binding the listener to 0.0.0.0 exposes real-time logs and screenshots and
should only be used on trusted LANs: locate the ui:TextBlock showing "默认 0.0.0.0
(允许所有设备访问)" and either append a short caution like "仅建议在可信局域网中使用" to its Text or
add a nearby smaller/tertiary ui:TextBlock (same Grid row/column or
Grid.Row="2") with that warning; ensure Foreground uses ThemeResource
TextFillColorTertiaryBrush and TextWrapping="Wrap" so styling and layout match
existing elements.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
BetterGenshinImpact/App.xaml.csBetterGenshinImpact/Core/Config/OtherConfig.csBetterGenshinImpact/Service/HttpLogServerService.csBetterGenshinImpact/User/I18n/en.jsonBetterGenshinImpact/View/Pages/CommonSettingsPage.xaml
Comment on lines
+75
to
+91
| private void BroadcastLog(string json) | ||
| { | ||
| var data = $"data: {Uri.EscapeDataString(json)}\n\n"; | ||
| var buffer = Encoding.UTF8.GetBytes(data); | ||
|
|
||
| foreach (var client in _clients.Keys) | ||
| { | ||
| try | ||
| { | ||
| client.OutputStream.Write(buffer, 0, buffer.Length); | ||
| client.OutputStream.Flush(); | ||
| } | ||
| catch | ||
| { | ||
| _clients.TryRemove(client, out _); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
同步广播会阻塞日志链路。
Emit() 调用链中对每个 SSE 客户端执行同步 Write/Flush（Line 84-85），慢客户端会拖慢甚至阻塞产生日志的线程。建议改为后台异步发送并加写超时/淘汰策略。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@BetterGenshinImpact/Service/HttpLogServerService.cs` around lines 75 - 91,
BroadcastLog is doing synchronous Write/Flush to each SSE client
(client.OutputStream.Write/Flush) which can block the logging thread; change it
to perform non-blocking background sends using asynchronous APIs (e.g.,
WriteAsync/FlushAsync) and avoid iterating _clients.Keys directly by
snapshotting the client list; run each send in a Task with a
CancellationToken/timeout and remove the client via _clients.TryRemove(client,
out _) on exception or timeout to implement eviction. Locate BroadcastLog and
replace sync calls to client.OutputStream.Write/Flush with asynchronous send
tasks, apply a per-client timeout/ cancellation policy, and ensure failed or
slow clients are removed from _clients.
Comment on lines
+94
to
+123
| private void StartServer() | ||
| { | ||
| var config = _configService.Get().OtherConfig.HttpLogServerConfig; | ||
| if (!config.Enabled) return; | ||
|
|
||
| _cts = new CancellationTokenSource(); | ||
| _listener = new HttpListener(); | ||
|
|
||
| var address = string.IsNullOrWhiteSpace(config.ListenAddress) ? "0.0.0.0" : config.ListenAddress; | ||
| var prefix = address == "0.0.0.0" ? "+" : address; | ||
|
|
||
| if (address != "0.0.0.0" && address != "localhost" && address != "127.0.0.1" && !System.Net.IPAddress.TryParse(address, out _)) | ||
| { | ||
| Log.Error($"Invalid listen address: {address}. Using 0.0.0.0 instead."); | ||
| prefix = "+"; | ||
| address = "0.0.0.0"; | ||
| } | ||
|
|
||
| _listener.Prefixes.Add($"http://{prefix}:{config.Port}/"); | ||
|
|
||
| try | ||
| { | ||
| _listener.Start(); | ||
| Task.Run(() => ListenAsync(_cts.Token)); | ||
| Log.Information($"日志服务器已启动 {address}:{config.Port}"); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Log.Error(ex, "启动日志服务器失败。请检查端口是否被占用，或是否需要 0.0.0.0 的管理员权限。"); | ||
| } |
Contributor
There was a problem hiding this comment.
Prefixes.Add 放在 try 外会导致启动异常逃逸。
Line 112 的前缀注册在 try 之外，配置异常时会绕过你当前的错误处理逻辑。请先做端口边界校验，并把 Prefixes.Add 放入同一个 try。
🧯 建议修复
private void StartServer()
{
var config = _configService.Get().OtherConfig.HttpLogServerConfig;
if (!config.Enabled) return;
+ if (config.Port is < 1 or > 65535)
+ {
+ Log.Error("Invalid port: {Port}. Expected range: 1-65535.", config.Port);
+ return;
+ }
_cts = new CancellationTokenSource();
_listener = new HttpListener();
var address = string.IsNullOrWhiteSpace(config.ListenAddress) ? "0.0.0.0" : config.ListenAddress;
var prefix = address == "0.0.0.0" ? "+" : address;
@@
- _listener.Prefixes.Add($"http://{prefix}:{config.Port}/");
-
try
{
+ _listener.Prefixes.Add($"http://{prefix}:{config.Port}/");
_listener.Start();
Task.Run(() => ListenAsync(_cts.Token));
Log.Information($"日志服务器已启动 {address}:{config.Port}");
}
catch (Exception ex)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private void StartServer() | |
| { | |
| var config = _configService.Get().OtherConfig.HttpLogServerConfig; | |
| if (!config.Enabled) return; | |
| _cts = new CancellationTokenSource(); | |
| _listener = new HttpListener(); | |
| var address = string.IsNullOrWhiteSpace(config.ListenAddress) ? "0.0.0.0" : config.ListenAddress; | |
| var prefix = address == "0.0.0.0" ? "+" : address; | |
| if (address != "0.0.0.0" && address != "localhost" && address != "127.0.0.1" && !System.Net.IPAddress.TryParse(address, out _)) | |
| { | |
| Log.Error($"Invalid listen address: {address}. Using 0.0.0.0 instead."); | |
| prefix = "+"; | |
| address = "0.0.0.0"; | |
| } | |
| _listener.Prefixes.Add($"http://{prefix}:{config.Port}/"); | |
| try | |
| { | |
| _listener.Start(); | |
| Task.Run(() => ListenAsync(_cts.Token)); | |
| Log.Information($"日志服务器已启动 {address}:{config.Port}"); | |
| } | |
| catch (Exception ex) | |
| { | |
| Log.Error(ex, "启动日志服务器失败。请检查端口是否被占用，或是否需要 0.0.0.0 的管理员权限。"); | |
| } | |
| private void StartServer() | |
| { | |
| var config = _configService.Get().OtherConfig.HttpLogServerConfig; | |
| if (!config.Enabled) return; | |
| if (config.Port is < 1 or > 65535) | |
| { | |
| Log.Error("Invalid port: {Port}. Expected range: 1-65535.", config.Port); | |
| return; | |
| } | |
| _cts = new CancellationTokenSource(); | |
| _listener = new HttpListener(); | |
| var address = string.IsNullOrWhiteSpace(config.ListenAddress) ? "0.0.0.0" : config.ListenAddress; | |
| var prefix = address == "0.0.0.0" ? "+" : address; | |
| if (address != "0.0.0.0" && address != "localhost" && address != "127.0.0.1" && !System.Net.IPAddress.TryParse(address, out _)) | |
| { | |
| Log.Error($"Invalid listen address: {address}. Using 0.0.0.0 instead."); | |
| prefix = "+"; | |
| address = "0.0.0.0"; | |
| } | |
| try | |
| { | |
| _listener.Prefixes.Add($"http://{prefix}:{config.Port}/"); | |
| _listener.Start(); | |
| Task.Run(() => ListenAsync(_cts.Token)); | |
| Log.Information($"日志服务器已启动 {address}:{config.Port}"); | |
| } | |
| catch (Exception ex) | |
| { | |
| Log.Error(ex, "启动日志服务器失败。请检查端口是否被占用，或是否需要 0.0.0.0 的管理员权限。"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@BetterGenshinImpact/Service/HttpLogServerService.cs` around lines 94 - 123,
In StartServer(), the call to _listener.Prefixes.Add is currently outside the
try/catch so any exceptions escape your handler; move the Prefixes.Add call into
the same try block that calls _listener.Start() and
Task.Run(()=>ListenAsync(...)) and perform port validation (e.g., ensure
config.Port is within valid TCP port range) before constructing the prefix, so
any prefix/port-related exceptions are caught by the existing catch; update
references to prefix/address construction and keep ListenAsync, _listener.Start
and the catch block unchanged.
Comment on lines
+168
to
+186
| private async Task HandleRequestAsync(HttpListenerContext context) | ||
| { | ||
| var request = context.Request; | ||
| var response = context.Response; | ||
|
|
||
| try | ||
| { | ||
| if (request.Url.AbsolutePath == "/") | ||
| { | ||
| ServeHtml(response); | ||
| } | ||
| else if (request.Url.AbsolutePath == "/logs") | ||
| { | ||
| await ServeLogsSseAsync(response); | ||
| } | ||
| else if (request.Url.AbsolutePath == "/screenshot") | ||
| { | ||
| ServeScreenshot(response); | ||
| } |
Contributor
There was a problem hiding this comment.
缺少鉴权会暴露日志与截图。
/logs 和 /screenshot 当前无访问控制，只要能连到该端口就可读取运行日志和游戏画面。建议至少增加访问令牌（query/header）或明确的 IP 白名单策略。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@BetterGenshinImpact/Service/HttpLogServerService.cs` around lines 168 - 186,
The endpoints ServeLogsSseAsync and ServeScreenshot are exposed without auth
inside HandleRequestAsync; add an access check early in HandleRequestAsync
(e.g., call a new ValidateAccess(HttpListenerRequest) helper) that verifies a
configured bearer token from a header or query parameter and/or checks
context.Request.RemoteEndPoint against an IP whitelist, and return a 401/403
response if validation fails; update calls to ServeLogsSseAsync and
ServeScreenshot to only execute after ValidateAccess passes and keep ServeHtml
publicly accessible or similarly protect it if desired.
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.
添加http日志服务器功能，允许在其他设备通过浏览器查看日志和当前游戏界面
Summary by CodeRabbit
发布说明