wis3guy / HalClient.Net

A library that simplifies interaction with API's that respond with the "application\hal+json" media type.
Microsoft Public License
19 stars 21 forks source link

Wouldn't it better that HalHttpClient won't throw exception by default? #20

Closed zijianhuang closed 7 years ago

zijianhuang commented 7 years ago

HalHttpClient is a wrapper of HttpClient, created by HalHttpClientFactory. The default behavior of HttpClient when having a non-2xx code is not to throw exception, but still to return the response message. To throw exception upon non-2xx codes, application programmers call responseMessage.EnsureSuccessStatusCode().

It could be better that HalHttpClientFactory to deliver HalHttpClient that conforms to the default behavior of HttpClient.

Even though the current codebase has IHalHttpClientConfiguration.OnError, however, it seems that IHalHttpClientConfiguration and HalHttpClientConfiguration are not exposed to application codes. The solution could be just add IHalHttpClientConfiguration to the properties of IHalClient.

wis3guy commented 7 years ago

Have a look at this section of the documentation: https://github.com/wis3guy/HalClient.Net#i-want-to-apply-common-configuration-to-all-ihalhttpclient-instances


You can set the ThrowOnError flag to false if you do not like working with the HalHttpRequestException exceptions, which is the default behavior. As can be seen in the following method (from the HalHttpClient class) an exception is not thrown in case you do so, regardles of the response's status code.

private async Task<IHalHttpResponseMessage> ProcessResponseMessage(HttpResponseMessage response)
{
    if (Configuration.AutoFollowRedirects &&
        ((response.StatusCode == HttpStatusCode.Redirect) ||
            (response.StatusCode == HttpStatusCode.SeeOther) ||
            (response.StatusCode == HttpStatusCode.RedirectMethod)))
        return await GetAsync(response.Headers.Location);

    var message = await HalHttpResponseMessage.CreateAsync(response, _parser);

    if (response.IsSuccessStatusCode || !Configuration.ThrowOnError)
        return message;

    throw new HalHttpRequestException(response.StatusCode, response.ReasonPhrase, message.Resource);
}

Assuming you have set the flag to false, you will be returned an instance of IHalHttpResponseMessage which has a Message property that exposes the received HttpResponseMessage for you to deal with as you see fit.

zijianhuang commented 7 years ago

I did consider extending the factory, but had chosen extending the interface to expose a property that is already available in the class object.

wis3guy commented 7 years ago

I do not want to sound cocky, but if you choose to use this library, why not play by its rules? You seem to want to modify the library to suit your way or working and/or design principles. If you do not like my implementation feel free to fork and modify the client.

Extending the factory to deviate from it's default behavior and do what you seem to want is easy as described.

Exposing such a property would mean you can modify configuration after constructing the factory, which i feel is something to be prevented. You would have to write guard code to prevent undesired and/or unexpected changes.