Resolved MCP client is not working with SSE transport#208
Open
bettercallsaulj wants to merge 7 commits intofix/improve_for_mcp_oauthfrom
Open
Resolved MCP client is not working with SSE transport#208bettercallsaulj wants to merge 7 commits intofix/improve_for_mcp_oauthfrom
bettercallsaulj wants to merge 7 commits intofix/improve_for_mcp_oauthfrom
Conversation
added 7 commits
March 28, 2026 17:47
…nt (#207) Bug: connect() returned success immediately after initiating TCP connect (which returns EINPROGRESS). When initialize() was called shortly after, connected_=false triggered reconnection logic, closing the in-progress connection before it could complete. Root Cause: The connect() method fulfilled its promise as soon as connection_manager_->connect() returned success, but that only meant the TCP connect was initiated, not completed. The actual TCP+SSL handshake happens asynchronously in the event loop. Fix: Added pending_connect_promise_ member that is fulfilled by handleConnectionEvent() when ConnectionEvent::Connected fires (after TCP+SSL handshake completes). The connect() method now correctly blocks until the connection is fully established. Changes: - Added pending_connect_promise_ and connect_promise_mutex_ members - Modified connect() to store promise and wait for event callback - Modified handleConnectionEvent() to fulfill promise on Connected - Handle error cases (RemoteClose, exceptions) by fulfilling with error - Thread-safe promise fulfillment with mutex protection - Comprehensive unit tests for promise behavior and thread safety
Bug: closeSocket() could be called multiple times during destruction chain, leading to: 1. Multiple raiseEvent() calls on destroyed ConnectionImpl 2. Pure virtual function calls 3. Use-after-free crashes Root Cause: When ConnectionImpl is destroyed, the destructor chain can trigger multiple closeSocket() calls on the transport socket. Each call would invoke raiseEvent() on the callbacks_ pointer, which points to the already-destroyed ConnectionImpl object. Fix: Added guard at the beginning of closeSocket() implementations to check if already in Closed state. If so, return immediately without processing. This prevents: - Double raiseEvent() calls - Timer cancellation on destroyed timers - State transitions from invalid states
…te (#207) Bug: After SSL handshake completed successfully, the handshake_retry_timer continued to fire, calling performHandshakeStep() on an already-connected socket. This caused the state machine to transition from Connected to Error, triggering a RemoteClose event and closing the working connection. Root Cause: onHandshakeComplete() only cancelled handshake_timer_ but not handshake_retry_timer_. The retry timer was enabled during handshake with exponential backoff to poll for handshake progress. After handshake completion, it continued firing every interval. Fix: Added handshake_retry_timer_->disableTimer() call in onHandshakeComplete(), matching the existing handshake_timer_ cancellation. Both timers are now properly cleaned up when handshake succeeds.
…shakes (#207) Bug: SSL handshakes frequently stalled or timed out, especially with remote servers. The handshake would initiate but never complete, with the client waiting indefinitely for handshake data that had already arrived. Root Cause: Three interconnected issues prevented handshake data from being properly processed: 1. doRead() returned stop() during handshake without actually reading data from the socket. When read events fired, the handshake data (ServerHello, Certificate, etc.) was never fed to the SSL BIO, causing the state machine to stall. 2. moveToBio() only read once from the socket. Large TLS records (up to 16KB) might not be returned in a single read(), causing incomplete handshake messages and stalls. 3. After sending handshake data, no readable event was signaled. For level-triggered events, data that arrived during callback processing wouldn't trigger a new read event. - Handles pre-handshake states correctly (return stop, not close) - Fast path for connected state
…ects (#207) Bug: Connections would report as established before TCP handshake completed, causing send() operations to fail with ENOTCONN error. This race condition occurred primarily with local connections where the fallback timer fired before the actual write event. Root Cause: A fallback timer checked SO_ERROR to detect connection completion. However, SO_ERROR == 0 does NOT mean "connected" - it means "no error yet". A socket still in EINPROGRESS state (TCP handshake ongoing) will have SO_ERROR == 0. The timer incorrectly interpreted this as connection success and called onConnected() prematurely. The proper approach: Wait for the Write event from libevent/kqueue, which fires when: 1. Connection succeeds (socket becomes writable) 2. Connection fails (error is signaled) Only THEN check SO_ERROR. At that point, 0 means success, non-zero means failure.
Added a complete, production-ready example demonstrating how to use the MCP client with HTTP+SSE (Server-Sent Events) transport. This example serves as both documentation and a testing tool for the HTTP/SSE fixes. Features: - Full HTTPS/SSE connection establishment with TLS - MCP protocol initialization with capability negotiation - Tool discovery and listing - Tool invocation with JSON arguments - Comprehensive error handling and logging - Architecture diagram in code comments - Detailed SSE communication pattern documentation Example Usage: ./mcp_client_sse_example http://localhost:8080/sse ./mcp_client_sse_example https://mcp.example.com/v1/sse Architecture Documented: - Complete transport stack (TCP → SSL → HTTP → SSE → JSON-RPC → MCP) - Filter chain configuration - Bidirectional communication pattern - SSE event stream handling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR must be merged after #205