refactor(stdio): improve stdio server message handling #73
Conversation
- Add context-aware message reading with cancellation support - Implement graceful EOF handling in input stream processing - Add comprehensive error handling and logging - Improve notification handling with dedicated goroutine
WalkthroughThis pull request refactors the notification and input stream handling within the Changes
Possibly related PRs
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
server/stdio.go (1)
122-151: Consider simplifying the duplicate EOF handlingThe processInputStream method properly handles EOF as a normal termination condition, but there's redundant EOF handling at lines 136-138 and 144-146. Since line 137 already returns nil when encountering EOF, the second check at lines 144-146 may never be reached.
func (s *StdioServer) processInputStream(ctx context.Context, reader *bufio.Reader, stdout io.Writer) error { for { if err := ctx.Err(); err != nil { return err } line, err := s.readNextLine(ctx, reader) if err != nil { if err == io.EOF { return nil } s.errLogger.Printf("Error reading input: %v", err) return err } if err := s.processMessage(ctx, line, stdout); err != nil { - if err == io.EOF { - return nil - } s.errLogger.Printf("Error handling message: %v", err) return err } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/stdio.go(2 hunks)
🔇 Additional comments (3)
server/stdio.go (3)
106-120: Great implementation of asynchronous notification handlingThe
handleNotificationsmethod provides a clean separation of concerns by moving notification processing to a dedicated goroutine. It properly handles context cancellation and logs errors without interrupting the notification flow.
153-183: Well-implemented context-aware read operationThe
readNextLinemethod effectively makes blocking I/O operations cancellable via context, which is an excellent practice. The implementation properly manages goroutines and channels, making the code both robust and maintainable.One minor suggestion: consider adding a comment about potential goroutine leaks if the parent routine doesn't ensure proper closure of the reader when the context is cancelled.
208-209: Clean refactoring of the Listen methodThe modifications to the Listen method leverage the newly added functions, making the code more modular and easier to maintain. The asynchronous handling of notifications and the structured processing of input streams significantly improve the code organization.
Split stdio server message handling into dedicated functions for better readability:
Each function has a single responsibility and clear error handling strategy.
Summary by CodeRabbit
New Features
Refactor