Fix: Check Process.alive? before sending to SSE handler#239
Open
mellelieuwes wants to merge 2 commits intocloudwalk:mainfrom
Open
Fix: Check Process.alive? before sending to SSE handler#239mellelieuwes wants to merge 2 commits intocloudwalk:mainfrom
mellelieuwes wants to merge 2 commits intocloudwalk:mainfrom
Conversation
This fixes a race condition where responses could be silently lost when an SSE handler process died but the transport hadn't yet processed the :DOWN message. The issue: 1. SSE handler process dies (network drop, crash, etc.) 2. :DOWN message is queued to transport GenServer 3. Before transport processes :DOWN, a new request calls get_sse_handler 4. get_sse_handler returns the stale PID 5. send/2 silently drops the message to the dead process 6. Client receives HTTP 202 but never gets the actual response The fix adds a Process.alive? check in route_sse_response before sending. If the handler is stale, it cleans up the entry and establishes a new SSE connection for the request.
Instead of returning a generic "Server not initialized" error when a session is missing or expired, return a specific session_expired error with code -32001 and message "Session expired or not initialized. Please reconnect." This gives clients: 1. A specific error code (-32001) they can detect and handle 2. A clear message telling users what to do 3. Potential for auto-reconnect in MCP clients
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.
Summary
This PR includes two related fixes for session handling:
1. Fix stale SSE handler race condition
When an SSE handler process dies but the transport hasn't yet processed the
:DOWNmessage, responses could be silently lost.The issue:
:DOWNmessage is queued to transport GenServer:DOWN, a new request callsget_sse_handlerget_sse_handlerreturns the stale PIDsend/2silently drops the message to the dead processThe fix:
Process.alive?check inroute_sse_responsebefore sending2. Add session_expired error type
When a session is missing or expired (e.g., after server restart), return a clear error message instead of the vague "Server not initialized".
Changes:
-32001forsession_expiredTest plan
🤖 Generated with Claude Code