Safe async API code

Question

I'm pretty new to async/await and I want to ensure that the code I've written is going to be safe and not cause any deadlocks.

The RestClient in the code will be injected as a singleton dependency via an IoC within an ASP.NET MVC application, so that the access token request is made only once and shared across all API requests within the application.

I'm using Polly as a retry manager to handle expired access tokens - I'm not sure if this is the best approach, but it seems like a good fit right now.

I've simplified the code example somewhat, just to keep things a bit more readable.

I'm really interested in taking on more async code, so I'd be very happy if anyone finds any potential issues with the code or suggestions on better practices.

public class RestClient {

    private TokenResponse _token;

    private const int RetryCount = 2;

    private readonly SemaphoreSlim _syncLock = new SemaphoreSlim(1);

    public async Task<T> MakeRequestAsync<T>(string uri) where T : class {
        return await Policy
            .Handle<InvalidAccessTokenException>()
            .RetryAsync(RetryCount, (exception, i) => InvalidateToken())
            .ExecuteAsync(async () => await ExecuteRequestAsync<T>(uri));
    }

    private async Task<TokenResponse> GetTokenAsync() {

        try {
            await _syncLock.WaitAsync().ConfigureAwait(false);

            if (_token == null) {
                using (var client = new HttpClient()) {
                    client.BaseAddress = new Uri(ProviderBaseUrl);

                    var response = await client.PostAsync(TokenEndpoint, new FormUrlEncodedContent(new[] {
                        new KeyValuePair<string, string>("grant_type", "password"),
                        new KeyValuePair<string, string>("client_id", ClientId),
                        new KeyValuePair<string, string>("client_secret", ClientSecret),
                        new KeyValuePair<string, string>("username", Username),
                        new KeyValuePair<string, string>("password", Password)
                    })).ConfigureAwait(false);

                    _token = await response.Content.ReadAsAsync<TokenResponse>().ConfigureAwait(false);
                }
            }
        }
        finally {
            _syncLock.Release();
        }

        return _token;
    }

    private async Task<T> ExecuteRequestAsync<T>(string uri) where T : class 
    {
        var token = await GetTokenAsync();

        if (token.access_token == null) {
            return null;
        }

        using (var client = new HttpClient()) {

            client.DefaultRequestHeaders.Add("Authorization", "Bearer " + token.access_token);

            var response = await client.GetAsync(new Uri(String.Format("{0}{1}", ProviderBaseUrl, uri))).ConfigureAwait(false);

            if (response.StatusCode == HttpStatusCode.Unauthorized) {
                throw new InvalidAccessTokenException();
            }

            return await response.Content.ReadAsAsync<T>().ConfigureAwait(false);
        }
    }

    private void InvalidateToken() 
    {
        _token = null;
    }
}

Answer 1

Why you don't want to use RestSharp ? Anyway, here is what I think;

• Don't dispose httpClient object on every request.
• Actualy "ExecuteRequestAsync" method doing only get. So it's a bad naming of this function. Maybe you can rename it as "GetAsync"
• Implement an interface for reusage, test & dependency injection.
• Don't set header everytime. Sets once when you get new token.

Maybe an example ( Here is what how I think to implement this code );

    private async Task<T> CheckAndInvokeAsync<T>(Func<string, Task<T>> method)
    {
        string token = string.Empty;

        try
        {
            token = await _tokenProvider.GetTokenIfNullOrExpiredAsync();

            if (!string.IsNullOrEmpty(token))
            {
                ConfigureHeader(); //configure httpClient header. Check if it's new token or old one etc.
                return await method(token);
            }

            var exception = new Exception();
            exception.Data.Add("StatusCode", HttpStatusCode.Unauthorized);
            throw exception;
        }
        catch (Exception ex)
        {
            Debug.WriteLine($"Exception : {ToEpochTime(DateTime.UtcNow)} Token : {token}");

            if (ex.Data.Contains("StatusCode") && ((HttpStatusCode)ex.Data["StatusCode"]) == HttpStatusCode.Unauthorized)
            {
                token = await _tokenProvider.GetTokenAsync();

                if (!string.IsNullOrEmpty(token))
                {
                    return await method(token);
                }
            }

            throw;
        }
    }

And usage like;

    public async Task PostAsJsonAsync(object data, string uri, string clientId)
    {
        await CheckAndInvokeAsync(async token =>
        {
            //_httpClient comes from constructor.
            HttpResponseMessage response = await _httpClient.PostAsJsonAsync(uri, data);

            if (!response.IsSuccessStatusCode)
            {
                var exception = new Exception($"Resource server returned an error. StatusCode : {response.StatusCode}");

                exception.Data.Add("StatusCode", response.StatusCode);

                throw exception;
            }

            return Task.FromResult(0);
        });
    }