Skip to content

Conversation

@cliffhall
Copy link
Member

@cliffhall cliffhall commented May 30, 2025

  • 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

    • 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
    • in onServerError()
      • if error message includes 404 OR error.cause includes ECONNREFUSED, report "Connection refused."

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

existing-success-sse

New SSE connection success

success-sse

Existing SSE connection fail (no server at address)

existing-fail-sse-no-server

New SSE connection fail (no server at address)

fail-sse-no-server

Existing SSE connection fail (no endpoint on server)

existing-fail-sse-no-support

New SSE connection fail (no endpoint on server)

fail-sse-no-support

STDIO Connections

Existing STDIO connection success

existing-success-stdio

New STDIO connection success

success-stdio

Existing STDIO connection fail

existing-fail-stdio

New STDIO connection fail

fail-stdio

StreamableHttp Connections

Existing StreamableHttp connection success

existing-success-streamable

New StreamableHttp connection success

success-streamable

Existing StreamableHttp connection fail

existing-fail-streamable

New StreamableHttp connection fail

fail-streamable

Breaking Changes

Nope.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Output formatting

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

This fixes #348

…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."
@cliffhall cliffhall marked this pull request as draft May 30, 2025 21:49
@cliffhall cliffhall marked this pull request as ready for review June 2, 2025 14:25
cliffhall added 3 commits June 2, 2025 10:25
…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
Copy link
Member

@evalstate evalstate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Cliff, this works well

@evalstate evalstate merged commit 5ade12d into modelcontextprotocol:main Jun 5, 2025
2 checks passed
@cliffhall cliffhall deleted the normalize-connection-logging branch June 5, 2025 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Normalize reporting of failed connection.

2 participants