-1

There is an error in one of my Asynchronous methods, but it never returns anything. I thought it would return a Task with a null value. I need it to return something so that I can continue the LoadCollection() method even if there is an error.

The error occurs at the serializer.Deserialize() method. However, this Newtonsoft method doesn't have any Exceptions written for it.

internal async Task LoadCollection(Project activeProject)
{
    Collections collections = await HttpClientInstance.DownloadCollectionAsync(activeProject); //never returns anything
    if (collections != null) //never hits this line
    {
        MyCollection = new ObservableCollection<WindowEntity>(collections.Collection.Select(x => x.Material).ToList());
    }           
    return;
}

public static async Task<Collections> DownloadCollectionAsync(Project activeProject)
{
    HttpRequestMessage msg = new HttpRequestMessage
    {
        Method = HttpMethod.Get,
        RequestUri = new Uri(HttpClientInstance.BaseUri + "projects/get-collections/")
    };
    msg.Headers.Add("projectID", $"{activeProject.Id}");
    string responseString = null;

    using (var response = await httpClient.SendAsync(msg, HttpCompletionOption.ResponseHeadersRead).ConfigureAwait(false))
    {
        if (response.IsSuccessStatusCode)
        {
            try
            {
                using (Stream s = await response.Content.ReadAsStreamAsync())
                using (StreamReader sr = new StreamReader(s))
                using (JsonReader reader = new JsonTextReader(sr))
                {
                    JsonSerializer serializer = new JsonSerializer();
                    return serializer.Deserialize<Collections>(reader); //error from this deserialization
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show(ex.Message);
            }
        }
    }
    return null;
}

What am I doing wrong with my error handling that causes this LoadCollection() method to never finish?

Ryan Daley
  • 15
  • 6
  • Debug and see where the code is hanging? – Ralf Mar 28 '23 at 09:45
  • 1
    You should never return a task with null value in async mode. Read more here to understand it: https://stackoverflow.com/questions/45347160/is-it-better-to-return-an-empty-task-or-null-c-sharp – D A Mar 28 '23 at 09:46
  • 1
    I recommend to use the .NET JSON API. It exposes an async API which would improve your code. It's very convenient to use. [How to serialize and deserialize (marshal and unmarshal) JSON in .NET](https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/how-to?pivots=dotnet-7-0). Additionally Stream implements IAsyncDisposable. IAsyncDisposable exposes DisposeAsync() which you should await in your async context. Consider to return an empty Collections instance instead of NULL. – BionicCode Mar 28 '23 at 10:11
  • Also change the `HttpCompletionOption` to `ResponseContentRead`. Otherwise you will end up deserializing an incomplete response message which will cause the deserializer to hang. – BionicCode Mar 28 '23 at 10:26
  • @BionicCode doesn't deserializing a stream prevent it from hanging? I thought this would only be the case if I was reading a string. – Ryan Daley Mar 28 '23 at 10:43
  • @DA they are not returning a `null` task, they are returning a `Task` which has a `null` result. If the method wasn't marked `async` then you'd be right. – Charlieface Mar 28 '23 at 11:02
  • @BionicCode If the stream ends then the deserializer will throw an EOF exception, and if the stream hangs then you have other problems anyway. There is no need to buffer the whole stream. – Charlieface Mar 28 '23 at 11:04
  • How do you know you are getting an error if it hangs and doesn't return? – Charlieface Mar 28 '23 at 11:05
  • 1
    `MessageBox.Show` is a UI method (it shows UI). So it must be run on a UI context. But I see `ConfigureAwait(false)`, which means it is likely running on a thread pool context instead of a UI context. – Stephen Cleary Mar 28 '23 at 12:59

1 Answers1

-2

First thing you need to know is that running an async method without using an await will not throw any exception. but you can use an extension method called ContinueWith to catch those exceptions, for example:

 Task<SomeObject> myTask = LoadAllTheThingsAsync();

 myTask.ContinueWith(P => 
 { 
     if (P.Exception != null) MessageBox.Show(ex.Message);
 }

And remember that Exceptions are not bad. Throw exceptions wherever your code doesn't works as you wanted. It helps you a lot during development phase. In your code you totally ignored non-successful HttpStatusCodes.

RezaNoei
  • 1,266
  • 1
  • 8
  • 24