1

I was feeling like I had quite a good grip on async await programming, but what happened today made me perplexed. I have been searching for an answer to this for a little while now, but was not able to find it anywhere. I have read quite a bit about async await programming, but my feeble mind is not capable of understanding what exactly is happening in this particular scenario.

I have these two methods:

public async Task<IEnumerable<ServerStatus>> GetServersStatusAsync()
{
    var serverStatuses = new List<ServerStatus>();
    try
    {
        ServerStatusRequest request = new ServerStatusRequest();
        var serverStatusResponse = await GetAsync<ServerStatusResponse>(request).ConfigureAwait(false);
    }
    // I would expect the exception thrown from GetAsync to be caught here. But it doesn't always do that.
    catch (Exception ex)
    {
        _log.Error("Failed to retrieve server statuses.", ex);
    }
    return serverStatuses;
}

And

private async Task<T> GetAsync<T>(IRequest request)
{
    string respString = string.Empty;
    try
    {
        
        var requestUrl = request.GetQueryString(_apiToken);
        _log.Debug($"Sending a request to {requestUrl}");
        CancellationTokenSource tokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(_timeoutInSec));

        //This call can throw an exception. It will always get caught inside this current try catch, 
        //but if after the await we are continuing on a different thread, 
        //the re-thrown/propagated exception is not caught by the calling method.
        var response = await _client.GetAsync(requestUrl, tokenSource.Token).ConfigureAwait(false);

        if (response.IsSuccessStatusCode || response.StatusCode == HttpStatusCode.BadRequest)
        {
            respString = await response.Content.ReadAsStringAsync();
            var deserializedObject = JsonConvert.DeserializeObject<T>(respString);
            return deserializedObject;
        }
    }
    catch (Exception ex) when (ex is JsonReaderException || ex is JsonSerializationException)
    {
        throw new JsonSerializationException($"Failed to deserialize {respString}", ex) { Source = respString };
    }

    return default(T);
}

I have added 2 comments to the code snippets as pointers, but the basic idea is:

In GetServerStatusAsync I have wrapped everything with a try catch as I want to handle the exceptions there. I call GetAsync which awaits a call to HttpClient.GetAsync with ConfigureAwait(false). When the call from HttpClient.GetAsync returns with an exception if we are no longer on the initial thread/context, it can be caught inside my GetAsync method, but will not get propagated to GetServerStatusAsync. If I remove ConfigureAwait(false), it always propagates down as I would expect, but with ConfigureAwait(false) it's more of a 50/50.

Is there something catastrophically wrong with my code or my understanding of async await?

Any help is much appreciated.

Edit: As per request in the comments I'm adding a simplified version of the method that calls GetServersStatusAsync and how I call that method (in a fire and forget fashion, but Login is wrapped with a try catch so that shouldn't be a big issue).

public async Task Login(Action<LoginResult> callback = null)
{

    LoginResult result = new LoginResult() { Success = false };
    try
    {
            var serverStatus = await GetServersStatusAsync();
            if (serverStatus.Any())
            {
                result.Success = true;
                callback?.Invoke(result);
                return;
            }
    }
    catch (Exception ex)
    {
        result.Message = Strings.UnknownException;
        _log.Error("Login failed due to unexpected exception", ex);
    }
    callback?.Invoke(result);
}
 _restClient.Login(OnLoginResponse);
binginsin
  • 104
  • 6
  • 1
    How do you call the `GetServersStatusAsync`? – Theodor Zoulias Aug 12 '20 at 16:03
  • 1
    @TheodorZoulias I call it like this: var serverStatus = await GetServersStatusAsync(); – binginsin Aug 12 '20 at 16:05
  • 1
    Ideally, if your exception is thrown in an asynchronous fashion, the exception is thrown onto some `ThreadPool` thread that is supposed to execute the continuation of your awaitable. It must be throwing asynchronously (after the `await` is called) in the above case and since you're attaching ConfigureAwait(false), it's not being captured on the same thread context. More here: https://stackoverflow.com/questions/35136801/configureawait-on-which-thread-is-the-exception-handled – Sai Gummaluri Aug 12 '20 at 16:17
  • 1
    @SaiGummaluri Right, but is the try block a context? I would've thought it can still capture the exceptions even in different context? – binginsin Aug 12 '20 at 16:26
  • 1
    Do you have any error handling at the site where you call the `GetServersStatusAsync`? I am asking because the `GetServersStatusAsync` itself is swallowing exceptions, so this could be a pattern repeated in the whole application. – Theodor Zoulias Aug 12 '20 at 16:27
  • 1
    @TheodorZoulias Yes I do have error handling there, the exception never gets to that point... It just disappears in the threadpool I think... – binginsin Aug 12 '20 at 16:31
  • 1
    Could you include in your question the method that calls the `GetServersStatusAsync`? – Theodor Zoulias Aug 12 '20 at 16:38
  • @TheodorZoulias I have included the code that calls it. – binginsin Aug 12 '20 at 16:58
  • 2
    @DainiusCerniauskas what's the point of `callback?.Invoke(result)` when you already use `async/await`? `await` returns execution to the original sync context. If `Login` is called from the UI thread, you only need to call `callback(result)`. If you *have* to use `Invoke` it means you're calling `Login` from a background thread. Are you calling `Login` from an `async void` method perhaps? – Panagiotis Kanavos Aug 12 '20 at 16:59
  • `_restClient.Login(OnLoginResponse);` this line never awaits for `Login` to complete and doesn't even keep track of the `Task` returned. The calling code never knows if login succeeded or not. If this code is used eg in an ASP.NET context where request threads are returned to the thread pool as soon as a request ends, the code may end up getting orphaned – Panagiotis Kanavos Aug 12 '20 at 17:04
  • @PanagiotisKanavos Is callback.Invoke different from callback()? I thought both are basically the same thing https://stackoverflow.com/questions/7907067/difference-between-actionarg-and-action-invokearg . To be fair, the idea was to make a call to Login and not wait for it and have it invoke a callback method where further processing is done, but I can actually wait for the Login to complete, in a synchronous fashion, because unfortunately the first calling method is not asynchronous. – binginsin Aug 12 '20 at 17:08
  • From what I see you are logging the errors, but you are not logging the successful executions. I suggest to add more thorough logging because my suspicion is that your problem is not related to exception handling specifically, but with how you have wired up your application together. I can see some idiomatic code in there. – Theodor Zoulias Aug 12 '20 at 17:12
  • @TheodorZoulias no, my problem is actually with exception handling, as I have stepped through the code with a debugger and I am experiencing the issue I talked about initially. Successful executions work perfectly fine. – binginsin Aug 12 '20 at 17:32
  • I am not a big fan of the debugger, and especially when it comes to async code. Logging is more trustworthy IMHO. – Theodor Zoulias Aug 12 '20 at 17:59
  • @TheodorZoulias Yes, I agree with you on that. The problem is on an exception during execution the messages are not logged as the exception is not propagated, and I'm trying to figure out why it is not propagated. – binginsin Aug 12 '20 at 18:06
  • 1
    I suggest to strip down repeatedly your code, removing all unrelated parts until you have a minimal reproducible example of the issue, and post it here. Then we will be able to verify if it's actually a problem of .NET's error handling, in which case we could use your example to submit a bug report to Microsoft. – Theodor Zoulias Aug 12 '20 at 22:33

0 Answers0