Correct usage of return Task.FromException
- In general returning from catch is not good coding practice.
Task.FromException is generally used when you want to rely on status of Task if a known failure condition is met. For example, if an object is null you know you should return a faulted task.Client can use state of task as faulted to show appropriate message to user.I modified the code just to tell you on example.
public async Task<List<Thing>> GetThings() { try { var endpoint = $"{Settings.ThingEndpoint}/things"; var response = await HttpClient.GetAsync(endpoint); var obj = JsonConvert.DeserializeObject<List<Thing>>(await response.Content.ReadAsStringAsync()); if(obj==null) { return await Task.FromException<List<Thing>>(new NullRefernceException()); } else { } } catch (Exception e) { Log.Logger.Error(e.ToString()); throw; } }
The reviewer is entirely correct.
The only situation you would use Task.FromException
is when you're in a method you cannot or won't implement using async
and await
, and you want the result of the task should be an exception.
Idiot example but anyway:
public Task<int> NotReallyAsync()
{
if (new Random().Next(2) == 0)
return Task.FromResult(42);
return Task.FromException<int>(new InvalidOperationException());
}
So let's deal with your questions one by one:
The reviewer is saying that
Task.FromException
should only be used in a non-async
/await
method, in anasync
/await
method, you should instead just rethrow the exception:catch (Exception e) { Log.Logger.Error(e.ToString()); throw; }
or if you implement an exception filter:
catch (Exception e) when (Log.Logger.ExceptionFilter(e)) { }
Yes, the reviewer is correct.
- Because it is unnecessary, instead just rethrow the exception. If you want to throw an exception, just throw it. The purpose of
async
/await
is to be able to write your method in a normal manner, so write a normal throw statement or a normal catch-block. - Non-
async
/await
methods, and only that.