How to gracefully disconnect a client?

[ye4241]

As in #50 and #39 , is there any graceful solution to disconnect client as OpenAI does. It will not show response error in postman. However with current library, it will display "Error: read ECONNRESET".

OpenAI

Lib.AspNetCore.ServerSentEvents

Sample code:

    public override async Task OnConnectAsync(HttpRequest request, IServerSentEventsClient client)
    {
        await base.OnConnectAsync(request, client);
        await client.SendEventAsync("hello");
        await client.SendEventAsync("[DONE]");
        await Task.Delay(500);
        client.Disconnect();
    }

I have tried to get _response call the CompleteAsync method, client will disconnect gracefully.

    public override async Task OnConnectAsync(HttpRequest request, IServerSentEventsClient client)
    {
        await base.OnConnectAsync(request, client);
        // ... some sends
        var response = (HttpResponse)client.GetType().GetField("_response", BindingFlags.NonPublic | BindingFlags.Instance)?.GetValue(client);
        await response.Body.FlushAsync();
        await response.CompleteAsync();
    }

[tpeczek]

Hi @ye4241,

The truth is that CompleteAsync wasn't always there and the current implementation is based on HttpContext.Abort for simplicity. Once it become available I wasn't actively chasing adopting because it leads to a breaking change and they way the connection ends doesn't have impact on browser scenarios (as browser, in line with the spec, will attempt to reconnect anyway which is what #39 aimed at solving).

That said, I recognize that non-browser adoption of SSE is growing and the approach to follow up any server-side disconnect with a reconnect is not always followed. This makes the aspect of graceful shutdown of network connection more important, so I guess it's time to bite the bullet.

I see your pull request, thank you for the head start. I might be able to take it further later today and if not I'll do it after Christmas.

[ye4241]

@tpeczek
Indeed, the current implementation is too simple, and there are occasional cases of forced disconnection or inability to reconnect. At present, it is still a broadly usable solution.
I'm still looking forward to a more comprehensive solution in the end.

[tpeczek]

One side comment to your sample code @ye4241. You are disconnecting as part of OnConnectAsync which is not a supported scenario. The OnConnectAsync (or IServerSentEventsService.ClientConnected) happens as part of the handshake, not after it's completed. By attempting to disconnect at this stage, you are essentially triggering a race condition which may result in failed cleanup and side effects.

I will probably minimise this as part of the current disconnect modifications, but without guarantee of making such scenario a supported one.