0

Application : Asp.Net Core Web API

I have requirement, On receiving the notification I should do 2 independent tasks. notification gets triggered as apart of web hook.

  1. call another service and then saves the response in log. Since this is independent task I do not want to wait the caller until we receive response. So I thought to wrap this function in Task.Run(), so that it will be run on another thread available on thread pool. I donot care wheather it fails or success, I just need to log the response.
     Task.Run(() => ProcessThroughRestClient(this.httpContext.Request, this.cspgConfiguration.Value.ApacNotificationService, this.restClient, this.logger));
  1. Saves the request object in DB for tracing. Since I must save the notification, I made it as awaitable task.
    await this.invoiceDbInstance.UpdateItemAsync(payment);

Below is the full code.

Main Method

public async Task<IActionResult> NotifyAsync()
    {
        this.logger.LogTrace("{CorrelationId} - notify async method has been called.", this.CorrelationId);

        Task.Run(() => ProcessThroughRestClient(this.httpContext.Request, this.Configuration.Value.NotificationService, this.restClient, this.logger));

        var request = this.GetDataFeedRequest(this.httpContext.Request);
        if (request != null)
        {
            this.logger.LogInformation("{CorrelationId} - data feed invoked with the order reference {OrderReference}", request.OrderReference, this.CorrelationId);
            Func<Invoice, bool> condition = x => x.SourceTransactionId == request.OrderReference;
            var payment = this.invoiceDbInstance.GetItem(condition);
            if (payment != null)
            {
                payment.TransactionStatus = request.IsSuccessStatusCode() ? TransactionStatus.Success : TransactionStatus.Failure;
                payment.TransactionId = request.PaymentReference;
                payment.UpdatedAt = DateTime.UtcNow.ToLongDateString();
                await this.invoiceDbInstance.UpdateItemAsync(payment);
                this.logger.LogInformation("{CorrelationId} - data feed updated  order reference {OrderReference} updated in Invoice with {PaymentReference}", request.OrderReference, this.CorrelationId, request.PaymentReference);
            }
            else
            {
                this.logger.LogInformation("{CorrelationId} - Payment not found.", this.CorrelationId);
            }
        }

        this.logger.LogTrace("{CorrelationId}- notify async method ended.", this.CorrelationId);
        return new OkResult();
    }

Http Call function which will be invoked through Task.Run()

private static HttpResponseMessage ProcessThroughRestClient(HttpRequest request, string url, IRestClient client, ILogger<PayDollarNotificationService> log)
    {
        try
        {
            log.LogTrace("Paydollar notify async ProcessThroughRestClient method has been called.");
            var parameters = request.Form?.ToDictionary(x => x.Key, x => x.Value.ToString());
            if (parameters.Any())
            {
                var response = client.PostAsync(url, RestClientHelper.BuildFormUrlEncodedContent(parameters)).Result;
                log.LogInformation("sent request to {url}.", url);
                log.LogInformation($"{url} response {response.ReadContent()?.Result}");
                return response;
            }
            else
            {
                log.LogInformation("No form parameters found.");
                return null;
            }
        }
        catch (Exception ex)
        {
            log.LogError($"An error occured: {ex.Message} {ex}");
        }

        return null;
    }

My question is, Is there any advantage using Task.Run() as above instead awitable task? or is it is going to be blocking thread?

Chandra Mohan
  • 729
  • 2
  • 10
  • 29
  • How would you feel about having your `ProcessThroughRestClient` being async but not awaitable ? You would not need Task.Run, and it would still run on it's own task, without blocking the rest of your calls – Irwene Oct 02 '20 at 11:33
  • Have you read Stephen Cleary's [Fire and Forget on ASP.NET](https://blog.stephencleary.com/2014/06/fire-and-forget-on-asp-net.html) blog post? – Theodor Zoulias Oct 02 '20 at 11:39
  • @TheodorZoulias that was quite old. I thought we might have better solutions now. more over I need to capture the response in logs. – Chandra Mohan Oct 02 '20 at 11:41
  • Why are you returnin `HttpResponseMessage` if you don't actually care for the result as you say? I would refactor `ProcessThroughRestClient` to return whatever it is that needs to be logged, and make that async. Then wrap the logging process in some other function and call that with `Task.run`. If you await the call to `ProcessThroughRestClient` in `NotifyAsync` the execution would return back to the code that called `NotifyAsync`, therefore the sequence inside `NotifyAsync` would be syncronous, in other words would block `NotifyAsync`, but not necessarily its caller. – Pedro Rodrigues Oct 02 '20 at 11:50
  • @PedroRodrigues I need to log the response as well. This will depends on the ProcessThroughRestClient. – Chandra Mohan Oct 02 '20 at 11:55
  • Ok. But did I answer your question in any way? Not sure I understood it to begin with. – Pedro Rodrigues Oct 02 '20 at 11:58
  • 2
    I don't think that using `Task.Run` on ASP.NET is now more reliable than it was 6 years ago. Here is a recent related question answered by Stephen Cleary: [Is it allowed to use Task.Run in an ASP.NET Core controller?](https://stackoverflow.com/questions/60283722/is-it-allowed-to-use-task-run-in-an-asp-net-core-controller) – Theodor Zoulias Oct 02 '20 at 11:58

0 Answers0