CODEWITHSUNDEEP
c#asynchronousxamarin.android
Apidcloud
Apidcloud

Reputation: 4058

Async Download and Deserialize

Good evening,

I am trying to refine some code using async programming and would like to know if the following code is well written or not, and if is there any way to improve it.

Its purpose is to download some JSON from a given URL and deserialize it to an object.

I have three (now four) questions (and 1 issue described at the end of the post):

Without further ado, here's the code:

internal static class WebUtilities
{
    /// <summary>
    /// Downloads the page of the given url
    /// </summary>
    /// <param name="url">url to download the page from</param>
    /// <param name="cancellationToken">token to cancel the download</param>
    /// <returns>the page content</returns>
    internal static async Task<string> DownloadStringAsync(string url, CancellationToken cancellationToken)
    {
        try
        {
            // create Http Client and dispose of it even if exceptions are thrown (same as using finally statement)
            using (var client = new HttpClient() {Timeout = TimeSpan.FromSeconds(5)})
            {
                // should I always do this?
                client.CancelPendingRequests();

                // do request and dispose of it when done
                using (var response = await client.GetAsync(url, cancellationToken).ConfigureAwait(false))
                {
                    // if response was successful (otherwise it will return null)
                    if (response.IsSuccessStatusCode)
                    {
                        // return its content
                        return await response.Content.ReadAsStringAsync().ConfigureAwait(false);
                    }
                }
            }
        }
        catch (Exception ex) when (ex is System.Net.Sockets.SocketException || 
                                    ex is InvalidOperationException || 
                                    ex is OperationCanceledException ||
                                    ex is System.Net.Http.HttpRequestException)
        {
            WriteLine("DownloadStringAsync task has been cancelled.");
            WriteLine(ex.Message);
            return null;
        }

        // return null if response was unsuccessful
        return null;
    }

    /// <summary>
    /// Downloads Json from a given url and attempts its deserialization to a given reference type (class)
    /// </summary>
    /// <typeparam name="T">the class to deserialize to</typeparam>
    /// <param name="url">url to download the json from</param>
    /// <param name="cancellationToken">token to cancel the download</param>
    /// <returns>the deserialized object</returns>
    internal static async Task<T> DownloadJsonAndDeserialize<T>(string url, CancellationToken cancellationToken) where T : class, new()
    {
        // download json from the given url
        var jsonResponse = await DownloadStringAsync(url, cancellationToken).ConfigureAwait(false);

        // if the response is invalid, no need to go further
        if (string.IsNullOrEmpty(jsonResponse))
            // return a default constructor instance
            return new T();

        // try to deserialize
        try
        {
            // Deserialize json data to the given .NET object
            // Should I use TaskEx.Run here?
            return Newtonsoft.Json.JsonConvert.DeserializeObject<T>(jsonResponse);
        }
        catch (Exception ex) when (ex is JsonException)
        {
            WriteLine("Something went wrong while deserializing json to the given reference type.");
            WriteLine(ex.Message);
        }

        // return a default constructor instance
        return new T();
    }
}

And to call the code, one would do something like:

internal static async Task CallAsync()
    {
        RootObject root = null;

        using (var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)))
        {
            var token = cts.Token;

            root = await WebUtilities.DownloadJsonAndDeserialize<RootObject>(URL, token).ConfigureAwait(false);
        }

        // do something with rootObject
        // any good way to know if rootObject was successfully created, before moving on?
    }

What would you change? and why?

Thanks!

Edit:

Issue found (created another question for it - Timeouts in Xamarin HTTP requests):

Interestingly enough, when a host cannot be reached (e.g., an offline local server), nothing happens after GetAsync. After several minutes (around 3) a System.Net.WebException is thrown saying Error: ConnectFailure (Connection timed out). The Inner exception is System.Net.Sockets.SocketsException (full log here: http://pastebin.com/MzHyp2FM).

I have tried to set client.Timeout to 5 seconds but that doesn't seem to work.

Could be a Xamarin bug, though (https://forums.xamarin.com/discussion/5941/system-net-http-httpclient-timeout-seems-to-be-ignored).

Either way, shouldn't cancellationToken cancel automatically after 10 seconds?

This timeout issue happens with/when:

The code works well with/when:

Conclusions

Upvotes: 0

Views: 2968

Answers (2)

Akash Kava
Akash Kava

Reputation: 39956

I am using following code in production without any issues.

// Creating and disposing HttpClient is unnecessary
// if you are going to use it multiple time
// reusing HttpClient improves performance !!
// do not worry about memory leak, HttpClient
// is designed to close resources used in single "SendAsync"
// method call
private static HttpClient client;


public Task<T> DownloadAs<T>(string url){

    // I know I am firing this on another thread
    // to keep UI free from any smallest task like
    // preparing httpclient, setting headers
    // checking for result or anything..., why do that on
    // UI Thread?

    // this helps us using excessive logging required for
    // debugging and diagnostics

    return Task.Run(async ()=> {

       // following parses url and makes sure
       // it is a valid url
       // there is no need for this to be done on 
       // UI thread
       Uri uri = new Uri(url);

       HttpRequestMessage request = 
          new HttpRequestMessage(uri,HttpMethod.Get);

       // do some checks, set some headers...
       // secrete code !!!

       var response = await client.SendAsync(request,
           HttpCompletionOption.ReadHeaders);

       string content = await response.Content.ReadAsStringAsync();

       if(((int)response.StatusCode)>300){

           // it is good to receive error text details
           // not just reason phrase

           throw new InvalidOperationException(response.ReasonPhrase  
              + "\r\n" + content);
       }
       return JsonConvert.DeserializeObject<T>(content);

    });
}

ConfigureAwait is tricky, and makes debugging more complicated. This code will run without any issues.

If you want to use CancellationToken then you can pass your token in client.GetAsync method.

Upvotes: 0

yaakov
yaakov

Reputation: 5850

Should I use TaskEx.Run to run Newtonsoft.Json.JsonConvert.DeserializeObject<T>?

Throwing the JSON deserialisation onto another thread likely won't achieve anything. You're freeing up the current thread but then need to wait for your work to be scheduled on another thread. Unless you need Newtonsoft.Json APIs specifically, consider using ReadAsAsync<T>. You can use that with a JsonMediaTypeFormatter, which uses Newtonsoft.Json internally anyway.

Is there any good way (without checking the object properties) to know if rootObject was successfully created?

You need to define what success looks like. For example, your JSON could be null, so rootObject is null. Or it could be missing properties, or your JSON has extra properties that were not deserialised. I don't think there's a way to fail the serialisation for excess or missing properties, so you would have to check that yourself. I could be wrong on this point.

Should I check somewhere whether Cancellation was requested or not?

You need to see what makes sense in your code and when it is being cancelled. For example, there's no point acting on the cancellation token as the very last line in your function, when all the work has been completed.

Does it make sense for your application to cancel the operation if the JSON has been downloaded, but not yet deserialised? Does it make sense for your application to cancel the operation if the JSON has been deserialised, but the object graph has not yet been validation (question 2)?

It really depends on your needs, but I'd probably cancel if the download has not yet completed - which it seems that you are already doing - but once the download is completed, that is the point of no return.

What would you change? and why?

Use ConfigureAwait(false) on the Tasks that you await. This prevents deadlocks if your code ever blocks and waits for the resulting Task (e.g. .Wait() or .Result).

Use using blocks instead of try-finally.

Upvotes: 2

Related Questions