-
Notifications
You must be signed in to change notification settings - Fork 746
fix: replace bufio.Scanner with bufio.Reader to support large message… #603
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
Conversation
…s in stdio transport The bufio.Scanner has a default 64KB token limit which causes 'token too long' errors when MCP servers send large messages (e.g., large tool responses, resource contents, or prompts). This change replaces Scanner with Reader.ReadString('\n') which can handle arbitrarily large lines. Changes: - client/transport/stdio.go: Changed stdout from *bufio.Scanner to *bufio.Reader - testdata/mockstdio_server.go: Applied same fix to mock server - client/transport/stdio_test.go: Added TestStdio_LargeMessages with tests for messages ranging from 1KB to 5MB to ensure the fix works correctly The original code (pre-commit 4e353ac) used bufio.Reader, but was incorrectly changed to Scanner claiming it would avoid panics with long lines. This fix reverts to the Reader approach which actually handles large messages correctly. Fixes issue where stdio clients fail with 'bufio.Scanner: token too long' error when communicating with servers that send large responses.
WalkthroughReplaces bufio.Scanner with bufio.Reader for stdio transport and mock server, updating read loops to use ReadString with CRLF trimming. Adjusts initialization in NewIO and spawnCommand. Adds large message stress tests for echo requests across sizes up to ~5MB, with a duplicated test block included. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
Merging this branch will not change overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
…s in stdio transport (mark3labs#603) The bufio.Scanner has a default 64KB token limit which causes 'token too long' errors when MCP servers send large messages (e.g., large tool responses, resource contents, or prompts). This change replaces Scanner with Reader.ReadString('\n') which can handle arbitrarily large lines. Changes: - client/transport/stdio.go: Changed stdout from *bufio.Scanner to *bufio.Reader - testdata/mockstdio_server.go: Applied same fix to mock server - client/transport/stdio_test.go: Added TestStdio_LargeMessages with tests for messages ranging from 1KB to 5MB to ensure the fix works correctly The original code (pre-commit 4e353ac) used bufio.Reader, but was incorrectly changed to Scanner claiming it would avoid panics with long lines. This fix reverts to the Reader approach which actually handles large messages correctly. Fixes issue where stdio clients fail with 'bufio.Scanner: token too long' error when communicating with servers that send large responses.
The bufio.Scanner has a default 64KB token limit which causes 'token too long' errors when MCP servers send large messages (e.g., large tool responses, resource contents, or prompts). This change replaces Scanner with Reader.ReadString('\n') which can handle arbitrarily large lines.
Changes:
The original code (pre-commit 4e353ac) used bufio.Reader, but was incorrectly changed to Scanner claiming it would avoid panics with long lines. This fix reverts to the Reader approach which actually handles large messages correctly.
Fixes issue where stdio clients fail with 'bufio.Scanner: token too long' error when communicating with servers that send large responses.
Description
Fixes #588
Type of Change
Checklist
MCP Spec Compliance
Additional Information
Summary by CodeRabbit
Bug Fixes
Tests