SignalR Core StartAsync() CancellationToken not respected?

[vflame]

Seems that the AspNetCore Signalr Client doesn't respect the cancellationtoken passed into the StartAsync() method. It will throw appropriately if the CTS timeout is sufficiently short (e.g., 1 millisecond);

Using this version of ASP.NET Core 3 Preview 5

var cts = new CancellationTokenSource(TimeSpan.FromSeconds(1));
           var _hubConnection = new HubConnectionBuilder()
                .WithUrl(".WithUrl("http://slowwly.robertomurray.co.uk/delay/5000/url/http://www.google.co.uk")") //artificially slow, response times of ~5 seconds before a 404 is returned
                .Build();
           await _hubConnection.StartAsync(cts.Token);

Expected behavior

StartAsync() should throw an OperationCancelledException after 1 second. Any negotiation or handshake should be aborted if it was not already successfully completed within 1 second.

Current behavior

StartAsync() will wait until an HttpRequestException (in this case, a 404 after 5 seconds) is thrown or the connection succeeds. An OperationCancelledException is never thrown.

[BalintFarkas]

We have encountered the same issue in the .NET Core 2.1 version (1.0.4) as well. It appears that the CancellationToken is simply not passed all the way down to the HttpClient that is doing the actual work. We have developed a fix and will be posting a pull request shortly.

[BalintFarkas]

We have posted the fix for the latest .NET Core 2.1 version (1.0.4) in this repository: https://github.com/visionary-broadcast-engineering/SignalR/tree/hubconnection-startasync-respect-cancellationtoken (it currently works if you don't skip negotiation)

We're working on creating it for the .NET Core 2.2 (1.1.0) and 3.0 versions and will submit pull requests for those in the Microsoft repo.

[mikaelm12]

Good point. I looked at your commits @BalintFarkas and it does seem as if the connection token passed into the HttpConnection doesn't get passed through. Good find

[mikaelm12]

@BalintFarkas Since you already have the fix in your fork this issue seems like a great candidate for a community contribution 😄 . Would you be interested?

[BalintFarkas]

Hey @mikaelm12 -

sure, we'd be delighted to! We have already created fixes for 1.1.0 and 1.0.4, and will create one for the current 3.x version tomorrow with my colleague @danielloczi, then submit PRs. Would that be acceptable?

[mikaelm12]

Sounds great! It's much appreciated!

[BalintFarkas]

Awesome, we'll get back to you in a few hours (it's midnight in my timezone :).

[mikaelm12]

Also just to be clear, you only have to make the fix for the current master branch (this is 3.0 for us). We will handle back porting to other branches if we deem it necessary.

[BalintFarkas]

The above PR contains the changes for the .NET Core 3.0 version. If anyone needs them for .NET Core 2.1 (1.0.4) or 2.2 (1.1.0), they're available in our repo.

1.0.4: https://github.com/visionary-broadcast-engineering/SignalR/tree/hubconnection-startasync-respect-cancellationtoken

1.1.0: https://github.com/visionary-broadcast-engineering/SignalR/tree/hubconnection-startasync-respect-cancellationtoken-netcore22