chore: Remove unnecessary SSHProxy.Serve() method#344
Conversation
| p.mu.Lock() | ||
|
|
||
| if p.shuttingDown { | ||
| p.mu.Unlock() | ||
| // reject the connection and return error | ||
| err := conn.Close() | ||
| if err != nil { | ||
| p.config.logger.Error("Failed to close connection", zap.Error(err)) | ||
| } | ||
|
|
||
| return errShuttingDown | ||
| } | ||
|
|
||
| p.mu.Unlock() | ||
|
|
||
| return p.serveConn(ctx, conn) |
There was a problem hiding this comment.
serveConn has the exact same logic to check for shutting down so this method is unnecessary.
There was a problem hiding this comment.
Pull request overview
This PR removes the exported SSHProxy.Serve() method and routes connection handling directly through the existing SSHProxy.serveConn() implementation to eliminate duplicated shutdown-guard logic.
Changes:
- Updated
SSHProxy.Start()to callserveConn()directly for each accepted connection. - Removed the now-redundant
SSHProxy.Serve()wrapper method. - Updated the shutdown-related unit test to invoke
serveConn().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/sshhandler/proxy.go | Removes Serve() and switches Start() to call serveConn() directly. |
| internal/sshhandler/proxy_test.go | Updates the shutdown/active-connection test to call serveConn() instead of Serve(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #344 +/- ##
==========================================
+ Coverage 86.07% 86.17% +0.09%
==========================================
Files 40 40
Lines 2830 2820 -10
==========================================
- Hits 2436 2430 -6
+ Misses 269 266 -3
+ Partials 125 124 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Changes
Remove unnecessary
SSHProxy.Serve()method because it duplicates logic inSSHProxy.serveConn().