Employee details becoming null in C# Parallel.ForEach loop when fetching data from API

Question

I have encountered an issue while fetching employee details from an API using a parallel foreach loop in C#. Please note I have used concurrentbag for the tasks to make it thread-safe and adding items to the list within the lock to avoid it being accessed by multiple threads at a time

Here's what I'm doing:

• I have a list of employee numbers.
• I iterate through this list using a parallel foreach loop.
• Inside the loop, I call an API with the current employee number to retrieve the employee details.
• I add the fetched employee details to a list.

However, I've noticed that sometimes the employee object in the employee details list becomes null. I'm unsure why this is happening and how to resolve it.

public List<ImportedEmployee> GetEmployees(List<String> empNumber)
{
    List<ImportedEmployee> employeeList = new List<ImportedEmployee>();

    try
    {
        List<ImportedEmployee> cb = new List<ImportedEmployee>();
        var tasks = new ConcurrentBag<Task>();

        Parallel.ForEach(empNumber, reference =>
        {
            tasks.Add(Task.Run(() =>
            {
                var importedEmployee = GetEmployeeDetails(reference.ToString());

                if (importedEmployee != null)
                {
                    lock (cb)
                    {
                        cb.Add(importedEmployee);
                    }
                }
            }));
        });

        Task.WaitAll(tasks.Where(t => t != null).ToArray());

        employeeList.AddRange(cb.ToList());
    }
    catch (Exception ex)
    {
        Log.Error("Error");
    }

    return employeeList;
}

public ImportedEmployee GetEmployeeDetails(string empNum)
{
    ImportedEmployee employeeDetails = new ImportedEmployee();

    try
    {
        var responseString = ServiceRequest($"Employees/{empNum}", null, HttpVerbs.Get).ToString();

        JObject result = JObject.Parse(responseString);
        employeeDetails.FirstName = result["FirstName"].ToString();
        employeeDetails.LastName = result["LastName"].ToString();
        employeeDetails.Username = result["LoginId"].ToString();
    }
    catch (Exception ex)
    {
        Log.Error("Error retrieving employee details.", ex);
    }

    return employeeDetails;
}

        public string ServiceRequest(string URI, string PostJSON, HttpVerbs Method)
        {
            string responseString;
            try
            {                
                HttpResponseMessage resp;
                using (HttpClient client = new HttpClient() { Timeout = TimeSpan.FromSeconds(Configuration.ClientTimeout) })
                {
                    HttpRequestMessage request = new HttpRequestMessage();
                    switch (Method)
                    {
                        case HttpVerbs.Get:
                            request = new HttpRequestMessage(HttpMethod.Get, BaseURL + URI);
                            break;
                        default:
                            request = new HttpRequestMessage(HttpMethod.Post, BaseURL + URI);
                            request.Content = new StringContent(PostJSON, null, "application/json");
                            break;
                    }
                    request.Headers.Add("Authorization", $"Bearer {AuthToken.Token}");
                    resp = client.SendAsync(request).Result;
                }
                responseString = resp.Content.ReadAsStringAsync().Result;
            }
            catch (Exception ex)
            {
                Log.Error("Error", ex);
                responseString = null;
            }
            return responseString;
        }

public class ImportedEmployee
    {
        public string EmployeeNumber { get; set; }
        public string Username { get; set; }
        public string FirstName { get; set; }
        public string LastName { get; set; }
}

Answer 1

You need to simplify this first & get rid of the duplicate concurrency. This might just be some incompatibility between Parallel & Task.

Try something like this first & let us know if you're still getting the nulls in the final list.

Note this is not guaranteed to be working code - I just did this in an online IDE to show you what we're talking about, so please correct any oversights.

This is the suggestion with AsParallel & PLINQ.

public List<ImportedEmployee> GetEmployees(List<String> empNumber)
{
    try
    {
      return empNumber.AsParallel()                   // parallelize
                    .Select(GetEmployeeDetails)     // call API
                    .OfType<ImportedEmployee>()     // filter out nulls
                    .ToList();                       
   }
   catch (Exception ex)
   {
       Log.Error("Error: " + ex.Message);
       return new List<ImportedEmployee>();
   }
}

This is what the code would look like without using Parallel.ForEach.

public List<ImportedEmployee> GetEmployees(List<String> empNumber)
{
    var employees = new List<ImportedEmployee>();

try
{
    Parallel.ForEach(empNumber, reference =>
    {
            var importedEmployee = GetEmployeeDetails(reference.ToString());

            if (importedEmployee != null)
            {
                lock (cb)
                {
                    cb.Add(importedEmployee);
                }
            }
    });

}
catch (Exception ex)
{
    Log.Error("Error: " + ex.Message);
}

return employeeList;}