-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Add support for ServerSentEventsResult and extension methods #60616
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
Changes from all commits
5d844ff
de86920
6f1f6be
664aa6d
ff67f8d
b0c76b8
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 |
---|---|---|
@@ -1,2 +1,10 @@ | ||
#nullable enable | ||
Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<T> | ||
Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<T>.ExecuteAsync(Microsoft.AspNetCore.Http.HttpContext! httpContext) -> System.Threading.Tasks.Task! | ||
Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<T>.StatusCode.get -> int? | ||
static Microsoft.AspNetCore.Http.HttpResults.RedirectHttpResult.IsLocalUrl(string? url) -> bool | ||
static Microsoft.AspNetCore.Http.Results.ServerSentEvents(System.Collections.Generic.IAsyncEnumerable<string!>! values, string? eventType = null) -> Microsoft.AspNetCore.Http.IResult! | ||
static Microsoft.AspNetCore.Http.Results.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<System.Net.ServerSentEvents.SseItem<T>>! values) -> Microsoft.AspNetCore.Http.IResult! | ||
static Microsoft.AspNetCore.Http.Results.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<T>! values, string? eventType = null) -> Microsoft.AspNetCore.Http.IResult! | ||
static Microsoft.AspNetCore.Http.TypedResults.ServerSentEvents(System.Collections.Generic.IAsyncEnumerable<string!>! values, string? eventType = null) -> Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<string!>! | ||
static Microsoft.AspNetCore.Http.TypedResults.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<System.Net.ServerSentEvents.SseItem<T>>! values) -> Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<T>! | ||
static Microsoft.AspNetCore.Http.TypedResults.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<T>! values, string? eventType = null) -> Microsoft.AspNetCore.Http.HttpResults.ServerSentEventsResult<T>! |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
|
||
using System.Diagnostics.CodeAnalysis; | ||
using System.IO.Pipelines; | ||
using System.Net.ServerSentEvents; | ||
using System.Security.Claims; | ||
using System.Text; | ||
using System.Text.Json; | ||
|
@@ -978,6 +979,49 @@ public static IResult AcceptedAtRoute<TValue>(string? routeName, RouteValueDicti | |
#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters | ||
=> value is null ? TypedResults.AcceptedAtRoute(routeName, routeValues) : TypedResults.AcceptedAtRoute(value, routeName, routeValues); | ||
|
||
/// <summary> | ||
/// Produces a <see cref="ServerSentEventsResult{TValue}"/> response. | ||
/// </summary> | ||
/// <param name="values">The values to be included in the HTTP response body.</param> | ||
/// <param name="eventType">The event type to be included in the HTTP response body.</param> | ||
/// <returns>The created <see cref="ServerSentEventsResult{TValue}"/> for the response.</returns> | ||
/// <remarks> | ||
/// Strings serialized by this result type are serialized as raw strings without any additional formatting. | ||
/// </remarks> | ||
#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters | ||
public static IResult ServerSentEvents(IAsyncEnumerable<string> values, string? eventType = null) | ||
#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters | ||
=> new ServerSentEventsResult<string>(values, eventType); | ||
|
||
/// <summary> | ||
/// Produces a <see cref="ServerSentEventsResult{T}"/> response. | ||
/// </summary> | ||
/// <typeparam name="T">The type of object that will be serialized to the response body.</typeparam> | ||
captainsafia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// <param name="values">The values to be included in the HTTP response body.</param> | ||
/// <param name="eventType">The event type to be included in the HTTP response body.</param> | ||
/// <returns>The created <see cref="ServerSentEventsResult{T}"/> for the response.</returns> | ||
/// <remarks> | ||
/// Strings serialized by this result type are serialized as raw strings without any additional formatting. | ||
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. Do we need to expose an option overriding this behavior? 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. We opted to expose fewer APIs for modifying the behavior of the formatter to start but I think we can pursue the proposal of adding an |
||
/// Other types are serialized using the configured JSON serializer options. | ||
/// </remarks> | ||
#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters | ||
public static IResult ServerSentEvents<T>(IAsyncEnumerable<T> values, string? eventType = null) | ||
#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters | ||
=> new ServerSentEventsResult<T>(values, eventType); | ||
|
||
/// <summary> | ||
/// Produces a <see cref="ServerSentEventsResult{T}"/> response. | ||
/// </summary> | ||
/// <typeparam name="T">The type of object that will be serialized to the response body.</typeparam> | ||
/// <param name="values">The values to be included in the HTTP response body.</param> | ||
/// <returns>The created <see cref="ServerSentEventsResult{T}"/> for the response.</returns> | ||
/// <remarks> | ||
/// Strings serialized by this result type are serialized as raw strings without any additional formatting. | ||
/// Other types are serialized using the configured JSON serializer options. | ||
/// </remarks> | ||
public static IResult ServerSentEvents<T>(IAsyncEnumerable<SseItem<T>> values) | ||
=> new ServerSentEventsResult<T>(values); | ||
|
||
/// <summary> | ||
/// Produces an empty result response, that when executed will do nothing. | ||
/// </summary> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Buffers; | ||
using System.Net.ServerSentEvents; | ||
using Microsoft.AspNetCore.Http.Metadata; | ||
using System.Reflection; | ||
using Microsoft.AspNetCore.Builder; | ||
using System.Text.Json; | ||
using Microsoft.Extensions.Options; | ||
using Microsoft.AspNetCore.Http.Json; | ||
using Microsoft.Extensions.DependencyInjection; | ||
using Microsoft.AspNetCore.Http.Features; | ||
|
||
namespace Microsoft.AspNetCore.Http.HttpResults; | ||
|
||
/// <summary> | ||
/// Represents a result that writes a stream of server-sent events to the response. | ||
/// </summary> | ||
/// <typeparam name="T">The underlying type of the events emitted.</typeparam> | ||
public sealed class ServerSentEventsResult<T> : IResult, IEndpointMetadataProvider, IStatusCodeHttpResult | ||
{ | ||
private readonly IAsyncEnumerable<SseItem<T>> _events; | ||
|
||
/// <inheritdoc/> | ||
public int? StatusCode => StatusCodes.Status200OK; | ||
|
||
internal ServerSentEventsResult(IAsyncEnumerable<T> events, string? eventType) | ||
{ | ||
_events = WrapEvents(events, eventType); | ||
} | ||
|
||
internal ServerSentEventsResult(IAsyncEnumerable<SseItem<T>> events) | ||
{ | ||
_events = events; | ||
} | ||
|
||
/// <inheritdoc /> | ||
public async Task ExecuteAsync(HttpContext httpContext) | ||
{ | ||
ArgumentNullException.ThrowIfNull(httpContext); | ||
|
||
httpContext.Response.ContentType = "text/event-stream"; | ||
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. Consider following what we do in SignalR 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. Oh, and line 47 if that's something we care about. 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. Bleh -- I am torn about adding the Firefox workaround here. That seems like something the Maybe we leave it out for now and see what kind of feedback we get? 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. Yeah, until we see feedback I think it's less interesting for an API. For SignalR it was important because we want to signal that the connection is active and let the user start sending messages. 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. Any reason not to disable buffering? 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.
What is that? 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. We should do what SignalR is doing, we should not wait for feedback. Disable buffering and do the crazy IIS workaround 😄 (did you test this in IIS?) 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.
SignalR applies a special workaround to send some filler data through the SSE stream before the actual data to force Firefox's EventSource to emit an open event. It's a reaction to this bug in Firefox. The discussion is around whether or not to apply this workaround here in the ServerSentEventResult or have it be part of the default writing beahvior in the SseFormatter. |
||
httpContext.Response.Headers.CacheControl = "no-cache,no-store"; | ||
httpContext.Response.Headers.Pragma = "no-cache"; | ||
httpContext.Response.Headers.ContentEncoding = "identity"; | ||
|
||
var bufferingFeature = httpContext.Features.GetRequiredFeature<IHttpResponseBodyFeature>(); | ||
bufferingFeature.DisableBuffering(); | ||
|
||
var jsonOptions = httpContext.RequestServices.GetService<IOptions<JsonOptions>>()?.Value ?? new JsonOptions(); | ||
|
||
// If the event type is string, we can skip JSON serialization | ||
// and directly use the SseFormatter's WriteAsync overload for strings. | ||
if (_events is IAsyncEnumerable<SseItem<string>> stringEvents) | ||
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. What about byte[]? 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. It's useful in scenaria where you need to write raw UTF-8 data, but it might make more sense if we exposed that using the IBufferWriter callback as with the underlying library. 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 seems like a good proposal. In the meantime, we can add special handling for byte arrays in our |
||
{ | ||
await SseFormatter.WriteAsync(stringEvents, httpContext.Response.Body, httpContext.RequestAborted); | ||
return; | ||
} | ||
|
||
await SseFormatter.WriteAsync(_events, httpContext.Response.Body, | ||
(item, writer) => FormatSseItem(item, writer, jsonOptions), | ||
httpContext.RequestAborted); | ||
captainsafia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
private static void FormatSseItem(SseItem<T> item, IBufferWriter<byte> writer, JsonOptions jsonOptions) | ||
{ | ||
if (item.Data is null) | ||
{ | ||
writer.Write([]); | ||
return; | ||
} | ||
|
||
// Handle byte arrays byt writing them directly as strings. | ||
if (item.Data is byte[] byteArray) | ||
{ | ||
writer.Write(byteArray); | ||
return; | ||
} | ||
|
||
// For non-string types, use JSON serialization with options from DI | ||
var runtimeType = item.Data.GetType(); | ||
var jsonTypeInfo = jsonOptions.SerializerOptions.GetTypeInfo(typeof(T)); | ||
|
||
// Use the appropriate JsonTypeInfo based on whether we need polymorphic serialization | ||
var typeInfo = jsonTypeInfo.ShouldUseWith(runtimeType) | ||
? jsonTypeInfo | ||
: jsonOptions.SerializerOptions.GetTypeInfo(typeof(object)); | ||
|
||
var json = JsonSerializer.SerializeToUtf8Bytes(item.Data, typeInfo); | ||
writer.Write(json); | ||
} | ||
|
||
private static async IAsyncEnumerable<SseItem<T>> WrapEvents(IAsyncEnumerable<T> events, string? eventType = null) | ||
{ | ||
await foreach (var item in events) | ||
{ | ||
yield return new SseItem<T>(item, eventType); | ||
} | ||
} | ||
|
||
static void IEndpointMetadataProvider.PopulateMetadata(MethodInfo method, EndpointBuilder builder) | ||
{ | ||
ArgumentNullException.ThrowIfNull(method); | ||
ArgumentNullException.ThrowIfNull(builder); | ||
|
||
builder.Metadata.Add(new ProducesResponseTypeMetadata(StatusCodes.Status200OK, typeof(SseItem<T>), contentTypes: ["text/event-stream"])); | ||
captainsafia marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} |
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.
Event types are values scoped to individual events, and not the entire stream. Given that the
SseItem<T>
overload hardcodes to serialization, how can this API produce raw SSE events that includes other per-event fields such as event types?I would guess that this was discussed during API review, but have you considered factoring into a separate method group (e.g.
ServerSentEventsRaw
) that also acceptsIAsyncEnumerable<SseItem<string>>
?Uh oh!
There was an error while loading. Please reload this page.
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.
Nevermind me, it seems the comment in line 1004 is addressing my concern.