-
Notifications
You must be signed in to change notification settings - Fork 707
Create sample client #265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create sample client #265
Conversation
WalkthroughA new Go example program, Changes
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
examples/simple_client/main.go (3)
63-79
: Consider adding a cancellation mechanism for the stderr reader goroutine.The goroutine that reads from stderr doesn't have a mechanism to stop when the main function exits or when the context is canceled. This could potentially lead to a goroutine leak.
Consider modifying the goroutine to respect context cancellation:
if stderr, ok := client.GetStderr(c); ok { - go func() { + go func() { + done := ctx.Done() buf := make([]byte, 4096) for { + // Check for context cancellation + select { + case <-done: + return + default: + } + n, err := stderr.Read(buf) if err != nil { if err != io.EOF { log.Printf("Error reading stderr: %v", err) } return } if n > 0 { fmt.Fprintf(os.Stderr, "[Server] %s", buf[:n]) } } }() }
158-193
: Command parsing could be enhanced to handle escaped quotes.While the function correctly handles basic quoted strings, it doesn't support escaped quotes inside quoted strings (e.g.,
"hello \"world\""
).The comment on line 160 already acknowledges this limitation, but if you want to enhance it for better usability, consider adding support for escaped quotes:
func parseCommand(cmd string) []string { var result []string var current string var inQuote bool var quoteChar rune + var escaped bool for _, r := range cmd { + if escaped { + current += string(r) + escaped = false + continue + } + + if r == '\\' { + escaped = true + continue + } switch { case r == ' ' && !inQuote: if current != "" { result = append(result, current) current = "" } case (r == '"' || r == '\''): if inQuote && r == quoteChar { inQuote = false quoteChar = 0 } else if !inQuote { inQuote = true quoteChar = r } else { current += string(r) } default: current += string(r) } } if current != "" { result = append(result, current) } return result }
52-57
: Consider adding error handling fornil
command.The
transport.NewStdio()
function might not validate that the command is not empty, which could lead to runtime errors.While the code already checks that
args
is not empty, it might be clearer to add a specific check for command validity:// Create stdio transport with verbose logging + if command == "" { + log.Fatalf("Error: Empty command specified") + } stdioTransport := transport.NewStdio(command, nil, cmdArgs...)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/simple_client/main.go
(1 hunks)
🔇 Additional comments (9)
examples/simple_client/main.go (9)
1-15
: Well-structured imports and package declaration.The imports are well-organized and include all necessary packages for I/O operations, logging, CLI flags, and the MCP-Go client libraries.
17-28
: Good CLI flag validation.The command-line flag handling is implemented correctly with proper validation to ensure exactly one transport mode is specified. The error message and usage display provide clear guidance to users.
30-32
: Appropriate context management.Using a context with timeout is a good practice. The 30-second timeout seems reasonable for this example, and the
defer cancel()
ensures proper cleanup.
97-100
: Simple and effective notification handler.The notification handler is straightforward and appropriate for an example client.
102-115
: Proper client initialization with error handling.The client initialization includes all necessary parameters and has appropriate error handling.
123-136
: Good practice checking capabilities before making requests.Checking if the server supports tools before attempting to list them is a good practice. The error handling is appropriately non-fatal for this feature.
138-151
: Consistent approach for resource listing.The resource listing follows the same pattern as tool listing, with appropriate capability checking and error handling.
83-91
: Good error handling for SSE transport creation and startup.The code properly checks for errors when creating and starting the SSE transport, with appropriate fatal error logging.
153-154
: Proper client cleanup.The client is correctly closed before program exit, ensuring clean resource management.
Summary by CodeRabbit