Cancellation Tokens on Async Methods

[Narnian12]

Description

• Optional cancellation tokens added to IAsyncResourceResolver async methods
  • Assigned to CancellationToken.None (default) by default
  • Propagates through all implementations
  • Most implementations accept it for API consistency, but do not use it
• Implemented async version of CommonWebResolver.TryResolveByUriAsync()
  • Only call that honors cancellation token by ultimately sending it to HttpClient.SendAsync()

Related issues

Resolves #3081.

Testing

• Added tests covering cancellation behavior on MultiResolver and WebResolver - wrapping CommonWebResolver with the async method honoring the token

FirelyTeam Checklist

• Update the title of the PR to be succinct and less than 50 characters
• Mark the PR with the label breaking change when this PR introduces breaking changes

[Narnian12]

I think this needs the breaking change label added. I am unable to add it - can someone else add that label to this PR?

[ewoutkramer]

Just one (minor) question about code duplication...

[ewoutkramer]

Should we implement this by calling the async version and blocking on it? I am trying to avoid the code duplication here....

[ewoutkramer]

About the breaking change....

I thought we could just add an optional param to the interface, but we cannot as existing clients would still break. But we can use default implementations:

interface X
{
	// New signature
	int Calculate(int a, int b, CancellationToken ct) =>
		Calculate(a,b);
	
	// Existing signature
	int Calculate(int a, int b);
}

We cannot do a breaking change until SDK7, which is probably almost 2 years out....

[Narnian12]

About the breaking change....

I thought we could just add an optional param to the interface, but we cannot as existing clients would still break. But we can use default implementations:

interface X
{
	// New signature
	int Calculate(int a, int b, CancellationToken ct) =>
		Calculate(a,b);
	
	// Existing signature
	int Calculate(int a, int b);
}

We cannot do a breaking change until SDK7, which is probably almost 2 years out....

That makes sense, thanks! I can abandon this PR and make another one that overloads the original methods, so we don't have breaking changes. Or if there are more pressing issues to contend with, I can take a look at those as well.

[andrzejskowronski]

I think it's fine to continue here, you'll just need to restore the original interface method and use default implementation for the new one that calls the original - and maybe some adjustments to implementers if necessary, maybe not :)