0

Note this might be a bit different from other questions as I use a try-catch in the async method and I just do nothing in the exception as it is not a critical problem if it was to fail.

I have a method (with more code not shown) coded like this:

public bool ArrangeCardOrder(bool IsFirstToLast)
{
    try
    {
    // Do stuff
    }
    catch (Exception ex)
    {
        await Helper.LogCrash();
        return false;
    }
}

The LogCrash method looks like this:

public static async Task LogCrash(string ex)
{
   try
   {
      var logCrash = new Cosmos.LogCrash()
      {
         Ex = exception
      };
      await App.CDB.InsertLogItem(logCrash);
   }
   catch (Exception)
   {

   }
}

InsertLogItem is async so I made LogCrash async. But then I have a problem with calling that as ArrangeCardOrder is not async.

Does anyone have a suggestion on how I can call LogCrash from ArrangeCardOrder bearing in mind that I already catch exceptions in LogCrash so I think that's already being handled.

Alan2
  • 23,493
  • 79
  • 256
  • 450
  • 1
    How about making `ArrangeCardOrder` async as well? – Sweeper Feb 29 '20 at 11:29
  • If you don't care when the method completes and whether it succeeds, just [fire and forget](https://stackoverflow.com/q/46053175/11683) by omitting the `await`. – GSerg Feb 29 '20 at 11:30
  • As suggested by @GSerg, try with fire & forget or if you want the result, try with update code like this [App.CDB.InsertLogItem(logCrash) .ConfigureAwait(false).GetAwaiter().GetResult();] – Raushan Kuamr Jha Feb 29 '20 at 11:42
  • There is no good way to do that. Refactor your code and make your method returning `Task`. I know it can be tricky if you have 99+ references to that method in your project... – OlegI Feb 29 '20 at 11:51
  • I do have potentially many references. – Alan2 Feb 29 '20 at 12:39
  • @RaushanKuamrJha - can you give me your suggestion as a suggested answer. – Alan2 Feb 29 '20 at 12:39

3 Answers3

2

For logging in particular, a common pattern is to always have synchronous log methods that actually just write to an in-memory queue, which is asynchronously written to the actual data store by a separate actor. So the log methods themselves are naturally synchronous (updating an in-memory queue), while the actual log writes (e.g., InsertLogItem) remain asynchronous but not directly called by the logging methods. Since it looks like you're writing your own logging framework (for some reason), I'd recommend this approach.

If you choose not to take the queue approach, then the next-recommended approach is to either go asynchronous all the way or synchronous all the way.

Ideally, your code should be synchronous or asynchronous all the way up. You currently have a method that is asynchronous (e.g., LogCrash). So ideally you should either make all the calling code asynchronous or make LogCrash synchronous.

// Ideal option #1
public async Task<bool> ArrangeCardOrder(bool IsFirstToLast)
{
  try
  {
    // Do stuff
  }
  catch (Exception ex)
  {
    await Helper.LogCrash();
    return false;
  }
}

public static async Task LogCrash(string ex)
{
  ... // unchanged
}

or the synchronous option:

// Ideal option #2
public bool ArrangeCardOrder(bool IsFirstToLast)
{
  try
  {
    // Do stuff
  }
  catch (Exception ex)
  {
    Helper.LogCrash();
    return false;
  }
}

public static void LogCrash(string ex)
{
  try
  {
    var logCrash = new Cosmos.LogCrash()
    {
      Ex = exception
    };
    App.CDB.InsertLogItem(logCrash);
  }
  catch (Exception)
  {
  }
}

Note that the synchronous option assumes that InsertLogItem is made synchronous as well.

The next-recommended option is to actually fire-and-forget. Unlike the queue-based approach, fire-and-forget will prevent you from being aware of any logging exceptions. To do fire-and-forget, you can just call the asynchronous method and ignore the task it returns:

public static void LogCrash(string ex)
{
  try
  {
    var logCrash = new Cosmos.LogCrash()
    {
      Ex = exception
    };
    var _ = App.CDB.InsertLogItem(logCrash);
  }
  catch (Exception)
  {
  }
}

If none of these options are desirable, then you can use a hack to call asynchronous code from synchronous code.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
0

it depends...

If you need the result of Helper.LogCrash() you'll need to make your ArrangeCardOrder() function async too. Consider using ValueTask instead of a normal task, if you typically return a in a non async way for performance reasons.

https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.valuetask-1?view=netcore-3.1

If you don't need the result and want to stay with the static function design: Simply ignore the result be removing the await keyword

If you plan to replace your Helper with a service approach, you should wait for the result and make ArrangeCardOrder() async too to ensure your Helper service was not disposed.

Christoph Lütjen
  • 5,403
  • 2
  • 24
  • 33
  • Hello, My application is full of methods that will call LogCrash so I think it's best to not make it async for now. When I remove await it the IDE shows there it's an issue. Could I run this somehow as a task but have some way to ignore the output without seeing the warning message? – Alan2 Feb 29 '20 at 12:38
  • You can disable the warning per usage or globally. I'd recommend not to disable it on project level, because not awaited async functions may result in tricky to find bugs. If you never want to wait for LogCrash(), let it return void and don't make it async. – Christoph Lütjen Feb 29 '20 at 13:31
-2

@Alan2: I think below-mentioned workaround may work for you (either V1 or V2):

public static void LogCrash(string ex)
{
   try
   {
      var logCrash = new Cosmos.LogCrash()
      {
         Ex = exception
      };      

      // V1: WAIT for RESULT: If you want to wait to get the result
      //await App.CDB.InsertLogItem(logCrash);
      App.CDB.InsertLogItem(logCrash).ConfigureAwait(false).GetAwaiter().GetResult();

      // V2: FIRE AND FORGOT: if you do not want to wait for the result   
      // var taskInsertLogItem =  Task.Run(async() => await App.CDB.InsertLogItem(logCrash););
   }
   catch (Exception ex)
   {
   //TODO: 
   }
}