Request is not async

Question

public class RollingRequests
    {
        private const int DefaultNumSimultaneousRequests = 10;
        private readonly HttpClient _client; // Don't worry about disposing see https://stackoverflow.com/questions/15705092/do-httpclient-and-httpclienthandler-have-to-be-disposed
        private readonly HttpCompletionOption _httpCompletionOption;
        private readonly int _numSimultaneousRequests;

        public RollingRequests() : this(DefaultNumSimultaneousRequests)
        {
        }

        public RollingRequests(int windowSize) : this(new HttpClient(), windowSize)
        {
        }

        public RollingRequests(HttpClient client, int numSimultaneousRequests, HttpCompletionOption httpCompletionOption = HttpCompletionOption.ResponseContentRead)
        {
            _client = client;
            _numSimultaneousRequests = numSimultaneousRequests;
            _httpCompletionOption = httpCompletionOption;
        }

        public async Task ExecuteAsync(List<string> urls, CancellationToken cancellationToken, Action<HttpResponseHeaders, string> requestCallback = null)
        {
            var nextIndex = 0;
            var activeTasks = new List<Task<Tuple<string, HttpResponseMessage>>>();

            var startingIndex = Math.Min(_numSimultaneousRequests, urls.Count);
            for (nextIndex = 0; nextIndex < startingIndex; nextIndex++)
            {
                activeTasks.Add(RequestUrlAsync(urls[nextIndex], cancellationToken));
            }

            while (activeTasks.Count > 0)
            {
                var finishedTask = await Task.WhenAny(activeTasks).ConfigureAwait(false);
                activeTasks.Remove(finishedTask);

                var retryUrl = await ProcessTask(await finishedTask, requestCallback).ConfigureAwait(false);

                // If retrying, add the URL to the end of the queue
                if (retryUrl != null)
                {
                    urls.Add(retryUrl);
                }

                if (nextIndex < urls.Count)
                {
                    activeTasks.Add(RequestUrlAsync(urls[nextIndex], cancellationToken));
                    nextIndex++;
                }
            }
        }

        private async Task<string> ProcessTask(Tuple<string, HttpResponseMessage> result, Action<HttpResponseHeaders, string> requestCallback = null)
        {
            var url = result.Item1;
            using (var response = result.Item2)
            {
                if (!response.IsSuccessStatusCode)
                {
                    return url;
                }

                if (requestCallback != null)
                {
                    string content = null;
                    if (_httpCompletionOption == HttpCompletionOption.ResponseContentRead)
                    {
                        content = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
                    }

                    requestCallback(response.Headers, content);
                }

                return null;
            }
        }

        private async Task<Tuple<string, HttpResponseMessage>> RequestUrlAsync(string url, CancellationToken ct)
        {
            var response = await _client.GetAsync(url, _httpCompletionOption, ct).ConfigureAwait(false);
            return new Tuple<string, HttpResponseMessage>(url, response);
        }
    }

This is a class that allows for X simultaneous requests to be on-going at once. When I am unit-testing this class and I moq the HttpClient giving each request a 1 second delay the initial activeTasks.Add is taking 5 seconds if I have 5 requests, suggesting to me that RequestUrlAsync isn't truly async.

Can anyone spot the issue?

Edit: This is how I am sleeping the mocked client

        _messageHandlerMock
                .Protected()
                .Setup<Task<HttpResponseMessage>>(MethodToMoq, ItExpr.IsAny<HttpRequestMessage>(), ItExpr.IsAny<CancellationToken>())
                .Callback(() => Thread.Sleep(1000))
                .ReturnsAsync(callback)
                .Verifiable();

When using `WhenAny` you usually would not also await the Tasks that you start. You pull the result out of the "any" that finished. — Crowcoder, May 19 '19 at 18:27
Can you be more specific about where I have an erroneous `await` ? — CuriousDeveloper, May 19 '19 at 18:36
You are awaiting the tasks in `RequestUrlAsync`. I've never tried that to see what happens when you then do a `WaitAny` or `WaitAll` but it might be why you see evidence that it not async - though it is still async. I suggest you do not await in that method, instead get the result of the Task from `WhenAny`. — Crowcoder, May 19 '19 at 18:43
You have more problems though. You are calling WhenAny in a loop but that only returns the first completed task. You then await a task that is already RanToCompletion. What is this code supposed to be doing? — Crowcoder, May 19 '19 at 18:52
It is suppose GET a list of URLs while having X simultaneous requests running at once, once a request completes if it wasnt a 200, it gets readded to the queue, otherwise it starts opening the next URL. The WhenAny code comes from the Throttling example here https://learn.microsoft.com/en-us/dotnet/standard/asynchronous-programming-patterns/consuming-the-task-based-asynchronous-pattern — CuriousDeveloper, May 19 '19 at 19:04
Have you heard of the Polly framework? It simplifies what it sounds like you are trying to do. Notice in the example you link to there is no await on the tasks `GetBitmapAsync`. — Crowcoder, May 19 '19 at 19:12
How are you waiting 1 second in the mocked HttpClient? Are you waiting synchronously or asynchronously? If the former, it might explain the behavior you're seeing — Kevin Gosse, May 19 '19 at 19:31
@KevinGosse I may not be doing it right I am using a thread sleep, I am not sure how to do make it do an async Task.Delay — CuriousDeveloper, May 19 '19 at 19:53
@CuriousDeveloper something like `.Returns(() => Task.Delay(1000))` — Kevin Gosse, May 19 '19 at 20:55
@Crowcoder I dont see the implementation of `GetBitMapAsync` but in this example it is done exactly like I have it, where both are `await` https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/async/cancel-remaining-async-tasks-after-one-is-complete — CuriousDeveloper, May 19 '19 at 22:09
@CuriousDeveloper Then `.Returns(async () => { await Task.Delay(1000); return callback(); })` — Kevin Gosse, May 20 '19 at 09:05

score 1 · Accepted Answer · answered May 19 '19 at 22:11

I tested your class RollingRequests with actual urls and works as expected. Then I replaced await _client.GetAsync(... with await Task.Delay(1000) and continued working as expected. Then replaced the same line with Thread.Sleep(1000) and replicated your problem.

Moral lesson: avoid blocking the current thread when running asynchronous code!

score 0 · Answer 2 · answered May 19 '19 at 20:58

(It would be easier to answer if you had provided a Minimal, Reproducible Example)

Mixing Thread.Sleep with asynchronous code isn't a good idea because it is a blocking call.

Mocking internals should also be avoided.

Here's a simple example of a test that takes about 1 second to execute 10 requests:

async Task Test()
{
    var httpClient = new HttpClient(new TestHttpMessageHandler());

    var ticks = Environment.TickCount;

    await Task.WhenAll(Enumerable.Range(0, 10).Select(_ => httpClient.GetAsync("https://stackoverflow.com/")));

    Console.WriteLine($"{Environment.TickCount - ticks}ms");
}

class TestHttpMessageHandler : HttpMessageHandler
{
    protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        await Task.Delay(1000);

        return new HttpResponseMessage();
    }
}

Request is not async

2 Answers2