-
Notifications
You must be signed in to change notification settings - Fork 6
Add configurable buffer size and tests for large message handling #157
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
Made buffer size configurable via bufferSize parameter (default 65535, min 1024, max 10MB) to handle different message size requirements. Fixed TCP socket handling to properly receive messages larger than the buffer by implementing multi-read logic that: - Peeks at message length header to determine total message size - Reads additional data in chunks until complete message is received - Validates messages don't exceed 100MB max size - Properly combines remaining data from previous reads Improved error handling to distinguish between TCP (can read more) and UDP/Unix datagram sockets (message gets truncated by OS). Added comprehensive tests with 1500-byte buffers for all socket types (TCP, UDP, Unix) to verify correct handling of messages within buffer, exceeding buffer, and multiple message scenarios. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Pre-allocate the data slice with the length of the remainingMsg (leftover from previous iteration) and the n bytes newly read into msgBuffer Copy the remainingMsg and the new n message bytes
|
Still on the works but submitting to get some initial feedback Generated several tests, maybe there is some code duplication there that can be removed |
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.
Pull request overview
This PR adds configurable buffer sizes and implements proper handling of large messages in the socket transport plugin. The key improvements focus on TCP socket handling with multi-read logic for messages larger than the buffer, while also adding comprehensive tests for all socket types (TCP, UDP, Unix).
- Added configurable
bufferSizeparameter with validation (default: 65535, min: 1024, max: 10MB) - Implemented TCP multi-read logic to handle messages exceeding buffer size by reading additional chunks
- Enhanced error handling to differentiate between TCP streams (can read more) and datagram sockets (OS truncates)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 16 comments.
| File | Description |
|---|---|
| plugins/transport/socket/main.go | Added buffer size configuration with validation, implemented TCP large message handling with multi-read logic, improved error messages for buffer overflow scenarios |
| plugins/transport/socket/main_test.go | Added comprehensive test suites for Unix, UDP, and TCP sockets testing small buffers with messages within/exceeding buffer size and multiple message scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Drop in favor of #158 |
Made buffer size configurable via bufferSize parameter (default 65535, min 1024, max 10MB) to handle different message size requirements.
Fixed TCP socket handling to properly receive messages larger than the buffer by implementing multi-read logic that:
Improved error handling to distinguish between TCP (can read more) and UDP/Unix datagram sockets (message gets truncated by OS).
Added comprehensive tests with 1500-byte buffers for all socket types (TCP, UDP, Unix) to verify correct handling of messages within buffer, exceeding buffer, and multiple message scenarios.