4
\$\begingroup\$

I'm using a .NET library which uses a pre-async/await type of asynchronicity. That is, it provides asynchronous (non-awaitable) method with a callback parameter.

I'm trying to write awaitable extension methods which will wrap some of those methods to make it easier to use with the async/await fashion but I'm not sure if this is the right approach.

Here's an example of one of those methods:

public void OAuthGetRequestTokenAsync(string callbackUrl, Action<FlickrResult<OAuthRequestToken>> callback)
{
    CheckApiKey();
    string url = //...

    Dictionary<string, string> parameters = OAuthGetBasicParameters();
    // [Code to add parameters]

    FlickrResponder.GetDataResponseAsync(this, url, parameters, (r) =>
    {
        var result = new FlickrResult<OAuthRequestToken>();
        if (r.Error != null)
        {
            if (r.Error is System.Net.WebException)
            {
                var ex = new OAuthException(r.Error);
                result.Error = ex;
            }
            else
            {
                result.Error = r.Error;
            }
            callback(result);
            return;
        }
        result.Result = FlickrNet.OAuthRequestToken.ParseResponse(r.Result);
        callback(result);
    });
}

And here's what I came up with to provide an awaitable extension method which wraps the one above:

static class FlickrExtensions
{
    public static Task<OAuthRequestToken> OAuthGetRequestTokenAsync(this Flickr flickr, 
                                                                    string callbackUrl)
    {
        var tcs = new TaskCompletionSource<OAuthRequestToken>();
        flickr.OAuthGetRequestTokenAsync(callbackUrl, r =>
        {
            if (r.HasError)
                tcs.TrySetException(r.Error);
            else
                tcs.TrySetResult(r.Result);
        });

        return tcs.Task;
    }

    // More extension methods to be added.
}

Are there any problems with my code or any downsides to this implementation?

Also, is there a better (more standard?) way to achieve this goal?

\$\endgroup\$
  • 1
    \$\begingroup\$ Your extension method looks pretty standard for what you are trying to achieve. \$\endgroup\$ – Nkosi May 29 at 13:12
  • \$\begingroup\$ @Nkosi Thanks for the validation! Does that make the question off-topic or anything? \$\endgroup\$ – Ahmed Abdelhameed May 30 at 18:50
  • 1
    \$\begingroup\$ @AhmedAbdelhameed doesn't make it off topic. Just might not get an answer since nothing really to improve. \$\endgroup\$ – CharlesNRice May 30 at 20:10
1
\$\begingroup\$

Error Handling

You are relying on OAuthGetRequestTokenAsync to catch all exceptions for you.

flickr.OAuthGetRequestTokenAsync(callbackUrl, r =>
{
    if (r.HasError)
        tcs.TrySetException(r.Error);
    else
        tcs.TrySetResult(r.Result);
});

Unhandled errors

However, OAuthGetRequestTokenAsync internally calls FlickrResponder.GetDataResponseAsync, which as you can see in the Reference Source does not catch all exceptions.

Most exceptions are caught and provided to the callback:

// .. snippet from FlickrNet.Internals.FlickrResponder.GetDataResponseAsync()
if (e.Error != null)
{
    result.Error = e.Error;
    callback(result);
    return;
}

But some are thrown to the caller:

// .. snippet from FlickrNet.Internals.FlickrResponder.GetDataResponseAsync()
using (var responseReader = new StreamReader(response.GetResponseStream()))
{
    string responseData = responseReader.ReadToEnd();

    throw new OAuthException(responseData, ex);
}

Refactored Code

To wrap this call in an async task, I would also catch these exceptions and handle them in the Task.TrySetException. In addition, since you provide a public API, I suggest to avoid NullReferenceException and check required arguments against null. Perhaps callbackUrl should also be checked early against null. I'm not sure about this, you'd have to verify.

public static Task<OAuthRequestToken> OAuthGetRequestTokenAsync(
    this Flickr flickr, string callbackUrl)
{
    if (flickr == null) throw new ArgumentNullException(nameof(flickr));
    var tcs = new TaskCompletionSource<OAuthRequestToken>();
    try
    {
        flickr.OAuthGetRequestTokenAsync(callbackUrl, r =>
        {
            if (r.HasError)
                tcs.TrySetException(r.Error);
            else
                tcs.TrySetResult(r.Result);
        });
    } 
    catch (Exception uncaughtError)
    {
        tcs.TrySetException(uncaughtError);
    }

    return tcs.Task;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.