Conversation
WalkthroughReplaced the blocking Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@main.go`:
- Around line 215-222: The HTTP server shutdown only stops srv; you must also
wait for the relay/background goroutine pool (common.relayGoPool from
common/gopool.go) to drain before exiting main. After calling srv.Shutdown(ctx)
and before returning, call the relay pool's shutdown/wait method (e.g.,
common.relayGoPool.Shutdown(ctx) or common.relayGoPool.Wait()) or implement a
drain loop that checks relayGoPool.ActiveWorkers()/len(relayGoPool.workers) and
blocks until zero (respecting the same ctx timeout) so long-lived relay tasks
complete gracefully.
- Around line 196-207: Move the LogStartupSuccess call to after you successfully
bind the socket: perform a net.Listen("tcp", ":"+port) first (add the net
import), if listen returns without error then call
common.LogStartupSuccess(startTime, port), then start the server with
srv.Serve(listener) in the goroutine (or wrap listener in the existing srv).
Update the goroutine to use srv.Serve(listener) and still treat
http.ErrServerClosed as non-fatal; ensure the listener is closed on shutdown so
resources are cleaned up.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| // Log startup success message | ||
| common.LogStartupSuccess(startTime, port) | ||
|
|
||
| err = server.Run(":" + port) | ||
| if err != nil { | ||
| common.FatalLog("failed to start HTTP server: " + err.Error()) | ||
| srv := &http.Server{ | ||
| Addr: ":" + port, | ||
| Handler: server, | ||
| } | ||
|
|
||
| go func() { | ||
| if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { | ||
| common.FatalLog("failed to start HTTP server: " + err.Error()) | ||
| } |
There was a problem hiding this comment.
Bind the listener before logging startup success.
Line 197 logs a successful startup before Line 205 has actually bound the port. If the bind fails, operators still get a false-positive “started” log and the real failure only appears from a background goroutine.
Suggested shape
- common.LogStartupSuccess(startTime, port)
-
- srv := &http.Server{
- Addr: ":" + port,
- Handler: server,
- }
-
- go func() {
- if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
- common.FatalLog("failed to start HTTP server: " + err.Error())
- }
- }()
+ ln, err := net.Listen("tcp", ":"+port)
+ if err != nil {
+ common.FatalLog("failed to start HTTP server: " + err.Error())
+ }
+ common.LogStartupSuccess(startTime, port)
+
+ srv := &http.Server{
+ Handler: server,
+ }
+
+ go func() {
+ if err := srv.Serve(ln); err != nil && !errors.Is(err, http.ErrServerClosed) {
+ common.FatalLog("failed to start HTTP server: " + err.Error())
+ }
+ }()Also add the net import.
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @main.go around lines 196 - 207, Move the LogStartupSuccess call to after you
successfully bind the socket: perform a net.Listen("tcp", ":"+port) first (add
the net import), if listen returns without error then call
common.LogStartupSuccess(startTime, port), then start the server with
srv.Serve(listener) in the goroutine (or wrap listener in the existing srv).
Update the goroutine to use srv.Serve(listener) and still treat
http.ErrServerClosed as non-fatal; ensure the listener is closed on shutdown so
resources are cleaned up.
</details>
<!-- fingerprinting:phantom:medusa:grasshopper:322129d1-9e03-4e22-80a1-b6f30e657d48 -->
<!-- This is an auto-generated comment by CodeRabbit -->
| // SSE streams may run for minutes; give them time to finish before forced exit | ||
| shutdownTimeout := time.Duration(common.GetEnvOrDefault("SHUTDOWN_TIMEOUT_SECONDS", 120)) * time.Second | ||
| ctx, cancel := context.WithTimeout(context.Background(), shutdownTimeout) | ||
| defer cancel() | ||
| if err := srv.Shutdown(ctx); err != nil { | ||
| common.SysError(fmt.Sprintf("server forced to shutdown: %v", err)) | ||
| } | ||
| common.SysLog("server exited") |
There was a problem hiding this comment.
Wait for relay/background workers before returning from main.
srv.Shutdown(ctx) only waits for the http.Server. The relay path also uses common.relayGoPool (common/gopool.go:1-23), and this file starts several long-lived goroutines above. Line 222 returns from main immediately afterward, so anything still running outside the HTTP server is still terminated abruptly on process exit. That leaves a gap in the graceful-shutdown guarantee for streams offloaded to those workers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@main.go` around lines 215 - 222, The HTTP server shutdown only stops srv; you
must also wait for the relay/background goroutine pool (common.relayGoPool from
common/gopool.go) to drain before exiting main. After calling srv.Shutdown(ctx)
and before returning, call the relay pool's shutdown/wait method (e.g.,
common.relayGoPool.Shutdown(ctx) or common.relayGoPool.Wait()) or implement a
drain loop that checks relayGoPool.ActiveWorkers()/len(relayGoPool.workers) and
blocks until zero (respecting the same ctx timeout) so long-lived relay tasks
complete gracefully.
1 participant
📝 变更描述 / Description
将 main.go 中原来的 server.Run() 阻塞启动方式改为 http.Server + goroutine 启动，监听 SIGINT/SIGTERM 信号后调用 srv.Shutdown() 实现优雅关闭。
这样在部署重启或容器停止时，已有的请求（特别是 SSE 长连接）能在超时时间内正常完成，而不是被直接杀掉。超时时间通过环境变量
SHUTDOWN_TIMEOUT_SECONDS控制，默认 120 秒。📸 运行证明 / Proof of Work
Summary by CodeRabbit