Skip to content

Send DELETE request when closing a Streamable HTTP session on the client #501

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+50 −2
Open

Send DELETE request when closing a Streamable HTTP session on the client #501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/ModelContextProtocol.Core/Client/StreamableHttpClientSessionTransport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,16 @@ public override async ValueTask DisposeAsync()

try
{
// Send DELETE request to terminate the session only send if we have a session ID per MCP spec
if (!string.IsNullOrEmpty(_mcpSessionId))
{
using var deleteRequest = new HttpRequestMessage(HttpMethod.Delete, _options.Endpoint);
CopyAdditionalHeaders(deleteRequest.Headers, _options.AdditionalHeaders, _mcpSessionId);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder to self to include _mcpProtocolVersion here once #500 is merged.


// Do not validate we get a successful status code, because server support for the DELETE request is optional
using var deleteResponse = await _httpClient.SendAsync(deleteRequest, CancellationToken.None).ConfigureAwait(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this fails, it looks like that failure will propagate (for things other than cancellation). Do we want to eat any failures?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also be:

Suggested change
using var deleteResponse = await _httpClient.SendAsync(deleteRequest, CancellationToken.None).ConfigureAwait(false);
(await _httpClient.SendAsync(deleteRequest, CancellationToken.None).ConfigureAwait(false)).Dispose();

Copy link
Contributor Author

@halter73 halter73 Jun 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on the fence about this. Would it be even better to make this fire-and-forget? We risk disposing the SocketsHttpHandler before the DELETE request completes, but maybe that's better than unnecessarily slowing down DisposeAsync.

In general, I do not like catching exceptions unless there is a clear reason we should. I've seen too many cases where overly broad catch blocks in teardown logic make it harder to diagnose errors related to graceful shutdown.

The flip side of this is that we might be disposing the client in reaction to some other sever-side error, and an error from the DELETE request could obscure that. And most people probably don't really care if the DELETE request succeeds or even that it gets sent.

}

if (_getReceiveTask != null)
{
await _getReceiveTask.ConfigureAwait(false);
Expand Down
42 changes: 40 additions & 2 deletions tests/ModelContextProtocol.AspNetCore.Tests/StreamableHttpClientConformanceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ namespace ModelContextProtocol.AspNetCore.Tests;
public class StreamableHttpClientConformanceTests(ITestOutputHelper outputHelper) : KestrelInMemoryTest(outputHelper), IAsyncDisposable
{
private WebApplication? _app;
private readonly List<string> _deleteRequestSessionIds = [];

private async Task StartAsync()
// Don't add the delete endpoint by default to ensure the client still works with basic sessionless servers.
private async Task StartAsync(bool enableDelete = false)
{
Builder.Services.Configure<JsonOptions>(options =>
{
Expand All @@ -28,14 +30,20 @@ private async Task StartAsync()
Services = _app.Services,
});

_app.MapPost("/mcp", (JsonRpcMessage message) =>
_app.MapPost("/mcp", (JsonRpcMessage message, HttpContext context) =>
{
if (message is not JsonRpcRequest request)
{
// Ignore all non-request notifications.
return Results.Accepted();
}

if (enableDelete)
{
// Add a session ID to the response to enable session tracking
context.Response.Headers.Append("mcp-session-id", "test-session-123");
}

if (request.Method == "initialize")
{
return Results.Json(new JsonRpcResponse
Expand Down Expand Up @@ -87,6 +95,15 @@ private async Task StartAsync()
throw new Exception("Unexpected message!");
});

if (enableDelete)
{
_app.MapDelete("/mcp", context =>
{
_deleteRequestSessionIds.Add(context.Request.Headers["mcp-session-id"].ToString());
return Task.CompletedTask;
});
}

await _app.StartAsync(TestContext.Current.CancellationToken);
}

Expand Down Expand Up @@ -136,6 +153,27 @@ public async Task CanCallToolConcurrently()
await Task.WhenAll(echoTasks);
}

[Fact]
public async Task SendsDeleteRequestOnDispose()
{
await StartAsync(enableDelete: true);

await using var transport = new SseClientTransport(new()
{
Endpoint = new("http://localhost/mcp"),
TransportMode = HttpTransportMode.StreamableHttp,
}, HttpClient, LoggerFactory);

await using var client = await McpClientFactory.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);

// Dispose should trigger DELETE request
await client.DisposeAsync();

// Verify DELETE request was sent with correct session ID
var sessionId = Assert.Single(_deleteRequestSessionIds);
Assert.Equal("test-session-123", sessionId);
}

private static async Task CallEchoAndValidateAsync(McpClientTool echoTool)
{
var response = await echoTool.CallAsync(new Dictionary<string, object?>() { ["message"] = "Hello world!" }, cancellationToken: TestContext.Current.CancellationToken);
Expand Down
Loading