Matthew
Matthew

Reputation: 25763

Task ContinueWith does not return expected value

This is probably one of those RTFM type questions, however, I cannot for the life of me figure out how chain tasks together in a way that works for Asp.net WebApi.

Specifically, I am looking how to use a DelegatingHandler to modify the response after it has been handled by the controller (add an additional header), and I am trying to unit test the DelegatingHandler by using a HttpMessageInvoker instance.

First Pattern

[TestMethod]
public void FirstTest()
{
    var task = new Task<string>(() => "Foo");

    task.ContinueWith(t => "Bar");

    task.Start();

    Assert.AreEqual("Bar", task.Result);
}

This fails on the assert because task.Result returns "Foo"

Second Pattern

[TestMethod]
public void SecondTest()
{
    var task = new Task<string>(() => "Foo");

    var continueTask = task.ContinueWith(t => "Bar");

    continueTask.Start();

    Assert.AreEqual("Bar", continueTask.Result);
}

This fails on continueTask.Start() with the exception of System.InvalidOperationException: Start may not be called on a continuation task.

Third Pattern

[TestMethod]
public void ThirdTest()
{
    var task = new Task<string>(() => "Foo");

    var continueTask = task.ContinueWith(t => "Bar");

    task.Start();

    Assert.AreEqual("Bar", continueTask.Result);
}

This test works the way I expect, however, I am not sure how to get this pattern to work with WebAPI.

Current Implementation

public class BasicAuthenticationHandler : DelegatingHandler
{
    protected override Task<HttpResponseMessage> SendAsync(
        HttpRequestMessage request, CancellationToken cancellationToken)
    {
        var task = base.SendAsync(request, cancellationToken);

        task.ContinueWith(AddWwwAuthenticateHeaderTask());

        return task;
    }

    private static Func<Task<HttpResponseMessage>, HttpResponseMessage> 
        AddWwwAuthenticateHeaderTask()
    {
        return task =>
        {
            var response = task.Result;

            if (response.StatusCode == HttpStatusCode.Unauthorized)
            {
                response.Headers.WwwAuthenticate.Add(
                    new AuthenticationHeaderValue("Basic", "realm=\"api\""));
            }

            return response;
        };
    }
}

However, when I invoke BasicAuthenticationHandler from a unit test, my header is not added before the Assert happens (if I debug, I notice that the header is added after the unit test fails).

[TestMethod]
public void should_give_WWWAuthenticate_header_if_authentication_is_missing()
{
    using (var sut = new BasicAuthenticationHandler())
    {
        sut.InnerHandler = new DelegatingHttpMessageHandler(
            () => new HttpResponseMessage(HttpStatusCode.Unauthorized));

        using (var invoker = new HttpMessageInvoker(sut))
        {
            var task = invoker.SendAsync(_requestMessage, CancellationToken.None);

            task.Start();

            Assert.IsTrue(
                task.Result.Headers.WwwAuthenticate.Contains(
                    new AuthenticationHeaderValue("Basic", "realm=\"api\"")));
        }
    }
}

If I change my production code to return the continuation task instead of the result from base.SendAsync then I get the 2nd unit test exception about calling Start on a continuation task.

I think I want to accomplish the third unit test pattern in my production code, however, I have no idea on how to write that.

How do I do what I want (add the header before the assert gets called)?

Upvotes: 2

Views: 1498

Answers (4)

noseratio
noseratio

Reputation: 61676

Try the following. Note task2.Unwrap(), I think this part hasn't been addressed by the other answers:

protected override Task<HttpResponseMessage> SendAsync(
    HttpRequestMessage request, CancellationToken cancellationToken)
{
    var task1 = base.SendAsync(request, cancellationToken);

    var task2 = task1.ContinueWith(t => AddWwwAuthenticateHeaderTask(),
        cancellationToken);

    return task2.Unwrap();
}

You need to unwrap the inner task because the type of task2 is Task<Task<HttpResponseMessage>>. This should provide correct continuation semantic and result propagation.

Check Stephen Toub's "Processing Sequences of Asynchronous Operations with Tasks". This complexity can be avoided with async/await.

If you can't use async/await, you still can further improve this code a bit, to avoid redundant thread switching otherwise caused by ContinueWith:

var task2 = task1.ContinueWith(
    t => AddWwwAuthenticateHeaderTask(), 
    cancellationToken,
    TaskContinuationOptions.ExecuteSynchronously,
    TaskScheduler.Default);

In either case, the continuation will not happen on the original synchronization context (which probably is AspNetSynhronizationContext). If you need to stay on the same context, use TaskScheduler.FromCurrentSynchronizationContext() instead of TaskScheduler.Default. A word of caution: this may cause a deadlock in ASP.NET.

Upvotes: 3

Yuval Itzchakov
Yuval Itzchakov

Reputation: 149538

When returning a Task, it should always be a Hot Task, meaning that the returned task has already been started. Making someone explicitly call Start() on a returned task is confusing and violates the guidelines.

To properly see the continuations result, do this:

protected override Task<HttpResponseMessage> SendAsync(
    HttpRequestMessage request, CancellationToken cancellationToken)
{
    return base.SendAsync(request, cancellationToken).ContinueWith(AddWwwAuthenticateHeaderTask()).Unwrap();
}

And you should modify base.SendAsync to return the already started Task

From the Task Asynchronous Pattern Guidelines:

All tasks returned from TAP methods must be “hot.” If a TAP method internally uses a Task’s constructor to instantiate the task to be returned, the TAP method must call Start on the Task object prior to returning it. Consumers of a TAP method may safely assume that the returned task is “hot,” and should not attempt to call Start on any Task returned from a TAP method. Calling Start on a “hot” task will result in an InvalidOperationException (this check is handled automatically by the Task class).

Upvotes: 4

Stephen Cleary
Stephen Cleary

Reputation: 456507

I cannot for the life of me figure out how chain tasks together in a way that works for Asp.net WebApi.

Embrace async and await. In particular, replace ContinueWith with await (and do not use the task constructor or Start):

protected override async Task<HttpResponseMessage> SendAsync(
    HttpRequestMessage request, CancellationToken cancellationToken)
{
    var response = await base.SendAsync(request, cancellationToken);
    if (response.StatusCode == HttpStatusCode.Unauthorized)
    {
        response.Headers.WwwAuthenticate.Add(
            new AuthenticationHeaderValue("Basic", "realm=\"api\""));
    }

    return response;
}

Upvotes: 3

Lee
Lee

Reputation: 144136

ContinueWith returns the mapped task so you need to return that:

var task = base.SendAsync(request, cancellationToken);
return task.ContinueWith(AddWwwAuthenticateHeaderTask());

Upvotes: 1

Related Questions