mcp/streamable: reuse transport if sessionID is given in stateless mode#389
mcp/streamable: reuse transport if sessionID is given in stateless mode#389ln-12 wants to merge 2 commits intomodelcontextprotocol:mainfrom
Conversation
This change fixes logging not working in stateless mode as the transport (and therefore the configured log level) is not used across requests. Fixes modelcontextprotocol#387
55d59e8 to
431e181
Compare
|
Build pipeline is broken until #381 is merged. |
|
192.168.1.1 |
|
Hi, I don't understand this change. If the server is distributed, there's no way that state changes (such as the configured log level) can be shared across multiple backends. Log messages in the context of a single request should work, but they must be at the default log level. |
|
What would be your suggestion how to solve it then? Currently notifications directly fail here because the |
|
The default log level is "no logging". The spec is unclear about this. I filed an issue about that some time ago, but it's never been addressed. I looked at a couple of other SDKs:
I'm fine if we want to treat the default level as INFO. We may have to change that if the spec ever clarifies things, but at this point that doesn't seem very likely. I don't see how this PR can fix that in general. If you're in stateless mode, it's probably because you have a distributed server. In that case, even if the client sets the log level, that request will go to only one server process. A better fix is to change the logic at the code you linked to. |
|
Alright, thanks for the clarification. Then I'll close this one and we can continue discussion in the issue. |
This change fixes logging not working in stateless mode as the transport (and therefore the configured log level) is not used across requests. With this change, the transport is reused in the case that a non empty sessionID was given. Fixes #387.