Skip to content

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.

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

Merged
merged 6 commits into from
Mar 3, 2025
Merged

Add support for ServerSentEventsResult and extension methods #60616

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5d844ff
Add support for ServerSentEventsResult and extension methods
captainsafia Feb 25, 2025
de86920
Address feedback
captainsafia Feb 26, 2025
6f1f6be
Use TaskCompletionSource to observe cancellation in tests
captainsafia Feb 26, 2025
664aa6d
Disable buffering and add byte[] handling
captainsafia Feb 26, 2025
ff67f8d
Merge branch 'main' of https://github.com/dotnet/aspnetcore into cs/m…
captainsafia Mar 3, 2025
b0c76b8
Rely on SSE in shared framework
captainsafia Mar 3, 2025
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
Prev Previous commit
Next Next commit
Address feedback
  • Loading branch information
@captainsafia
captainsafia committed Feb 26, 2025
commit de869209f614ea1cdcbe243df2b09bf535a522e4
2 changes: 1 addition & 1 deletion eng/SharedFramework.External.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
<ExternalAspNetCoreAppReference Include="Microsoft.Extensions.Options.DataAnnotations" Version="$(MicrosoftExtensionsOptionsDataAnnotationsVersion)" />
<ExternalAspNetCoreAppReference Include="Microsoft.Extensions.Options" Version="$(MicrosoftExtensionsOptionsVersion)" />
<ExternalAspNetCoreAppReference Include="Microsoft.Extensions.Primitives" Version="$(MicrosoftExtensionsPrimitivesVersion)" />
<ExternalAspNetCoreAppReference Include="System.Net.ServerSentEvents" Version="$(SystemNextServerSentEvents)" />
<ExternalAspNetCoreAppReference Include="System.Net.ServerSentEvents" Version="$(SystemNetServerSentEvents)" />
<ExternalAspNetCoreAppReference Include="System.Security.Cryptography.Xml" Version="$(SystemSecurityCryptographyXmlVersion)" />
<ExternalAspNetCoreAppReference Include="System.Threading.RateLimiting" Version="$(SystemThreadingRateLimitingVersion)" />

Expand Down
2 changes: 2 additions & 0 deletions src/Framework/test/TestData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ static TestData()
"Microsoft.Net.Http.Headers",
"System.Diagnostics.EventLog",
"System.Diagnostics.EventLog.Messages",
"System.Net.ServerSentEvents",
"System.Security.Cryptography.Pkcs",
"System.Security.Cryptography.Xml",
"System.Threading.RateLimiting",
Expand Down Expand Up @@ -305,6 +306,7 @@ static TestData()
{ "Microsoft.JSInterop" },
{ "Microsoft.Net.Http.Headers" },
{ "System.Diagnostics.EventLog" },
{ "System.Net.ServerSentEvents" },
{ "System.Security.Cryptography.Xml" },
{ "System.Threading.RateLimiting" },
};
Expand Down
6 changes: 3 additions & 3 deletions src/Http/Http.Results/src/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ 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!>! value, string? eventType = null) -> Microsoft.AspNetCore.Http.IResult!
static Microsoft.AspNetCore.Http.Results.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<System.Net.ServerSentEvents.SseItem<T>>! value) -> Microsoft.AspNetCore.Http.IResult!
static Microsoft.AspNetCore.Http.Results.ServerSentEvents<T>(System.Collections.Generic.IAsyncEnumerable<T>! value, string? eventType = null) -> Microsoft.AspNetCore.Http.IResult!
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>!
26 changes: 17 additions & 9 deletions src/Http/Http.Results/src/Results.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -982,37 +982,45 @@ public static IResult AcceptedAtRoute<TValue>(string? routeName, RouteValueDicti
/// <summary>
/// Produces a <see cref="ServerSentEventsResult{TValue}"/> response.
/// </summary>
/// <param name="value">The value to be included in the HTTP response body.</param>
/// <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> value, string? eventType = null)
public static IResult ServerSentEvents(IAsyncEnumerable<string> values, string? eventType = null)
Copy link
Member

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 accepts IAsyncEnumerable<SseItem<string>>?

Copy link
Member

@eiriktsarpalis eiriktsarpalis Feb 26, 2025 •
edited
Loading

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.

#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
=> new ServerSentEventsResult<string>(value, eventType);
=> 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>
/// <param name="value">The value to be included in the HTTP response body.</param>
/// <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.
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to expose an option overriding this behavior?

Copy link
Member Author

Choose a reason for hiding this comment

The 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 Action<Sse<T>, IBufferWriter> argument to these overloads in the event we hear feedback on it.

/// 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> value, string? eventType = null)
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>(value, eventType);
=> 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="value">The value to be included in the HTTP response body.</param>
/// <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>
public static IResult ServerSentEvents<T>(IAsyncEnumerable<SseItem<T>> value)
=> new ServerSentEventsResult<T>(value);
/// <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.
Expand Down
29 changes: 16 additions & 13 deletions src/Http/Http.Results/src/ServerSentEventsResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Buffers;
using System.Net.ServerSentEvents;
using System.Text;
using Microsoft.AspNetCore.Http.Metadata;
using System.Reflection;
using Microsoft.AspNetCore.Builder;
Expand Down Expand Up @@ -41,29 +40,33 @@ public async Task ExecuteAsync(HttpContext httpContext)
ArgumentNullException.ThrowIfNull(httpContext);

httpContext.Response.ContentType = "text/event-stream";
Copy link
Member

Choose a reason for hiding this comment

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

Consider following what we do in SignalR
Lines 33-41
https://source.dot.net/#Microsoft.AspNetCore.Http.Connections/Internal/Transports/ServerSentEventsServerTransport.cs,33

Copy link
Member

Choose a reason for hiding this comment

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

Oh, and line 47 if that's something we care about.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 SseFormatter should be doing? Although admittedly it's a little gross to hardcode workarounds for buggy clients.

Maybe we leave it out for now and see what kind of feedback we get?

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to disable buffering?

Copy link
Member

Choose a reason for hiding this comment

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

That seems like something the SseFormatter should be doing?

What is that?

Copy link
Member

@davidfowl davidfowl Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The 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?)

