3

I've been banging my head against a wall for two days now, and frankly I'm annoyed with myself because I just can't seem to get it.

I'm in a webapi context. During this request I need to send some data to one of our other systems, this system is slow to return, due to heavy calculations and multiple database saves etc etc. I need to log the result of this operation, regardless of whether it is successful or not. But I don't want to wait around for it to finish.

I've read that I should be async await all the way from top to bottom. I would have to convert numerous methods if I decided to do this, as I'm already 3 or 4 methods deep, which I fear would branch out even more.

What are my options here? If I go async await all the way down, what do I do with the methods higher up the stack, like my WebApi controllers?

Here is my code, I've tried to thin it down as much as I can. Right now I'm using Task.Result() in the method PushResult(). Which to my understanding is blocking the async? This code works in that the request gets sent. But the TestLog is always last, not first. Therefore not async.

    //I'm in a public service and referenced twice
    private void MyEndProcess()
    {
        // other stuff

        _vendorPushService.PushResult(); // This could take a while and I have to wait for it!

        _logService.PostLog(LogType.TestLog, "Test");
    }

    //I'm referenced above and somewhere else in the code base
    public void PushResult()
    {   
        ExternalResultModel externalResultModel = _resultService.GetExternalResultModel();

        PushedResultModel pushedResult = new PushedResultModel();

        try
        {
            pushedResult = _vendorRequestService.PushResultAsync(externalResultModel).Result;
        }
        catch (Exception ex)
        {
            pushedResult.Success = false;
        }

        if (pushedResult.Success)
        {
            _logService.PostLog(LogType.SuccessLog, pushedResult.Message);
        }
        else
        {
            _logService.PostLog(LogType.FailedLog, pushedResult.Message);
        }
    }

    public async Task<PushedResultModel> PushResultAsync(ExternalResultModel externalResultModel)
    {
        // setup the requestMessage
        HttpResponseMessage responseMessage = await _httpRequestService
            .SendRequest(requestMessage)
            .ConfigureAwait(false);

        return new PushedResultModel
        {
            Success = responseMessage.IsSuccessStatusCode,
            Message = await responseMessage.Content.ReadAsStringAsync()
        };
    }

    public class HttpRequestService : IHttpRequestService
    {
        private readonly HttpClient _httpClient; 

        public HttpRequestService(IHttpClientAccessor httpClientAccessor)
        {
            _httpClient = httpClientAccessor.HttpClient;
        }

        public async Task<HttpResponseMessage> SendRequest(HttpRequestMessage requestMessage)
        {
            HttpResponseMessage httpResponseMessage = await _httpClient.SendAsync(requestMessage).ConfigureAwait(false);

            return httpResponseMessage;
        }
    }
Jack Pettinger
  • 2,715
  • 1
  • 23
  • 37
  • 3
    If you're not willing to async all the way up, then just leave everything synchronous and don't put any asynchronous code in there. You'll only cause *problems* by adding some asynchronous code without making the whole call stack asynchronous. – Servy Dec 12 '17 at 14:34
  • 2
    If you don't want to wait around for it to finish, then this is essentially a fire-and-forget scenario. This has it's own issues in a web api context. I'd recommend having a look here: http://blog.stephencleary.com/2014/06/fire-and-forget-on-asp-net.html. In terms of answering your actual question, you might want to have a look at [`ContinueWith`](https://msdn.microsoft.com/en-us/library/dd321405(v=vs.110).aspx), but do read Stephen's post first. – bornfromanegg Dec 12 '17 at 14:41
  • 3
    If you need to start background operation and not wait for it to finish before returning response to client - async await will not be of much help. – Evk Dec 12 '17 at 14:54
  • @Evk Can you expand on that? I need to log the result, regardless of success/failure. But I don't want the user to have to wait for it to finish. – Jack Pettinger Dec 12 '17 at 15:05
  • 1
    Then read link above about fire and forget jobs in asp.net – Evk Dec 12 '17 at 15:07
  • @JackPettinger How long is slow Greater or less than 90 Seconds? – johnny 5 Jul 19 '18 at 16:28
  • if it's going to be less than 90 seconds you can use the new [BackgroundWorkerQueue](https://blog.mariusschulz.com/2014/05/07/scheduling-background-jobs-from-an-asp-net-application-in-net-4-5-2) – johnny 5 Jul 19 '18 at 16:31

