Skip to content

Conversation

ln-12
Copy link
Contributor

@ln-12 ln-12 commented Sep 1, 2025

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.

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
@ln-12 ln-12 force-pushed the fix_stateless_mode_logging branch from 55d59e8 to 431e181 Compare September 1, 2025 10:58
@ln-12
Copy link
Contributor Author

ln-12 commented Sep 1, 2025

Build pipeline is broken until #381 is merged.

@jba jba requested a review from findleyr September 1, 2025 12:10
@Thaikoma
Copy link

Thaikoma commented Sep 2, 2025

192.168.1.1

@findleyr
Copy link
Contributor

findleyr commented Sep 2, 2025

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.

@ln-12
Copy link
Contributor Author

ln-12 commented Sep 4, 2025

What would be your suggestion how to solve it then? Currently notifications directly fail here because the logLevel is always empty.

@jba
Copy link
Contributor

jba commented Sep 4, 2025

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:

  • TypeScript: the SDK doesn't provide any help for logging.
  • C#: it seems to behave like ours: no logging if no level is set.

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.

@ln-12
Copy link
Contributor Author

ln-12 commented Sep 4, 2025

Alright, thanks for the clarification. Then I'll close this one and we can continue discussion in the issue.

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.

Server notifications are not sent out in stateless mode
4 participants