Skip to content

Instantly share code, notes, and snippets.

@ayende ayende/http.cs
Last active Apr 3, 2019

Embed
What would you like to do?
static HttpClient client = new HttpClient{ BaseAddres = "http://api.server.url/" };
static async Task<Product> GetProductAsync(string id)
{
Product product = null;
HttpResponseMessage response = await client.GetAsync( $"api/products/{id}");
if (response.IsSuccessStatusCode)
{
product = await response.Content.ReadAsAsync<Product>();
}
return product;
}
@ghuntley

This comment has been minimized.

Copy link

commented Apr 3, 2019

It's good enough to serve production as is once ConfigureAwait(false) is respected.

static HttpClient client = new HttpClient{ BaseAddress = "http://api.server.url/" };

static async Task<Product> GetProductAsync(string id)
{
    Product product = null;
    HttpResponseMessage response = await client.GetAsync($"api/products/{id}").ConfigureAwait(false);
    if (response.IsSuccessStatusCode)
    {
        product = await response.Content.ReadAsAsync<Product>().ConfigureAwait(false);
    }
    return product;
}

If one wants to gold plate:

  • configure BaseAddress / HttpClient via ctor if you want to go the DI route.
  • add correlationid/useragent/assembly-[name+version]-of-caller headers via httpmessagehandler into HttpClient.
  • consider metrics/logging and make it the responsibility of a calling handler.
  • look into retry strategies and make responsibility of the caller but consider if you need access to the responseCode to make such a decision
@ghuntley

This comment has been minimized.

Copy link

commented Apr 3, 2019

tbh it's all boilerplate that can and should be code generated via refit. See https://github.com/reactiveui/refit/tree/master/samples/Meow

tldr: burn it, use refit

@gortok

This comment has been minimized.

Copy link

commented Apr 3, 2019

In addition to what’s been said already, I would add domain sanity checking around “id”. Cut off the call if they try to send an invalid domain Id in, and use a modelbinder/IsModelStateValid to ascertain whether the resulting API call should even be made.

Going further depends on the purpose of the code.

Is the code meant to hide that another URL is being called for this information entirely?
Or is it meant to proxy a call server side to get around CORS issues or to pass along information we don’t want a client to see/use (like an internal API key)?
Or is there another reason?

Depending on the reason And how the resulting API reacts to the following I’d include returns that handle the different types of 40X and 5XX errors that could come out of the proxy API call and give the user actionable yet friendly errors if something goes wrong.

At this point, there are two consumers of this API, us and by proxy (pun not intended) our users, and it’s a coin flip which one will see an issue in this other API first.

Purely a personal choice, but I’d include a wrapper viewmodel that included products as a part of it; but had a strongly typed model that included all these other things we would want to show ina view or log internally depending on what happened:
Error
ErrorMessage
API HttpErrorCode
RequestTimeDateUtc
Response (at least log in the case of errors if you want to hide the response from the user)
Product
DetailedError
Back off
RetryTimes

If this is .NET Core 2.x or better there’s also an updated HTTPClientFactory that is preferred for use over the raw client. HttpClient is mostly threadsafe (there are properties that aren’t), but none of those properties are present in this example.

I’m unclear on the need/reason for this to be static; but barring more information it’s not to me to say whether it should be changed. I’d have to see the larger context around this code to know whether that’s the right direction or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.