Normalize connection logging#478
Merged
evalstate merged 6 commits intomodelcontextprotocol:mainfrom Jun 5, 2025
Merged
Conversation
…e logging is verbose and different for every server type. This is an attempt to make the console output cleaner and more readable.
* In server/index.js
- in createTransport()
- stringify query params to put them on one line
- remove logging of transport creation (done in callers)
- in /mcp POST handler
- report POST request if session id is present
- otherwise, report new StreamableHttp connection request
- report creation of server transport
- report creation of client transport
- in /stdio GET handler
- report new STDIO connection request
- report creation of server transport
- report creation of client transport
- report 401 as authentication error without stack
- in stderr data handler
- if it includes "MODULE_NOT_FOUND"
- report "Command not found"
- remove transports
- otherwise send to client as usual
- in /sse GET handler
- report SSE connection request
- report 401 as authentication error without stack
- report 404 as server that may not support SSE
- report error with "ECONNREFUSED" as "Connection refused" without stack
- only create, map, and proxy transports if serverTransport was successfully created
- in /message POST handler
- report received POST message
* In mcpProxy.ts
- in onServerError()
- if error message includes 404 OR error.cause includes ECONNREFUSED, report "Connection refused."
…o get the session id for this leg of the proxy if it is a StreamableHttp connection.
* In server/src/index.ts
- slight format tweak of the Client <-> Proxy message so that the two messages will line up when displayed
* In server/src/mcpProxy.ts
- add reportedServerSession boolean initialized to false
- in transportToServer.onmessage,
- if reportedServerSession isn't set yet, report the sessionId if present on the transport, and then set reportedServerSession to true
evalstate
approved these changes
Jun 5, 2025
Member
evalstate
left a comment
There was a problem hiding this comment.
Thanks Cliff, this works well
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
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.
In server/index.js
In mcpProxy.ts
Motivation and Context
Normalize the reporting of connection success and errors. The logging is verbose and different for every server type. This is an attempt to make the console output cleaner and more readable.
How Has This Been Tested?
Connection attempts when server is present or absent. Before and after for each server type follows
SSE Connections
Existing SSE connection success
New SSE connection success
Existing SSE connection fail (no server at address)
New SSE connection fail (no server at address)
Existing SSE connection fail (no endpoint on server)
New SSE connection fail (no endpoint on server)
STDIO Connections
Existing STDIO connection success
New STDIO connection success
Existing STDIO connection fail
New STDIO connection fail
StreamableHttp Connections
Existing StreamableHttp connection success
New StreamableHttp connection success
Existing StreamableHttp connection fail
New StreamableHttp connection fail
Breaking Changes
Nope.
Types of changes
Checklist
Additional context
This fixes #348