-1

I have wrapped the action in Task.Run but it seems that I am missing something very basic. But unable to figure it out.

public void SaveOrderList(List<Order> inputList)
{
    Dictionary<string, string> result = new Dictionary<string, string>();
    string code = string.Empty;
    Task.Run(() =>
    {
        foreach (var item in inputList)
        {
            code = CreateSingleOrder(item);
            result.Add(item.TicketNumber, code);
        }

        ////TODO: Write logic to send mail
        emailSender.SendEmail("abc@xyz.com");
    });
}

Since there can be many entries in inputList and each entry may take 5 sec to process, I don't want the UI to be blocked for end user. Instead, I will send a mail and notify how many processed successfully and what all are failed.

To achieve this, best I knew was Task.Run. But, the problem is as soon as function completes, I don't see that the code inside the foreach loop ever worked because it never made to the DB.

Can anyone help me find out what is that I am missing here.

Just for information, this function is called from Web API and Web API POST method is called from javascript. Below is the code for Web API endpoint.

[HttpPost, Route("SaveOrderList")]
[ResponseType(typeof(bool))]
public IHttpActionResult SaveOrderList(List<Order> orderList)
{
     orderManagerService.SaveOrderList(orderList)
     return this.Ok();
}

Thanks in advance for help.

Camilo Terevinto
  • 31,141
  • 6
  • 88
  • 120
user2861226
  • 169
  • 13
  • First suggestion would be to run your program with debugger attached, put a breakpoint on the lines within the foreach loop and see what happens. If your debugger puts a breakpoint on the whole lambda expression rather than the individual line, then move the foreach loop in to its own method, where that will not be a problem – LordWilmore Apr 03 '18 at 12:49
  • 1
    You shouldn't implement a Run and forget code on the web. No one will guarantee that the process won't be killed after the response is sent to the client. – hardkoded Apr 03 '18 at 12:50
  • @LordWilmore When I put debugger inside the foreach loop, everything works as I expect. All the entries in the list executes one by one and then emailSender gets invoked correctly. When I remove all the breakpoint in the application, then nothing seem to work. – user2861226 Apr 03 '18 at 12:54
  • @hardkoded Is there a way to handle a situation where UI should not be blocked for a long task? I think I can put a try catch inside the task.run so that if there is any error, I will read the dictionary and figure out how many were executed before this exception is thrown. – user2861226 Apr 03 '18 at 12:56
  • 2
    Well, you are creating a fire-and-forget task, it shouldn't ever wait until the task completes. You are also returning a 200 OK when you have absolutely no idea whether the code passed correctly or not – Camilo Terevinto Apr 03 '18 at 12:56
  • Also, if you are using AJAX, just put a loading/spinner while the response is obtained from the service. A webpage is only blocked if you are POSTing with synchronous AJAX or a post back. – Camilo Terevinto Apr 03 '18 at 12:57
  • @user2861226 I would implement a background process. Or, as Camilo said, and open ajax call. – hardkoded Apr 03 '18 at 13:01
  • @CamiloTerevinto Thanks for the information. I think the AJAX I am using is asynchronous. But thanks for the pointers. I am returning 200 OK response because, I don't want user to be blocked for no reason as each entry takes 5-8 sec to get processed. Its not just pushing into DB, I am making an external SOAP service call that processes this order at their end. What if I implement try catch inside the for loop and notify user via mail about what all orders are failed with reason. – user2861226 Apr 03 '18 at 13:31
  • @hardkoded No one will guarantee that the process won't be killed after the response is sent to the client. Is this means that the process will keep eating the memory till IIS or whatever web server is recycled? If that is the case, this is a serious concern. – user2861226 Apr 03 '18 at 13:34
  • @user2861226 exactly – hardkoded Apr 03 '18 at 13:36

1 Answers1

0

You need to consider carefully how this works. There are a few suggestions in this article:

https://blog.stephencleary.com/2014/06/fire-and-forget-on-asp-net.html

But I would point out that 'fire and forget' on a web application is usually the wrong approach.

For your example, you really want to consider your UX - if I make an order on your site and then only find out some time later that the order failed (via email, which I may not be checking), I'd not be too impressed. It would be better to await the save result, or make multiple API requests for single order items and show the incremental result of successful orders on your front end.

I'd also suggest a hard look at why your order saving is so slow - this will continue to be problematic for you until it's faster.

Paddy
  • 33,309
  • 15
  • 79
  • 114