PR 501 added logging to the streamable server, but didn't pass a non-nil
logger to the streamable transport (an oversight). However, tests still
passed due to net/http's panic recovery.
Update test servers to enforce that handlers do not panic, and fix the
panic by:
1. Passing along the configured logger to the session transport.
2. Using ensureLogger in Connect, since we technically allow
StreamableServerTransports to be constructed by the user.
Fixes#556
mcp/server.go: implement server‑side logging
- Thread a *slog.Logger through server paths to improve debuggability
- Introduce no‑op discard handler + ensureLogger so loggers are never
nil
- Use ensured loggers in Server
- address todo comments related to adding logging support
All tests are passing and no functionality should change with this PR
Fixes https://github.com/modelcontextprotocol/go-sdk/issues/170
In practice, the streamable client transport was having trouble
connecting to various backends, because they are nonconformant in
various ways.
While it would be nice if all servers conformed to the spec, in practice
there are certain spec violations that are recoverable, and we can and
should recover them. Specifically:
- tolerate 404 instead of 405 for the hanging GET (#393)
- tolerate (=ignore) spurious response body for notifications and
responses, since we know none are expected
Additionally, fix a bug that we weren't parsing Content-Type correctly.
Fixes#521
## Problem
Context propagation was broken in `StreamableClientTransport` because:
1. `Connect()` used `context.Background()` instead of the parent context
2. `Close()` created a race condition where DELETE requests were
cancelled before completion
## Solution
- Use parent context when creating the connection context in `Connect()`
- Reorder `Close()` operations to perform cleanup DELETE before
cancelling context
## Impact
- Request-scoped values (auth tokens, trace IDs) now propagate correctly
to background HTTP operations
- Eliminates race condition in cleanup operations
- Maintains proper Go context semantics
Fixes#513
Add a framework for feature documentation, and start populating it with
our SDK documentation.
This framework is as follows:
- internal/docs/**.src.md is the markdown source for the docs/
directory.
- The x/example/internal/cmd/weave tool is used to compile these docs to
the top-level docs, supporting both linked code samples and generated
tables of contents.
- The readme-check workflow is updated to check these docs as well.
- The structure of these docs follows the MCP spec.
- Wherever possible, example code is linked from actual Go documentation
examples, so that it is testable.
Some minor modifications to the weave tool were made to support this
framework.
Additionally, partially fill out this documentation with content on
base protocol and client features, as well as troubleshooting help.
Along the way, a bug was encountered that our LoggingTransport was not
concurrency safe. This is fixed with a mutex.
Fixes https://github.com/modelcontextprotocol/go-sdk/issues/466
Fixes https://github.com/modelcontextprotocol/go-sdk/issues/409
Updates https://github.com/modelcontextprotocol/go-sdk/issues/442
Previously, the error message received from ClientSession.Wait would
only report the closeErr, which would often be nil even if the client
transport was broken. Wait should return the reason the session
terminated, if abnormal.
I'm not sure of the exact semantics of this, but surely returning nil is
less useful than returning a meaningful non-nil error. We can refine our
handling of errors once we have more feedback.
Also add a test for client termination on HTTP server shutdown,
described in #265. This should work as long as (1) the session is
stateful (with a hanging GET), or (2) the session is stateless but the
client has a keepalive ping.
Also: don't send DELETE if the session was terminated with 404; +test.
Fixes#265
In #272, we made transports open structs and deprecated their
constructors. However, we left the constructors in place to allow go:fix
directives to facilitate migration.
Now let's remove the constructors before cutting the release.
Fixes#305
The streamable client connection can break for a variety of reasons,
asynchronously to the client's request. Decorate these failures with
additional context to clarify why they occurred.
Add a test for the failure message of #393.
Fixes#393
Use a fake streamable server to facilitate testing client behavior.
For this commit, just update the existing test (moved to a new file for
isolation). Subsequent CLs will add more tests.
Improve one client error message that occurred while debuging tests.
For #393
During initialization, the default logging level is set to info allowing
servers to emit logs immediately without waiting for a client to set a
log level.
The updated example shows how to send a notification in the context of
the request.
Fixes#387
The condition sessionID == "" was not quite right for disallowing GET
requests. Since we decided to differentiate "stateless" vs "sessionless"
servers, we need to disallow GET requests for both.
For #393
Exporting StreamableHTTPOptions.JSONResponse is helpful for some users behind proxies that don't work well with text/event-stream.
For #211
Co-authored-by: ccpro10 <co1797249167@gmail.com>
This CL utilizes the event store to write outgoing messages and removes
the unbounded outgoing data structure.
It also adds a new interface [EventStore.Open]
For #190
An onClose function is passed to the ServerSession and ClientSession to
help cleanup resources from the caller.
The onClose function will be executed as part of the ServerSession and
ClientSession closure.
Fixes#258
If there is a TokenInfo in the request context of a
StreamableServerTransport, then propagate it through to the
ServerRequest that is passed to server methods like callTool.
Fixes#317.
A few problems with reconnection cropped up in the review of PR #307.
We should allow for the hanging GET to fail with StatusMethodNotAllowed.
This simply means that the server does not support sending notifications
or requests over the GET, which is allowed in the spec.
Also, we should fix the initial delay of the hanging GET request: it
should start with 0 delay. Fix the math for this and subsequent
attempts. Incidentally, this makes the tests take 3s on my machine, down
from 9s.
Also address some comments from #307.
Several improvements for the stateless streamable mode, plus support for
a 'distributed' (or rather, distributable) version of the stateless
server.
- Add a 'Stateless' option to StreamableHTTPOptions and
StreamableServerTransport, which controls stateless behavior.
GetSessionID may still return a non-empty session ID.
- Audit validation of stateless mode to allow requests with a session
id. Propagate this session ID to the temporary connection.
- Peek at requests to allow 'initialize' requests to go through to the
session, so that version negotiation can occur (FIXME: add tests).
Fixes#284
For #148
Implement the concurrency model described in #26: notifications are
synchronous, but calls are asynchronous (except for 'initialize'). To
achieve this, implement jsonrpc2.Async(ctx) to signal asynchronous
handling. This is simpler to use than returning ErrAsyncResponse and
calling Respond, and since this is an internal detail we don't need to
worry too much about whether it's idiomatic.
Add tests that verify both features, for both client and server.
Also:
- replace req.ID.IsValid with req.IsCall
- remove the methodHandler type as we can just use MethodHandler
Fixes#26
Do a pass through the streamable client reconnection logic, and fix
several bugs.
- Establish the initial GET even if MaxRetries is 0 (#256). This was
broken because the GET bypasses the initial request and going straight
to the SSE GET reconnection logic.
- Release the stream ownership when POST requests exit.
- Don't reconnect POST requests if we've received the expected response.
- Move unexported reconnection config to constants. Otherwise it is too
hard to set ReconnectOptions (you have to use the DefaultOptions and
mutate).
Fixes#256
In #202, I added the checkRequest helper to validate incoming requests,
and invoked it in the stremable transports to preemptively reject
invalid HTTP requests, so that a jsonrpc error could be translated to an
HTTP error.
However, this introduced a bug: since cancellation was handled in the
jsonrpc2 layer, we never had to validate it in the mcp layer, and
therefore never added methodInfo. As a result, it was reported as an
invalid request in the http layer.
Add a test, and a fix. The simplest fix was to create stubs that are
placeholders for cancellation.
This was discovered in the course of investigating #285.
Allow other header values for the 'Accept' header that imply
application/json and text/event-stream for requests to the Streamable
HTTP Transport.
Fixes#290
Unit tests here seemed like unnecessary complexity and wouldn't fit
cleanly with the style of the `streamable_test.go` tests. Happy to add
those if desired though.
As described in #272, there is really no reason for transports to be
closed structs with constructors, since their state must be established
by a call to Connect. Making them open structs simplifies their APIs,
and means that all transports can be extended in the future: we don't
have to create empty Options structs just for the purpose of future
compatibility.
For now, the related constructors and options structs are simply
deprecated (with go:fix directives where possible). A future CL will
remove them prior to the v1.0.0 release.
For #272
Support stateless streamable sessions by adding a GetSessionID function
to StreamableHTTPOptions. If GetSessionID returns "", the session is
stateless, and no validation is performed. This is implemented by
providing the session a trivial initialization state.
To implement this, some parts of #232 (distributed sessions) are copied
over, since they add an API for creating an already-initialized session.
In total, the following new API is added:
- StreamableHTTPOptions.GetSessionID
- ServerSessionOptions (a new parameter to Server.Connect)
- ServerSessionState
- ClientSessionOptions (a new parameter to Client.Connect, for symmetry)
For #10
Add a new (currently unexported) jsonResponse option to
StreamableServerTransportOptions, and use it to control the response
content type, serving application/json if set.
Additionally:
- A transportOptions field is added to the StreamableHTTPOptions.
- A messages iterator is added to encapsulate the iteration of stream
messages, since the handling of JSON and SSE responses are otherwise
very different.
- The serving flow is refactored to avoid returning (statusCode,
message), primarily because this seemed liable to lead to redundant
calls to WriteHeader, because only local logic knows whether or not
any data has been written to the response.
- The serving flow is refactored to delegate to responseJSON and
responseSSE, according to the currently unexported jsonResponse
option.
- A bug is fixed where all GET streams were considered persistent: the
terminal condition req.Method == http.MethodPost && nOutstanding == 0
was not right: GET requests may implement stream resumption.
Updates #211
Make several cleanups of the streamable client transport, encountered
during work on JSON support for the streamable server:
- The 'Close' condition is differentiated from asynchronous failures. A
failure should unblock Read with an error, at which point the JSON-RPC
connection will be broken and closed.
- Fields are reordered in streamableClientConn to make guards more
apparent.
- The handling of sessionID is simplified: we simply set the session ID
whenever we receive response headers. No need to have special handling
for the first request, as the serializeation of session initialization
is implemented in Client.Connect.
- Since the above bullet makes Write a trivial wrapper around
postMessage, the two methods are merged.
- A bug is fixed where JSON responses were handled synchronously in
Write. This lead to deadlock when a hanging client->server request is
waiting on a server->client request. Now JSON is handled symmetrically
to SSE: the Write returns once response headers are received.
asynchronous to Write.
- The httpConnection interface is renamed to clientConnection, and
receive the entire InitializeResult.
- streamableClientConn receivers are renamed to be consistently 'c'.
As reported in #227, there was a race where streams could be terminated
before the final reply was written. Fix this by checking the number of
outstanding requests in the same critical section that acquires outgoing
messages to write.
Fixes#227
Add an isRequest field to methodInfo, used it to reject non-notification
requests that lack a valid ID. Furthermore, lift this validation to the
transport layer for HTTP server transports, so that we can preemptively
reject bad HTTP requests.
Fixes#194Fixes#197
- move done field outside of mutex hat
- refactor handler into a function that returns `error` to avoid repeated `http.Error` calls
- use an atomic for the signal channel to simplify locking
Introduce an EventStore interface to store events for resumable streams.
Provide an in-memory implementation.
Still to do: connect to streaming transports.
For #10
Export the Event type in preparation for providing user-definable
storage for resumable streams.
Also, move event code to a separate file.
This PR is just renaming and code motion.
