-
Notifications
You must be signed in to change notification settings - Fork 644
Add client completion notification and details #1368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
631841d
0b8138c
860e132
0d60fe2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| namespace ModelContextProtocol.Client; | ||
|
|
||
| /// <summary> | ||
| /// Provides details about why an MCP client session completed. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// Transport implementations may return derived types with additional strongly-typed | ||
| /// information, such as <see cref="StdioClientCompletionDetails"/>. | ||
| /// </para> | ||
| /// </remarks> | ||
| public class ClientCompletionDetails | ||
| { | ||
| /// <summary> | ||
| /// Gets the exception that caused the session to close, if any. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// This is <see langword="null"/> for graceful closure. | ||
| /// </remarks> | ||
| public Exception? Exception { get; set; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| using System.Net; | ||
|
|
||
| namespace ModelContextProtocol.Client; | ||
|
|
||
| /// <summary> | ||
| /// Provides details about the completion of an HTTP-based MCP client session, | ||
| /// including sessions using the legacy SSE transport or the Streamable HTTP transport. | ||
| /// </summary> | ||
| public sealed class HttpClientCompletionDetails : ClientCompletionDetails | ||
| { | ||
| /// <summary> | ||
| /// Gets the HTTP status code that caused the session to close, or <see langword="null"/> if unavailable. | ||
| /// </summary> | ||
| public HttpStatusCode? HttpStatusCode { get; set; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| namespace ModelContextProtocol.Client; | ||
|
|
||
| /// <summary> | ||
| /// Provides details about the completion of a stdio-based MCP client session. | ||
| /// </summary> | ||
| public sealed class StdioClientCompletionDetails : ClientCompletionDetails | ||
| { | ||
| /// <summary> | ||
| /// Gets the process ID of the server process, or <see langword="null"/> if unavailable. | ||
| /// </summary> | ||
| public int? ProcessId { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the exit code of the server process, or <see langword="null"/> if unavailable. | ||
| /// </summary> | ||
| public int? ExitCode { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets the last lines of the server process's standard error output, or <see langword="null"/> if unavailable. | ||
| /// </summary> | ||
| public IReadOnlyList<string>? StandardErrorTail { get; set; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,11 +20,12 @@ internal sealed partial class StreamableHttpClientSessionTransport : TransportBa | |
|
|
||
| private readonly McpHttpClient _httpClient; | ||
| private readonly HttpClientTransportOptions _options; | ||
| private readonly CancellationTokenSource _connectionCts; | ||
| private readonly CancellationTokenSource _connectionCts = new(); | ||
| private readonly ILogger _logger; | ||
|
|
||
| private string? _negotiatedProtocolVersion; | ||
| private Task? _getReceiveTask; | ||
| private volatile TransportClosedException? _disconnectError; | ||
|
|
||
| private readonly SemaphoreSlim _disposeLock = new(1, 1); | ||
| private bool _disposed; | ||
|
|
@@ -42,7 +43,6 @@ public StreamableHttpClientSessionTransport( | |
|
|
||
| _options = transportOptions; | ||
| _httpClient = httpClient; | ||
| _connectionCts = new CancellationTokenSource(); | ||
| _logger = (ILogger?)loggerFactory?.CreateLogger<HttpClientTransport>() ?? NullLogger.Instance; | ||
|
|
||
| // We connect with the initialization request with the MCP transport. This means that any errors won't be observed | ||
|
|
@@ -96,6 +96,13 @@ internal async Task<HttpResponseMessage> SendHttpRequestAsync(JsonRpcMessage mes | |
| // We'll let the caller decide whether to throw or fall back given an unsuccessful response. | ||
| if (!response.IsSuccessStatusCode) | ||
| { | ||
| // Per the MCP spec, a 404 response to a request containing an Mcp-Session-Id | ||
| // indicates the session has ended. Signal completion so McpClient.Completion resolves. | ||
| if (response.StatusCode == HttpStatusCode.NotFound && SessionId is not null) | ||
| { | ||
| SetSessionExpired(response.StatusCode); | ||
| } | ||
|
|
||
| return response; | ||
| } | ||
|
|
||
|
|
@@ -184,18 +191,14 @@ public override async ValueTask DisposeAsync() | |
| { | ||
| LogTransportShutdownFailed(Name, ex); | ||
| } | ||
| finally | ||
| { | ||
| _connectionCts.Dispose(); | ||
| } | ||
| } | ||
| finally | ||
| { | ||
| // If we're auto-detecting the transport and failed to connect, leave the message Channel open for the SSE transport. | ||
| // This class isn't directly exposed to public callers, so we don't have to worry about changing the _state in this case. | ||
| if (_options.TransportMode is not HttpTransportMode.AutoDetect || _getReceiveTask is not null) | ||
| { | ||
| SetDisconnected(); | ||
| SetDisconnected(_disconnectError ?? new TransportClosedException(new HttpClientCompletionDetails())); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot Assuming that the |
||
| } | ||
| } | ||
| } | ||
|
|
@@ -204,8 +207,8 @@ private async Task ReceiveUnsolicitedMessagesAsync() | |
| { | ||
| var state = new SseStreamState(); | ||
|
|
||
| // Continuously receive unsolicited messages until canceled | ||
| while (!_connectionCts.Token.IsCancellationRequested) | ||
| // Continuously receive unsolicited messages until canceled or disconnected | ||
| while (!_connectionCts.Token.IsCancellationRequested && IsConnected) | ||
| { | ||
| await SendGetSseRequestWithRetriesAsync( | ||
| relatedRpcRequest: null, | ||
|
|
@@ -285,6 +288,13 @@ await SendGetSseRequestWithRetriesAsync( | |
|
|
||
| if (!response.IsSuccessStatusCode) | ||
| { | ||
| // Per the MCP spec, a 404 response to a request containing an Mcp-Session-Id | ||
| // indicates the session has ended. Signal completion so McpClient.Completion resolves. | ||
| if (response.StatusCode == HttpStatusCode.NotFound && SessionId is not null) | ||
| { | ||
| SetSessionExpired(response.StatusCode); | ||
| } | ||
|
|
||
| // If the server could be reached but returned a non-success status code, | ||
| // retrying likely won't change that. | ||
| return null; | ||
|
|
@@ -474,4 +484,23 @@ private static TimeSpan ElapsedSince(long stopwatchTimestamp) | |
| return TimeSpan.FromSeconds((double)(Stopwatch.GetTimestamp() - stopwatchTimestamp) / Stopwatch.Frequency); | ||
| #endif | ||
| } | ||
|
|
||
| private void SetSessionExpired(HttpStatusCode statusCode) | ||
| { | ||
| // Store the error before canceling so DisposeAsync can use it if it races us, especially | ||
| // after the call to Cancel below, to invoke SetDisconnected. | ||
| _disconnectError = new TransportClosedException(new HttpClientCompletionDetails | ||
| { | ||
| HttpStatusCode = statusCode, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot Can you please remove the |
||
| Exception = new McpException( | ||
| "The server returned HTTP 404 for a request with an Mcp-Session-Id, indicating the session has expired. " + | ||
| "To continue, create a new client session or call ResumeSessionAsync with a new connection."), | ||
| }); | ||
|
|
||
| // Cancel to unblock any in-flight operations (e.g., SSE stream reads in | ||
| // SendGetSseRequestWithRetriesAsync) that are waiting on _connectionCts.Token. | ||
| _connectionCts.Cancel(); | ||
|
|
||
| SetDisconnected(_disconnectError); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.