Require CancellationToken for client results (and fix cancel closing connection)

[BrennanConroy]

Fixes #43054

For other types like StreamItem and Invocations we create StreamBindingFailureMessage and InvocationBindingFailureMessage respectively in the hub protocols when a failure occurs. We might want to do something similar here, but since it's late in RC1 I think we should opt for not adding a new public class and instead ignore the result value of completion messages in this scenario.

Additionally, this change adds a default timeout of 30 seconds to client result calls. This is done at the HubLifetimeManager layer because we aren't able to control all call sites of proxy.InvokeAsync. This is also an option that could potentially result in new API (DefaultClientResultTimeout or something). But again, late in RC1 that might be hard to swing. Users can pass in their own CancellationToken if they want to customize how long they will wait for a result, and this will override the 30 second default.

Went with requiring user to pass in a CancellationToken to suggest that they think about cancellation when using the feature.

[BrennanConroy]

Changed to remove the default timeout and require passing in a CancellationToken.

Edit: This would need to go through a quick API review if we agree on the approach.

[halter73]

I think it makes sense to try to force people to think about cancellation. Making them explicitly pass in CancellationToken.None/default seems like a reasonable way to do this. And we could add a default later if we really wanted to, right?

Anyway, let's update #43054 with the API proposal so more people can discuss.

[davidfowl]

And we could add a default later if we really wanted to, right?

With the current API I don't think so.

[BrennanConroy]

And we could add a default later if we really wanted to, right?

With the current API I don't think so.

We could add an option to set a default, but our default would need to be infinite 😄

[BrennanConroy]

@dotnet/aspnet-admins could I get a merge please? Code check is failing due to approved unshipped file changes.

[dougbu]

@adityamandaleeka is this approved for RC1❔

[adityamandaleeka]

Approved, this prevents a potentially bad disconnect issue in new 7.0 work.

[dougbu]

Will complain the auto-squish didn't work and that'll hopefully remind me to merge this 😄

[BrennanConroy]

Ok, now it's ready.

[adityamandaleeka]

Looks like @dougbu or @wtgodbe have to merge this since it's technically red.

[dougbu]

You could have done the JIT thing @adityamandaleeka but pinging me was fine, especially because my thought that auto-squish would complain when a build failed didn't pan out😀

[adityamandaleeka]

That's a big hammer and I want to be sparing in my use of it 😄