Skip to content

Conversation

@carldebilly
Copy link
Owner

@carldebilly carldebilly commented Sep 6, 2025

This PR adds comprehensive Server-Sent Events (SSE) support to the Yllibed.HttpServer library. The implementation includes SSE session management, helper utilities, content negotiation, and streaming response infrastructure. The PR also modernizes the project structure by introducing central package management, updating test frameworks, and adding new solution file format support.

  • Adds complete SSE implementation with SseSession, SseHandler, and helper classes
  • Introduces streaming response infrastructure to support SSE and other streaming scenarios
  • Modernizes build system with central package management and updated dependencies

@carldebilly carldebilly requested a review from Copilot September 6, 2025 04:28
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive Server-Sent Events (SSE) support to the Yllibed.HttpServer library. The implementation includes SSE session management, helper utilities, content negotiation, and streaming response infrastructure. The PR also modernizes the project structure by introducing central package management, updating test frameworks, and adding new solution file format support.

  • Adds complete SSE implementation with SseSession, SseHandler, and helper classes
  • Introduces streaming response infrastructure to support SSE and other streaming scenarios
  • Modernizes build system with central package management and updated dependencies

Reviewed Changes

Copilot reviewed 49 out of 51 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Yllibed.HttpServer/Sse/*.cs New SSE implementation classes for sessions, handlers, and helpers
Yllibed.HttpServer/Server.HttpServerRequest.cs Added streaming response support and modernized async patterns
Yllibed.HttpServer/IHttpServerRequest.cs Added SetStreamingResponse method signature
Yllibed.HttpServer/Extensions/*.cs New Accept header validation and modernized extension methods
Yllibed.HttpServer/Helpers/AcceptHeaderHelper.cs New Accept header parsing according to HTTP specs
Test files Comprehensive SSE test coverage and modernized test infrastructure
Project files Central package management, updated dependencies, and solution modernization

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

var headers = Request.Headers;
if (headers != null && headers.TryGetValue("Last-Event-ID", out var values))
{
foreach (var v in values) { _lastEventId = v?.Trim(); break; }
Copy link

Copilot AI Sep 6, 2025

Choose a reason for hiding this comment

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

This loop only uses the first value and then breaks immediately. Consider using FirstOrDefault() for clarity: _lastEventId = values.FirstOrDefault()?.Trim();

Copilot uses AI. Check for mistakes.
#pragma warning disable MA0040

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static Task FlushAsync(this TextWriter writer, CancellationToken ct = default) => writer.FlushAsync();
Copy link

Copilot AI Sep 6, 2025

Choose a reason for hiding this comment

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

The FlushAsync method ignores the cancellation token parameter. In .NET Standard 2.0, TextWriter.FlushAsync() doesn't accept a CancellationToken, but the extension method should acknowledge this limitation or throw if the token is already cancelled.

Suggested change
public static Task FlushAsync(this TextWriter writer, CancellationToken ct = default) => writer.FlushAsync();
public static Task FlushAsync(this TextWriter writer, CancellationToken ct = default)
{
if (ct.IsCancellationRequested)
return Task.FromCanceled(ct);
return writer.FlushAsync();
}

Copilot uses AI. Check for mistakes.
public static Task FlushAsync(this TextWriter writer, CancellationToken ct = default) => writer.FlushAsync();

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static Task<string> ReadLineAsync(this TextReader reader, CancellationToken ct = default) => reader.ReadLineAsync();
Copy link

Copilot AI Sep 6, 2025

Choose a reason for hiding this comment

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

Similar to FlushAsync, this ReadLineAsync method ignores the cancellation token parameter. The .NET Standard 2.0 version doesn't support CancellationToken, but the method signature suggests it should be honored.

Suggested change
public static Task<string> ReadLineAsync(this TextReader reader, CancellationToken ct = default) => reader.ReadLineAsync();
public static Task<string> ReadLineAsync(this TextReader reader, CancellationToken ct = default)
{
if (ct.IsCancellationRequested)
return Task.FromCanceled<string>(ct);
return reader.ReadLineAsync();
}

Copilot uses AI. Check for mistakes.
@carldebilly carldebilly merged commit 3307cff into master Sep 6, 2025
2 checks passed
@carldebilly carldebilly deleted the dev/sse-support branch September 6, 2025 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants