CODEWITHSUNDEEP
c#async-awaittask
Vinit Divekar
Vinit Divekar

Reputation: 918

Task.ContinueWith() results in error intermittently

I would like to call an async method for all the routes in the TierRoutes IEnumerable. The method returns a list of objects. When the Task is completed for the async call, I would like to update a property with the TierRoute's name property.

var orgTasks = _options.TierRoutes
    .Select(route => _ascendClient.FindAllOrganizations(new OrganizationSearchRequestModel { BaseAddress = route.BaseAddress})
        .ContinueWith(task =>
        {
            Console.WriteLine($"Inside ContinueWith for {route.BaseAddress}");
            return task.Result.Select(organizationDetail =>
            {
                organizationDetail.TierName = route.Name;
                return organizationDetail;
            });
        }));
var result = await Task.WhenAll(orgTasks);

The ContinueWith delegate results in TaskCancelledException intermittently. Open to alternate suggestions without using ContinueWith to solve the issue.

Upvotes: 2

Views: 98

Answers (2)

Panagiotis Kanavos
Panagiotis Kanavos

Reputation: 131706

The code never checks task.Exception for errors so if FindAllOrganizations fails, perhaps because too many concurrrent calls were made, ContinueWith will throw. This needs to be extracted into a separate method with proper exception handling.

public record Result(bool Ok,OrganizationDetail? Result,TierRoute Request);

async ValueTask<Result> GetDetailAsync(TierRoute route,CancellationToken token)
{
    var request = new OrganizationSearchRequestModel { BaseAddress = route.BaseAddress};
    try
    {
        var detail=await _ascendClient.FindAllOrganizations(request,token)
        detail.TierName=route.Name;
        return new Result(true,detail,route);
    }
    catch(Exception exc)
    {
        _logger.LogError($"Request Failed {request}");
        return new Result(false,null,route);
    }
}

The calling code would change to just this, after which results can be inspected for successful results:

var orgTasks = _options.TierRoutes
                       .Select(route => GetDetailsAsync(route,token));
var results = await Task.WhenAll(orgTasks);
foreach(var result in results.Where(r=>r.Ok))
{
...
}

The original code fires off all requests at once and may be causing the exceptions, either because the service crashes or throttles the client. In this case, it's better to use Parallel.ForEachAsync to limit the number of concurrent requests:

var opts=new ParallelOptions { MaxDegreeOfParallelism = 5};
var results= new ConcurrentQueue<Result>();

await Parallel.ForEachAsync(_options.TierRoutes,
    async (r,t)=>
    {
        var result=await GetDetailsAsync(r,t);
        results.Enqueue(result);
    },ops);

foreach(var result in results.Where(r=>r.Ok))
{
...
}

The default DOP is equal to the number of CPU cores

Upvotes: 2

Fildor
Fildor

Reputation: 16128

If you refactor a little bit:

async Task<IEnumerable<TOrganizationDetails>> GetOrganizationDetailsAsync(TRoute route)
{
    var organizations = await _ascendClient.FindAllOrganizations( route.ToOrganizationSearchRequestModel() );
    return organizations.Select(o => o.TierName = route.Name);
}

//... this is not needed, I just like to "declutter"
internal static class RouteMappingExtensions
{
    internal static OrganizationSearchRequestModel ToOrganizationSearchRequestModel (this TRoute route) =>
        new() {BaseAddress = route.BaseAddress};
}

You could use Parallel.ForEachAsync on that. If that's still needed, then. I'd first try without to keep low stress on the API and just sequentially call this for each route.

If you then find you need parallel calls, try ParallelForEachAsync with a low number of MaxDegreeOfParallelism, increasing as needed. But keep an eye on it to avoid flooding the API.

Mind that introducing parallelism will make exception handling more complex. If you just iterate and call above method, the await will unwrap exceptions. As soon as you go parallel (using whichever method to do it) you will need to get more explicit about catching exceptions.

P.S. of course you need to replace TRoute and TOrganizationDetails with your actual types.

Upvotes: 2

Related Questions