-
Notifications
You must be signed in to change notification settings - Fork 586
Streamable HTTP resumability + redelivery + SSE polling via server-side disconnect #1077
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
1e407ce
677d9ea
63fb022
11494e1
493062a
fe71286
2065c0e
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 |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ internal sealed class StreamableHttpHandler( | |
| ILoggerFactory loggerFactory) | ||
| { | ||
| private const string McpSessionIdHeaderName = "Mcp-Session-Id"; | ||
| private const string LastEventIdHeaderName = "Last-Event-ID"; | ||
|
|
||
| private static readonly JsonTypeInfo<JsonRpcMessage> s_messageTypeInfo = GetRequiredJsonTypeInfo<JsonRpcMessage>(); | ||
| private static readonly JsonTypeInfo<JsonRpcError> s_errorTypeInfo = GetRequiredJsonTypeInfo<JsonRpcError>(); | ||
|
|
@@ -81,17 +82,80 @@ await WriteJsonRpcErrorAsync(context, | |
| return; | ||
| } | ||
|
|
||
| StreamableHttpSession? session = null; | ||
| ISseEventStreamReader? eventStreamReader = null; | ||
|
|
||
| var sessionId = context.Request.Headers[McpSessionIdHeaderName].ToString(); | ||
| var session = await GetSessionAsync(context, sessionId); | ||
| var lastEventId = context.Request.Headers[LastEventIdHeaderName].ToString(); | ||
|
|
||
| if (!string.IsNullOrEmpty(sessionId)) | ||
| { | ||
| session = await GetSessionAsync(context, sessionId); | ||
| if (session is null) | ||
| { | ||
| // There was an error obtaining the session; consider the request failed. | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| if (!string.IsNullOrEmpty(lastEventId)) | ||
| { | ||
| if (HttpServerTransportOptions.Stateless) | ||
| { | ||
| await WriteJsonRpcErrorAsync(context, | ||
| "Bad Request: The Last-Event-ID header is not supported in stateless mode.", | ||
| StatusCodes.Status400BadRequest); | ||
| return; | ||
| } | ||
|
|
||
| eventStreamReader = await GetEventStreamReaderAsync(context, lastEventId); | ||
| if (eventStreamReader is null) | ||
| { | ||
| // There was an error obtaining the event stream; consider the request failed. | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| if (session is not null && eventStreamReader is not null && !string.Equals(session.Id, eventStreamReader.SessionId, StringComparison.Ordinal)) | ||
| { | ||
| await WriteJsonRpcErrorAsync(context, | ||
| "Bad Request: The Last-Event-ID header refers to a session with a different session ID.", | ||
| StatusCodes.Status400BadRequest); | ||
| return; | ||
| } | ||
|
|
||
| if (eventStreamReader is null || string.Equals(eventStreamReader.StreamId, StreamableHttpServerTransport.UnsolicitedMessageStreamId, StringComparison.Ordinal)) | ||
| { | ||
| await HandleUnsolicitedMessageStreamAsync(context, session, eventStreamReader); | ||
| } | ||
| else | ||
| { | ||
| await HandleResumePostResponseStreamAsync(context, eventStreamReader); | ||
| } | ||
| } | ||
|
|
||
| private async Task HandleUnsolicitedMessageStreamAsync(HttpContext context, StreamableHttpSession? session, ISseEventStreamReader? eventStreamReader) | ||
| { | ||
| if (HttpServerTransportOptions.Stateless) | ||
| { | ||
| await WriteJsonRpcErrorAsync(context, | ||
| "Bad Request: Unsolicited messages are not supported in stateless mode.", | ||
| StatusCodes.Status400BadRequest); | ||
| return; | ||
| } | ||
|
|
||
| if (session is null) | ||
| { | ||
| await WriteJsonRpcErrorAsync(context, | ||
| "Bad Request: Mcp-Session-Id header is required", | ||
| StatusCodes.Status400BadRequest); | ||
| return; | ||
| } | ||
|
|
||
| if (!session.TryStartGetRequest()) | ||
| if (eventStreamReader is null && !session.TryStartGetRequest()) | ||
| { | ||
| await WriteJsonRpcErrorAsync(context, | ||
| "Bad Request: This server does not support multiple GET requests. Start a new session to get a new GET SSE response.", | ||
| "Bad Request: This server does not support multiple GET requests. Use Last-Event-ID header to resume or start a new session.", | ||
| StatusCodes.Status400BadRequest); | ||
| return; | ||
| } | ||
|
|
@@ -111,7 +175,7 @@ await WriteJsonRpcErrorAsync(context, | |
| // will be sent in response to a different POST request. It might be a while before we send a message | ||
| // over this response body. | ||
| await context.Response.Body.FlushAsync(cancellationToken); | ||
| await session.Transport.HandleGetRequestAsync(context.Response.Body, cancellationToken); | ||
| await session.Transport.HandleGetRequestAsync(context.Response.Body, eventStreamReader, cancellationToken); | ||
| } | ||
| catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) | ||
| { | ||
|
|
@@ -120,6 +184,12 @@ await WriteJsonRpcErrorAsync(context, | |
| } | ||
| } | ||
|
|
||
| private static async Task HandleResumePostResponseStreamAsync(HttpContext context, ISseEventStreamReader eventStreamReader) | ||
| { | ||
| InitializeSseResponse(context); | ||
| await eventStreamReader.CopyToAsync(context.Response.Body, context.RequestAborted); | ||
| } | ||
|
|
||
|
Comment on lines
+187
to
+192
Collaborator
Author
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. Note that when resuming a POST stream, we read directly from the This differs from how the
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.
This scares me a little. Why should you have to poll If the issue is that you could have a new I think by far the simplest thing is to force all the messages sent over any resumption stream over the ISseEventStreamReader, solicited or unsolicited. Directly writing to a resumption stream should not be possible. |
||
| public async Task HandleDeleteRequestAsync(HttpContext context) | ||
| { | ||
| var sessionId = context.Request.Headers[McpSessionIdHeaderName].ToString(); | ||
|
|
@@ -131,14 +201,7 @@ public async Task HandleDeleteRequestAsync(HttpContext context) | |
|
|
||
| private async ValueTask<StreamableHttpSession?> GetSessionAsync(HttpContext context, string sessionId) | ||
| { | ||
| StreamableHttpSession? session; | ||
|
|
||
| if (string.IsNullOrEmpty(sessionId)) | ||
| { | ||
| await WriteJsonRpcErrorAsync(context, "Bad Request: Mcp-Session-Id header is required", StatusCodes.Status400BadRequest); | ||
| return null; | ||
| } | ||
| else if (!sessionManager.TryGetValue(sessionId, out session)) | ||
| if (!sessionManager.TryGetValue(sessionId, out var session)) | ||
| { | ||
| // -32001 isn't part of the MCP standard, but this is what the typescript-sdk currently does. | ||
| // One of the few other usages I found was from some Ethereum JSON-RPC documentation and this | ||
|
|
@@ -194,12 +257,16 @@ private async ValueTask<StreamableHttpSession> StartNewSessionAsync(HttpContext | |
| { | ||
| SessionId = sessionId, | ||
| FlowExecutionContextFromRequests = !HttpServerTransportOptions.PerSessionExecutionContext, | ||
| EventStreamStore = HttpServerTransportOptions.EventStreamStore, | ||
| RetryInterval = HttpServerTransportOptions.RetryInterval, | ||
| }; | ||
| context.Response.Headers[McpSessionIdHeaderName] = sessionId; | ||
| } | ||
| else | ||
| { | ||
| // In stateless mode, each request is independent. Don't set any session ID on the transport. | ||
| // If in the future we support resuming stateless requests, we should populate | ||
| // the event stream store and retry interval here as well. | ||
| sessionId = ""; | ||
| transport = new() | ||
| { | ||
|
|
@@ -246,6 +313,28 @@ private async ValueTask<StreamableHttpSession> CreateSessionAsync( | |
| return session; | ||
| } | ||
|
|
||
| private async ValueTask<ISseEventStreamReader?> GetEventStreamReaderAsync(HttpContext context, string lastEventId) | ||
| { | ||
| if (HttpServerTransportOptions.EventStreamStore is not { } eventStreamStore) | ||
| { | ||
| await WriteJsonRpcErrorAsync(context, | ||
| "Bad Request: This server does not support resuming streams.", | ||
| StatusCodes.Status400BadRequest); | ||
| return null; | ||
| } | ||
|
|
||
| var eventStreamReader = await eventStreamStore.GetStreamReaderAsync(lastEventId, context.RequestAborted); | ||
| if (eventStreamReader is null) | ||
| { | ||
| await WriteJsonRpcErrorAsync(context, | ||
| "Bad Request: The specified Last-Event-ID is either invalid or expired.", | ||
| StatusCodes.Status400BadRequest); | ||
| return null; | ||
| } | ||
|
|
||
| return eventStreamReader; | ||
| } | ||
|
|
||
| private static Task WriteJsonRpcErrorAsync(HttpContext context, string errorMessage, int statusCode, int errorCode = -32000) | ||
| { | ||
| var jsonRpcError = new JsonRpcError | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -108,4 +108,17 @@ public required Uri Endpoint | |||||
| /// Gets sor sets the authorization provider to use for authentication. | ||||||
| /// </summary> | ||||||
| public ClientOAuthOptions? OAuth { get; set; } | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// Gets or sets the maximum number of reconnection attempts when an SSE stream is disconnected. | ||||||
|
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.
Suggested change
|
||||||
| /// </summary> | ||||||
| /// <value> | ||||||
| /// The maximum number of reconnection attempts. The default is 2. | ||||||
| /// </value> | ||||||
| /// <remarks> | ||||||
| /// When an SSE stream is disconnected (e.g., due to a network issue), the client will attempt to | ||||||
| /// reconnect using the Last-Event-ID header to resume from where it left off. This property controls | ||||||
| /// how many reconnection attempts are made before giving up. | ||||||
| /// </remarks> | ||||||
| public int MaxReconnectionAttempts { get; set; } = 2; | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change prepares for the future possibility of allowing stateless servers to resume POST streams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still respond with a
405 Method Not AllowedHTTP status for non-resume GET requests? I don't want stateless servers wasting resources keeping an SSE stream open they're never going to send unsolicited messages over.https://modelcontextprotocol.io/specification/2025-11-25/basic/transports#listening-for-messages-from-the-server
Statelessprobably isn't the right thing to base this behavior off of, and we should probably instead offer a separate configurable option on whether to support GETs for unsolicited messages. After all, you could design a Stateless app that sends unsolicited messages and a stateful one that doesn't, but I think that should be another PR if we do that.I think it's okay to map the GET handler for resumability assuming we actually support it in stateless mode, but we should still respond with a 405 for non-resuming GET requests for unsolicited messages even if the 405 is not entirely correct since the method is allowed at least in some cases.