Best way to handle exception in multi task to call WCF operation

Question

I have implemented a service name ExamClient which have two operations one is Ping which return a basic string which mean service is available and one is FindStudy which search in DB it may take a long to be proceeded.

In the other side I have several endpoints of ExamClient I wand to run FindStudy per end point by task so in a Dispatcher I have something like this:

public FindStudies_DTO_OUT FindStudies(FindStudies_DTO_IN findStudies_DTO_IN)
{
    List<Study_C> ret = new List<Study_C>(); 
    List<Task> tasks = new List<Task>();
    foreach (var sp in Cluster)
    {
        string serviceAddress = sp.GetLibraryAddress(ServiceLibrary_C.PCM) + "/Exam.svc";
        var task = Task.Run(() =>
        {
            ExamClient examClient = new ExamClient(serviceAddress.GetBinding(), new EndpointAddress(serviceAddress), Token);
            var ping = Task.Run(() =>
            {
                examClient.Ping();
            }); 
            if (!ping.Wait(examClient.Endpoint.Binding.OpenTimeout))
            {
                Logging.Log(LoggingMode.Warning, "Timeout on FindStudies for:{0}, address:{1}", sp.Name, serviceAddress);
                return new List<Study_C>(); // if return null then need to manage it on ret.AddRange(t.Result);
            }  
            return (examClient.FindStudies(findStudies_DTO_IN).Studies.Select(x =>
            {
                x.StudyInstanceUID = string.Format("{0}|{1}", sp.Name, x.StudyInstanceUID);
                x.InstitutionName = sp.Name;
                return x;
            }));
        });
        task.ContinueWith(t =>
        {
            lock (ret)
            {
                ret.AddRange(t.Result);
            }
        }, TaskContinuationOptions.OnlyOnRanToCompletion);
        task.ContinueWith(t =>
        {
            Logging.Log(LoggingMode.Error, "FindStudies failed for :{0}, address:{1}, EXP:{2}", sp.Name, serviceAddress, t.Exception.ToString());
        }, TaskContinuationOptions.OnlyOnFaulted);

        tasks.Add(task);
    } 
    try
    {
        Task.WaitAll(tasks.ToArray());
    }
    catch (AggregateException aggEx)
    {
        foreach (Exception exp in aggEx.InnerExceptions)
        {
            Logging.Log(LoggingMode.Error, "Error while FindStudies EXP:{0}", exp.ToString());
        }
    }

    return new FindStudies_DTO_OUT(ret.Sort(findStudies_DTO_IN.SortColumnName, findStudies_DTO_IN.SortOrderBy));
}

First I have to run Ping per end point to know connection is established after that FindStudy.

if there are three end pints in Cluster six task be run in parallel mode, 3 for Ping and 3 for FindStudy.

I think something is wrong with my code to handle exception nice... So what is the best way to implement this scenario ?

thanks in advance.

Answer 1

Let me throw my answer to simplify and remove unnecessary code block. And bit of explanation along the code.

public FindStudies_DTO_OUT FindStudies(FindStudies_DTO_IN findStudies_DTO_IN)
{
    // Thread-safe collection
    var ret = new ConcurrentBag<Study_C>()

    // Loop cluster list and process each item in parallel and wait all process to finish. This handle the parallism better than task run
    Parallel.Foreach(Cluster, (sp) =>
    {
        var serviceAddress = sp.GetLibraryAddress(ServiceLibrary_C.PCM) + "/Exam.svc";

        ExamClient examClient = new ExamClient(serviceAddress.GetBinding(), new EndpointAddress(serviceAddress), Token);

        try
        {
            examClient.Ping();              
            // declare result variable type outside try catch to be visible below               
            var result = examClient.FindStudies(findStudies_DTO_IN);
        }
        catch(TimeoutException timeoutEx)
        {
            // abort examclient to dispose channel properly
            Logging.Log(LoggingMode.Warning, "Timeout on FindStudies for:{0}, address:{1}", sp.Name, serviceAddress);
        }
        catch(FaultException fault)
        {
            Logging.Log(LoggingMode.Error, "FindStudies failed for :{0}, address:{1}, EXP:{2}", sp.Name, serviceAddress, fault.Exception.ToString());
        }
        catch(Exception ex)
        {
            // anything else
        }
        // add exception type as needed for proper logging

        // use inverted if to reduce nested condition
        if( result == null )
            return null;

        var study_c = result.Studies.Select(x =>
        {
            x.StudyInstanceUID = string.Format("{0}|{1}", sp.Name, x.StudyInstanceUID);
            x.InstitutionName = sp.Name;
            return x;
        }));

        // Thread-safe collection
        ret.AddRange(study_c);      
    });

    // for sorting i guess concurrentBag has orderby; if not working convert to list
    return new FindStudies_DTO_OUT(ret.Sort(findStudies_DTO_IN.SortColumnName, findStudies_DTO_IN.SortOrderBy));
}

Note : Code haven't tested but the gist is there. Also I feels like task.run inside task.run is bad idea can't remember which article I read it (probably from Stephen Cleary not sure).