2 Answers2

0

You should implement async await all the way from top to bottom.

If I go async await all the way down, what do I do with the methods higher up the stack, like my WebApi controllers?

Just make your controller actions async like this:

[RoutePrefix("api")]
public class PresidentsController : ApiController
{
    [Route("presidents")]
    public async Task<IHttpActionResult> GetPresidents()
    {
        await Task.Delay(TimeSpan.FromSeconds(10)).ConfigureAwait(false);
        return Ok();
    }
}

It's easiest way to implement async methods. Even if it will add some work to change everything to async it will benefit in future, because You will avoid many problem with async code.

If you absolutly HAVE to use async method in synchronous methods make it block in ONE place, like this:

public void MySyncMethod()
    {
        try
        {
            this.MyAsyncMethod().Wait();

        }
        catch (Exception exception)
        {
            //omited
        }
    }
private async Task MyAsyncMethod()
{
     await AsyncLogic().ConfigureAwait(false);
}

But i don't recommend it. You should just use async await all the way to controller action.

Carl Manaster
  • 39,912
  • 17
  • 102
  • 155
garret
  • 1,134
  • 8
  • 16
  • 2
    This example will deadlock in ASP.NET unless you use `ConfigureAwait(false)`, i.e. `await AsyncLogic().ConfigureAwait(false)`. See https://blog.stephencleary.com/2012/07/dont-block-on-async-code.html – bornfromanegg Dec 12 '17 at 17:12
  • You're right, I forgot about that, ty. Funny thing is that it work in localhost. Example was edited. – garret Dec 13 '17 at 07:57
-1

In your comment you said you want to process a task in the background and not make the client calling your API wait. To do that, you don't really need to use async/await.

Try this:

private void MyEndProcess()
{
    // other stuff

    Task.Run(_vendorPushService.PushResult()).ConfigureAwait(false); //fire and forget

    _logService.PostLog(LogType.TestLog, "Test");
}

The Task.Run will start the task, and the ConfigureAwait(false) tells it that it does not need to resume on the same context that we're currently on (meaning that the context can close before the task is finished - i.e. the response can be sent back without waiting for the task to finish).

You will get a compiler warning that you're not awaiting Task.Run, but that's what you want.

Keep in mind that when you do this, HttpContext.Current will not be available inside PushResult.

Gabriel Luci
  • 38,328
  • 4
  • 55
  • 84
  • `ConfigureAwait` is pointless when you're not actually awaiting it. Configuring an await that you never perform isn't accomplishing anything. – Servy Dec 12 '17 at 15:44
  • You're also advocating the OP use inherently asynchronous methods, block on the result in a method, then start that sync-over-async wrapper in a thread pool thread using `Task.Run`. That's all kinds of messed up and unproductive. – Servy Dec 12 '17 at 15:45
  • It blocks a thread, yes (until the task is done anyway), but it doesn't block the request, which is what the OP is looking for. I've used fire-and-forget tasks in Web API before. It has its dangers, but it has its place as well. The `ConfigureAwait` is important as I have found that without it the response won't send back until the task finishes, which is not fire-and-forget. – Gabriel Luci Dec 12 '17 at 15:48
  • Oh, I didn't even *get* to the fact that fire and forget is entirely inappropriate here. No, this is *not* an appropriate use of fire and forget. This is precisely an *inappropriate* use of that. And of course if you *did* want to fire and forget (which you don't) then you wouldn't want to synchronously block on an asynchronous operation, only to then do that synchronous work in a thread pool thread, only to ignore the result and not even care if it runs at all. Finally, no `ConfigureAwait` isn't important here. **It isn't doing anything**. It only does something if there is an `await`. – Servy Dec 12 '17 at 15:53
  • @GabrielLuci I do find that `ConfigureAwait` is not documented as well as it could be, but it does not do what you seem to think it does. `ConfigureAwait(false)` does _not_ mean "don't wait for this task". It means, "if this task is `await`ed, don't attempt to resume on the captured SynchronisationContext." Also, the 'context' we're talking about here is the SynchronisationContext. This does not 'close', as you put it. Perhaps you're confusing this with the HttpContext? – bornfromanegg Dec 12 '17 at 17:41