0

Do anyone know why the following code does not catch my ConnectionException, I've spend hours with it...

    public async Task LoadContacts(string filter)
    {
        // We will send find contacts message to all servers supporting addressbook.
        var addressBookServers = _addressBookService.GetAddressBookServerList();
        // Create task array here to be able to run each task in parallel manner.
        var tasksToProcess = new List<Task<SearchContactsResultDto>>(addressBookServers.Count);

        for (int i = 0; i < addressBookServers.Count; i++)
            tasksToProcess.Add(_addressBookService.SearchContactsAsync(addressBookServers[i].Id, filterUpCaseNoDiacritics));

        while (tasksToProcess.Count > 0)
        {
            var processedTask = await Task.WhenAny(tasksToProcess);
            tasksToProcess.Remove(processedTask);
            try
            {
                var serverResponse = await processedTask.ConfigureAwait(false);
                var vmToAdd = serverResponse.SearchedContacts
                .Where(sc => !SearchedContacts.Exists(c => c.BabelName == sc.BabelName))
                .Select(sc => CreateSearchContactViewModel(serverResponse.ServerId, null, sc.Name, sc.BabelName, sc.ContactId));

                SearchedContacts.AddRange(vmToAdd);
            }
            catch (ErrorMessageException eme) { Log.Warning(s => s.Set($"An {nameof(ErrorMessageException)} of type {eme.ErrorMessage.Cause} threw as a response to {nameof(Core.Json.Messages.MsgFindContacts)}. See exception details for further information.", eme)); }
            catch (ConnectionException ce) { Log.Info(s => s.Set($"Connection with server cannot be reached. Message of type {nameof(Core.Json.Messages.MsgFindContacts)} cannot be send", ce)); }
            catch (TimeoutException te) { Log.Info(s => s.Set($"Request on a message of type {nameof(Core.Json.Messages.MsgFindContacts)} timeouted. See exception details for further details.", te)); }
            catch (Exception) {}
        }
        IsLoadingContacts = false;
    }

When SearchContactsAsync throws an Exception, this exception is not catched by LoadContacts method and is propagated as a unhandled AggregateException. I've wrote some unit tests and they all pass, the problem occurs in running application. I appreciate any help.

SearchContactsAsync implementation:

public async Task<SearchContactsResultDto> SearchContactsAsync(int serverId, string filter)
    {
        var msgFindContactsRes = await _communicationService.SendFindContactsAsync(serverId, filter)
            .ConfigureAwait(false);

        return new SearchContactsResultDto()
        {
            ServerId = serverId,
            SearchedContacts = msgFindContactsRes.Contacts,
            PageNumber = msgFindContactsRes.PageNumber,
            PageSize = msgFindContactsRes.PageSize
        };
    }

SendFindContacsAsync impl:

public Task<MsgFindContactsRes> SendFindContactsAsync(int serverId, string filter)
    {
        var serverSender = serverConnectionProvider.ProvideSender(serverId);

        var msgFindContacts = messageFactory.CreateMsgFindContacts(filter);

        return serverSender.SendAsync<MsgFindContactsRes>(msgFindContacts);
    }

SendAsync:

public async Task<TExpectedResponse> SendAsync<TExpectedResponse>(IMessage message)
        where TExpectedResponse : class, IMessage
    {
        if (message == null)
            throw new ArgumentNullException($"Argument {nameof(message)} cannot be null.");

        var response = await _queue.ProvideAsync(message).ConfigureAwait(false);

        if (response is TExpectedResponse)
            return response as TExpectedResponse;
        else throw new InvalidServerResponseException($"Invalid response to a message of type {message.Header.Type}, expected message type: {typeof(TExpectedResponse).FullName}, received message type: {response.Header.Type}. Server id: {_serverConnection.ServerId}");
    }

ProvideAsync using TPL queue and TCS:

public Task<TItemResult> ProvideAsync(TItemData item)
    {
        TaskCompletionSource<TItemResult> tcs = new TaskCompletionSource<TItemResult>();

        // async enqueue an item with its task completion source
        _queue.SendAsync<QueueItem<TItemData, TItemResult>>(new QueueItem<TItemData, TItemResult>(tcs, item));

        return tcs.Task;
    }

And finally the queue consumer which throws an exception using the TaskCompletionSource:

private async Task StartConsumer(Func<TItemData, TItemResult> consumer)
    {
        while (await _queue.OutputAvailableAsync())
        {
            QueueItem<TItemData, TItemResult> res = await _queue.ReceiveAsync();
            try
            {
                var result = consumer(res.Data);
                res.Tcs?.SetResult(result);
            }
            catch (Exception e)
            {
                res?.Tcs.SetException(e);
                throw;
            }
        }
    }