Copy link
Member Author

Choose a reason for hiding this comment

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

That seems like something the SseFormatter should be doing?

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";

await SseFormatter.WriteAsync(_events, httpContext.Response.Body,
(item, writer) => FormatSseItem(item, writer, httpContext),
httpContext.RequestAborted);
}
var jsonOptions = httpContext.RequestServices.GetService<IOptions<JsonOptions>>()?.Value ?? new JsonOptions();

private static void FormatSseItem(SseItem<T> item, IBufferWriter<byte> writer, HttpContext httpContext)
{
// Emit string and null values as-is
if (item.Data is string stringData)
// 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)
Copy link
Member

Choose a reason for hiding this comment

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

What about byte[]?

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

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

but it might make more sense if we exposed that using the IBufferWriter callback as with the underlying library.

This seems like a good proposal. In the meantime, we can add special handling for byte arrays in our FormatSseItem callback.

{
writer.Write(Encoding.UTF8.GetBytes(stringData));
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);
}

private static void FormatSseItem(SseItem<T> item, IBufferWriter<byte> writer, JsonOptions jsonOptions)
{
if (item.Data is null)
{
writer.Write([]);
return;
}

// For non-string types, use JSON serialization with options from DI
var jsonOptions = httpContext.RequestServices.GetService<IOptions<JsonOptions>>()?.Value ?? new JsonOptions();
var runtimeType = item.Data.GetType();
var jsonTypeInfo = jsonOptions.SerializerOptions.GetTypeInfo(typeof(T));

Expand All @@ -72,8 +75,8 @@ private static void FormatSseItem(SseItem<T> item, IBufferWriter<byte> writer, H
? jsonTypeInfo
: jsonOptions.SerializerOptions.GetTypeInfo(typeof(object));

var json = JsonSerializer.Serialize(item.Data, typeInfo);
writer.Write(Encoding.UTF8.GetBytes(json));
var json = JsonSerializer.SerializeToUtf8Bytes(item.Data, typeInfo);
writer.Write(json);
}

private static async IAsyncEnumerable<SseItem<T>> WrapEvents(IAsyncEnumerable<T> events, string? eventType = null)
Expand Down
14 changes: 11 additions & 3 deletions src/Http/Http.Results/src/TypedResults.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@ public static AcceptedAtRoute<TValue> AcceptedAtRoute<TValue>(TValue? value, str
/// <summary>
/// Produces a <see cref="ServerSentEventsResult{TValue}"/> response.
/// </summary>
/// <param name="values">The value to be included in the HTTP response body.</param>
/// <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>
Expand All @@ -1087,9 +1087,13 @@ public static ServerSentEventsResult<string> ServerSentEvents(IAsyncEnumerable<s
/// 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 value to be included in the HTTP response body.</param>
/// <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.
/// 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 ServerSentEventsResult<T> ServerSentEvents<T>(IAsyncEnumerable<T> values, string? eventType = null)
#pragma warning restore RS0026 // Do not add multiple public overloads with optional parameters
Expand All @@ -1099,8 +1103,12 @@ public static ServerSentEventsResult<T> ServerSentEvents<T>(IAsyncEnumerable<T>
/// 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 value to be included in the HTTP response body.</param>
/// <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 ServerSentEventsResult<T> ServerSentEvents<T>(IAsyncEnumerable<SseItem<T>> values)
=> new(values);

Expand Down
59 changes: 53 additions & 6 deletions src/Http/Http.Results/test/ServerSentEventsResultTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Net.ServerSentEvents;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Text;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http.Json;
Expand All @@ -16,7 +17,7 @@ namespace Microsoft.AspNetCore.Http.HttpResults;
public class ServerSentEventsResultTests
{
[Fact]
public async Task ExecuteAsync_SetsContentType()
public async Task ExecuteAsync_SetsContentTypeAndHeaders()
{
// Arrange
var httpContext = GetHttpContext();
Expand All @@ -28,6 +29,8 @@ public async Task ExecuteAsync_SetsContentType()

// Assert
Assert.Equal("text/event-stream", httpContext.Response.ContentType);
Assert.Equal("no-cache,no-store", httpContext.Response.Headers.CacheControl);
Assert.Equal("no-cache", httpContext.Response.Headers.Pragma);
}

[Fact]
Expand All @@ -52,20 +55,20 @@ public async Task ExecuteAsync_WritesStringEventsToResponse()
}

[Fact]
public async Task ExecuteAsync_WritesStringsEventsWithType()
public async Task ExecuteAsync_WritesStringsEventsWithEventType()
{
// Arrange
var httpContext = GetHttpContext();
var events = new[] { "event1" }.ToAsyncEnumerable();
var events = new[] { "event1", "event2" }.ToAsyncEnumerable();
var result = TypedResults.ServerSentEvents(events, "test-event");

// Act
await result.ExecuteAsync(httpContext);

// Assert
var responseBody = Encoding.UTF8.GetString(((MemoryStream)httpContext.Response.Body).ToArray());
Assert.Contains("event: test-event\n", responseBody);
Assert.Contains("data: event1\n\n", responseBody);
Assert.Contains("event: test-event\ndata: event1\n\n", responseBody);
Assert.Contains("event: test-event\ndata: event2\n\n", responseBody);
}

[Fact]
Expand Down Expand Up @@ -207,7 +210,51 @@ public async Task ExecuteAsync_WithPolymorphicType_SerializesCorrectly()

// Assert
var responseBody = Encoding.UTF8.GetString(((MemoryStream)httpContext.Response.Body).ToArray());
Assert.Contains(@"""extra"":""Additional""", responseBody);
Assert.Contains(@"data: {""extra"":""Additional"",""name"":""Test"",""value"":42}", responseBody);
}

[Fact]
public async Task ExecuteAsync_ObservesCancellationViaRequestAborted()
{
// Arrange
var cts = new CancellationTokenSource();
var httpContext = GetHttpContext();
httpContext.RequestAborted = cts.Token;
var firstEventReceived = new TaskCompletionSource();
var secondEventAttempted = new TaskCompletionSource();

var events = GetEvents(cts.Token);
var result = TypedResults.ServerSentEvents(events);

// Act & Assert
var executeTask = result.ExecuteAsync(httpContext);

// Wait for first event to be processed then cancel the request and wait
// to observe the cancellation
await firstEventReceived.Task;
cts.Cancel();
await secondEventAttempted.Task;

// Verify the execution was cancelled and only the first event was written
await Assert.ThrowsAsync<TaskCanceledException>(() => executeTask);
var responseBody = Encoding.UTF8.GetString(((MemoryStream)httpContext.Response.Body).ToArray());
Assert.Contains("data: event1\n\n", responseBody);
Assert.DoesNotContain("data: event2\n\n", responseBody);

async IAsyncEnumerable<string> GetEvents([EnumeratorCancellation] CancellationToken cancellationToken = default)
{
try
{
yield return "event1";
firstEventReceived.SetResult();
await Task.Delay(1, cancellationToken);
yield return "event2";
}
finally
{
secondEventAttempted.SetResult();
}
}
}

private static void PopulateMetadata<TResult>(MethodInfo method, EndpointBuilder builder)
Expand Down
Loading