Correct usage of return Task.FromException

Question

I recently observed a code review between two developers.

The following code was submitted:

 public async Task<List<Thing>> GetThings()
    {
        try
        {
            var endpoint = $"{Settings.ThingEndpoint}/things";
            var response = await HttpClient.GetAsync(endpoint);
            return JsonConvert.DeserializeObject<List<Thing>>(await response.Content.ReadAsStringAsync());
        }
        catch (Exception e)
        {
            Log.Logger.Error(e.ToString());
            return await Task.FromException<List<Thing>>(e);
        }
    }

Which received the following review comments:

There is absolutely no need to return await Task.FromException>(e), this is something you do when dealing with non awaited task. In this case the catch will capture whatever exception var response = await HttpClient.GetAsync(endpoint); will throw. You should just remove it and catch the exception as is

I do not fully understand why not to use Task.FromException in this case, so i have the following Questions:

What is the reviewer saying?
Is the reviewer correct?
Why not return await Task.FromException?
What is a correct scenario to return await Task.FromException?

1. The reviewer is saying you should throw; instead (after logging) 2: Yes, the reviewer is correct. 3/4. Task.FromException is for use in library code in non-async/await methods — Lasse V. Karlsen
– Lasse V. Karlsen, Commented Jun 4, 2019 at 13:07
By "non async/await method" I mean a method that is not written as async Task, just as Task, inside you have to return tasks, like Task.FromResult or Task.FromException. — Lasse V. Karlsen
– Lasse V. Karlsen, Commented Jun 4, 2019 at 13:08
@LasseVågsætherKarlsen Thank you. I do not understand why you would use in a non async method. And why you would not use in an asyc method. Can you explain further in detail please? — fourbeatcoder
– fourbeatcoder, Commented Jun 4, 2019 at 13:12

Lasse V. Karlsen · Accepted Answer · 2019-06-04 13:15:19Z

25

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 an async/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.

answered Jun 4, 2019 at 13:15

Lasse V. Karlsen

394k107 gold badges651 silver badges845 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

TRS Over a year ago

Task.FromResult will throw an aggregate Exception which needs to be dissected for re throwing further.

TRS · Accepted Answer · 2019-06-04 13:20:42Z

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;

        }
    }

Collectives™ on Stack Overflow

Correct usage of return Task.FromException

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related