Lukáš Koten
  • 1,191
  • 10
  • 22
  • Catch the `AggregateException` and unwrap it (loop through the `InnerExceptions` property. It will then tell you exactly what is wrong with your code and give you an idea as to which part of your code is actually failing. – Adrian Sanguineti Dec 09 '16 at 00:50
  • Try removing `ConfigureAwait(false)` from your method. This is just a guess, while I'm trying to reproduce this with a real service like you're doing. – Alisson Reinaldo Silva Dec 09 '16 at 01:06
  • 2
    Did you put a breakpoint in all of your three "catches" to see if it really reaches there sometime? I think there is a chance of a exception being caught by one of your three "catches" (ErrorMessageException, ConnectionException or TimeoutException, but not the generic Exception catch block), and the `Log.Info()`or `Log.Warning()` methods are throwing exceptions themselves, which are not caught because there is no another try catch block outside your existing try catches. – Alisson Reinaldo Silva Dec 09 '16 at 01:59
  • @Alisson, good point, I tried to add breakpoint into it and you are right.. The exception is caught, anyway it still propagates to caller. I cant see the reason why is this happening and how to fix it right now. I tried to remove Log callings but nothing has changed... – Lukáš Koten Dec 09 '16 at 06:44
  • @Alisson, removing ConfigureAwait did not help... – Lukáš Koten Dec 09 '16 at 07:02
  • @LukášKoten: Does `SearchContactsAsync` throw the exception directly, before any asynchronous work? – Stephen Cleary Dec 19 '16 at 02:40
  • @StephenCleary: No, SearchContactsAsync does not throw the exception directly, it is during an asynchronous work... I added some code under the hood in last edit. – Lukáš Koten Dec 21 '16 at 20:52

3 Answers3

1

The tasks for SearchContactsAsync are here

for (int i = 0; i < addressBookServers.Count; i++)
     tasksToProcess.Add(_addressBookService.SearchContactsAsync(addressBookServers[i].Id, filterUpCaseNoDiacritics));

Those tasks are awaited outside of the try block:

var processedTask = await Task.WhenAny(tasksToProcess);
tasksToProcess.Remove(processedTask);
try
{
    // Rest of your code

Therefore if one of those tasks throw an exception, nothing is handling it. Move the lines

var processedTask = await Task.WhenAny(tasksToProcess);
tasksToProcess.Remove(processedTask);

inside the try block.

Ivan Vargas
  • 703
  • 3
  • 9
  • 3
    I think those tasks are not being awaited outside the try block. When he calls `WhenAny`, the exception is not propagated yet, because `WhenAny()` just returns that task which finished. The exception is propagated when he awaits that task resulted from `WhenAny()`, which he does inside try block with this: `var serverResponse = await processedTask.ConfigureAwait(false);`. – Alisson Reinaldo Silva Dec 09 '16 at 01:31
  • Yes, you are absolutely right. `await await Task.WhenAny(tasksToProcess);` would throw the exception at that point. – Ivan Vargas Dec 09 '16 at 03:44
0

You are not catching aggregate exception and this is why your exception goes unhandled.

//Following code is just an outline and you need to correct according to your requirement.

          try {
                 //your task
              }
          catch (AggregateException ae)
          {
             ae.Handle( x => 
                       { 
                            // Handle an ErrorMessageException
                            if (x is ErrorMessageException) {
                                //do your logging
                                return true; //if you handled it through logging or reporting
                            }
                            // Handle an ConnectionException
                            else if(x is ConnectionException){ 
                              //do your logging for connection
                              return true;  //if you handled it through logging or reporting 
                             }
                           else
                               return false; //This leave the exception as unhandled aggregate exception.

                       });
          }
S.N
  • 4,910
  • 5
  • 31
  • 51
  • Sorry, i have edited my code to catch any Exception, but the AggregateException is still propagating. – Lukáš Koten Dec 09 '16 at 00:33
  • 1
    @LukášKoten, If you enclose an activity within a Task, any type of exception raised inside the activity will be propagated as AggregateException. You need to unwrap this aggregate exception to see what went wrong internally. If you read the msdn link, it has the detail of how Handle method switch different type of exceptions. – S.N Dec 09 '16 at 00:40
  • I thought that awaiting the task should unwrap the aggregate exception internally so it behaves as any other exception. – Lukáš Koten Dec 09 '16 at 00:52
  • I still think the service is throwing that `ConnectionException`, and the `AggregateException` is being thrown by the Log stuff done inside the catch block, I couldn't see for what reason the generic `Exception` catch couldn't handle the exception, if it was being thrown inside the try block. – Alisson Reinaldo Silva Dec 09 '16 at 03:59
-2

Oh well, I can see the problem right now. The last line of the code I've posted... it throws the exception again which is never ever caught... What a bad mistake. Anyway, thanks all participates for help...

Lukáš Koten
  • 1,191
  • 10
  • 22