Add client completion notification and details#1368
Add client completion notification and details#1368stephentoub merged 4 commits intomodelcontextprotocol:mainfrom
Conversation
src/ModelContextProtocol.Core/Client/StdioClientCompletionDetails.cs
Outdated
Show resolved
Hide resolved
halter73
left a comment
There was a problem hiding this comment.
I would like to see this implemented for stateful Streamable HTTP when there are 404s as well, but I think this is already a nice improvement even without that part. I have a small preference for the abstract Completion property over the method, but I don't care too much either way on that.
src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs
Outdated
Show resolved
Hide resolved
860e132
cedcddc to
860e132
Compare
halter73
left a comment
There was a problem hiding this comment.
It turns out 10x wasn't enough 😭
Failed Checks: - ClientRespectsRetryField: Client MUST respect the retry field, waiting the given number of milliseconds before attempting to reconnect Error: Client reconnected very late (5355ms instead of 500ms). Client appears to be ignoring the retry field and using its own backoff strategy.
src/ModelContextProtocol.Core/Client/StreamableHttpClientSessionTransport.cs
Outdated
Show resolved
Hide resolved
src/ModelContextProtocol.Core/Client/StreamableHttpClientSessionTransport.cs
Show resolved
Hide resolved
I think there's a real pre-existing bug lurking. |
|
This appears to be failing the API check due to adding an abstract member even though the McpClient constructor is experimental. Should we suppress this? |
Maybe, but then why would the retry happen after 5 seconds? The To me, this seems like the main issue is likely with the tests. It might be caused by a low core count CI machine and too much test parallelism causing thread pool starvation. Or maybe some tests not properly cleaning up after themselves causing similar results. |
Introduce a new mechanism for reporting MCP client session completion details, including both graceful and error terminations. Add a Completion property to McpClient, exposing a `Task<ClientCompletionDetails>` that completes with details about session closure. Implement extensible completion details (including StdioClientCompletionDetails for stdio transports) and propagate them via a new internal TransportClosedException.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
a467c3d to
5bf5a63
Compare
5bf5a63 to
de6cab4
Compare
Introduce a new mechanism for reporting MCP client session completion details, including both graceful and error terminations. Add a Completion property to McpClient, exposing a
Task<ClientCompletionDetails>that completes with details about session closure. Implement extensible completion details (including StdioClientCompletionDetails for stdio transports) and propagate them via a new internal TransportClosedException.Closes #1332
Closes #438