Fable.SignalR icon indicating copy to clipboard operation
Fable.SignalR copied to clipboard

Support cancellation of streams created with StreamFrom in .NET client

Open kentcb opened this issue 4 years ago • 4 comments
trafficstars

This relates to the .NET client - I am not sure if it affects other clients.

Is your feature request related to a problem?

IHubConnection<...> has a StreamFrom that accepts a CancellationToken and forwards onto the SignalR hub's StreamAsync method, but HubConnection<...>.StreamFrom does not likewise accept a CancellationToken. This means clients cannot cancel streams.

Describe the solution you'd like

Cancellation should be exposed to clients, allowing them to proactively cancel a stream.

Describe alternatives you've considered

I've hacked this into my own copy of the code to prove it would work how I'd expect (how a raw SignalR stream works, basically). It required changes to both client and server.

kentcb avatar Sep 01 '21 06:09 kentcb

Hi @Shmew. Would you accept a PR that addresses this? I guess the biggest concern would be that it's a breaking change as the function signature for "stream from" would need to change.

kentcb avatar Sep 01 '21 09:09 kentcb

Hi @kentcb,

Yeah I'd accept a PR for this. I think it can be made non-breaking as it could be an optional parameter.

Shmew avatar Sep 04 '21 20:09 Shmew

@Shmew

Just looking at this now and realized I conflated two separate cancellation problems:

  1. .NET clients being able to cancel a stream established with HubConnection<...>.StreamFrom. This one is easy and non-breaking.
  2. .NET server implementation being able to detect the cancellation of a stream. The streamTo that gets passed into AddSignalR does not currently include a CT in its signature, meaning the server has no idea if the client has cancelled. This will be a breaking change, but as far as I can tell it is absolutely critical because otherwise streams could remain open indefinitely on the server.

I'll create a separate issue for point 2 because I believe they're independently addressable.

kentcb avatar Sep 14 '21 23:09 kentcb

Actually, the second issue is more subtle. There is a CT buried away in FableHub<...>.Context.ConnectionAborted, but it is only triggered if - as the name suggests - the connection itself is aborted. It isn't triggered if the client cancels the token passed in when establishing the stream, which means streams could be left open for a long time if the client is long-lived. Anyway, will continue to investigate and create a separate issue once I have my head around it.

kentcb avatar Sep 15 '21 01:09 kentcb