CancellationToken is not propagated to all async methods

[Jericho]

Some async methods in FluentHttlClient do not have access to a CancellationToken which prevents them from propagating this token when they invoke other async methods. For example:

internal static async Task<HttpRequestMessage> CloneAsync(this HttpRequestMessage request)

should look like this:

internal static async Task<HttpRequestMessage> CloneAsync(this HttpRequestMessage request, CancellationToken cancellationToken = default)

If you think this is a worthwhile improvement, I'll be happy to work on a PR.

[Jericho]

There are exceptions (of course!). When a method accepts a IRequest parameter, it doesn't need a separate cancellation token, it can simply reference the request's cancellation token like in this example:

protected virtual async Task<HttpResponseMessage> SendImplAsync(IRequest request)
{
    this.AssertNotDisposed();

    // clone request (to avoid issues when resending messages)
    HttpRequestMessage requestMessage = await request.Message.CloneAsync().ConfigureAwait(false);

    // dispatch request
    return await this.BaseClient
        .SendAsync(requestMessage, request.CancellationToken)
        .ConfigureAwait(false);
}

As you can see in this example, request.CancellationToken is propagated to this.BaseClient.SendAsyncso a CancellationToken parameter wouldn't be necessary in this case. Having said that, I notice that no cancellation token is propagated when CloneAsync is invoked. That would be something I would improve.

[Jericho]

Another exception:

 public abstract class MediaTypeFormatterBase : MediaTypeFormatter
 {
        ... snip ...

     public override Task<object> ReadFromStreamAsync(Type type, Stream stream, HttpContent content, IFormatterLogger formatterLogger)

As much as I would like to add a cancellation token to the ReadFromStreamAsync method, the signature is defined in a 3rd party nuget package which we can't modify.

[Jericho]

The following methods on IResponse would benefit from a CancellationToken parameter:

Task<T> As<T>();
Task<T[]> AsArray<T>();
Task<byte[]> AsByteArray();
Task<string> AsString();
Task<Stream> AsStream();
Task<JToken> AsRawJson();
Task<JObject> AsRawJsonObject();
Task<JArray> AsRawJsonArray();

[Jericho]

I went ahead and submitted a PR to show you the changes I propose.

[Pathoschild]

I'm fine with the idea in general, but most users will chain the request and response together. In that case setting the cancellation token again is a bit redundant:

var model = await client
    .GetAsync(...)
    .WithCancellationToken(cancellationToken)
    .As<T>(cancellationToken);

What do you think of:

1. passing the request's cancellation token to the response automatically;
2. and adding a response.WithCancellationToken method (in case you need a different one for the response parsing)?

[Jericho]

The As<T>() call in your code sample is in fact IRequest.As<T> (as opposed to IResponse.As<T>). This method does not have a cancellation token parameter and I did not modify the signature of this method.

I did however modify the body of this method to ensure that the cancellation token is propagated to the IResponse.As<T> like so:

public async Task<T> As<T>() // <-- notice the absence of cancellation token parameter
{
    IResponse response = await this.AsResponse().ConfigureAwait(false);
    return await response.As<T>(this.CancellationToken).ConfigureAwait(false); // <-- notice this.CancellationToken is automatically passed to response.As<T>
}

What do you think of:

1. passing the request's cancellation token to the response automatically;

I completely agree. In fact that's what my goal was. You can review the changes I made to methods such as IRequest.As<T>, IRequest.AsByteArray and IRequest.ToString and you will see that I am automatically passing the request's cancellation token to the corresponding response method. In other words:

// example of calls before I modified them:
return await response.As<T>().ConfigureAwait(false);
return await response.AsByteArray().ConfigureAwait(false);
return await response.AsString().ConfigureAwait(false);

// the same calls after I modified them:
return await response.As<T>(this.CancellationToken).ConfigureAwait(false);
return await response.AsByteArray(this.CancellationToken).ConfigureAwait(false);
return await response.AsString(this.CancellationToken).ConfigureAwait(false);

So I sincerely think that I already achieved what you are suggesting.

2. and adding a response.WithCancellationToken method (in case you need a different one for the response parsing)?

I will be gone tomorrow but I'll look into your second suggestion over the weekend. I kinda like it, it would be consistent with how we currently specify the cancellation token on the request.

[Pathoschild]

Yep, but IResponse is part of the public API too (see examples below), so the As* methods should be consistent between IRequest and IResponse. I'm suggesting we set the default cancellation token in Request.Execute:

IResponse response = new Response(responseMessage, this.Formatters, this.CancellationToken);

Then there's no need for As* to have different signatures:

current PR	proposed design
```cs // using IRequest T model = await client .GetAsync(...) .WithCancellationToken(cancellationToken) .As(); // using IResponse IResponse response = await client .GetAsync(...) .WithCancellationToken(cancellationToken); T model = await response .As(cancellationToken); // <-- repeated ```	```cs // using IRequest T model = await client .GetAsync(...) .WithCancellationToken(cancellationToken) .As(); // using IResponse IResponse response = await client .GetAsync(...) .WithCancellationToken(cancellationToken); T model = await response .As(); ```

And we can still have a different token for fetching & reading if we want:

IResponse response = await client
    .GetAsync(...)
    .WithCancellationToken(fetchToken);

T model = await response 
    .WithCancellationToken(readToken)
    .As<T>();

[Jericho]

My trip got cancelled today so I had time to look into your second suggestion. Turned out to be much simpler than I expected to replace the cancellation token on the IResponse async methods with WithCancellationToken.

[Pathoschild]

Done via #114 for the upcoming FluentHttpClient 4.3.0. Thanks!