reactiveui / refit

The automatic type-safe REST library for .NET Core, Xamarin and .NET. Heavily inspired by Square's Retrofit library, Refit turns your REST API into a live interface.
https://reactiveui.github.io/refit/
MIT License
8.58k stars 747 forks source link

Custom ApiException #272

Closed geocine closed 4 years ago

geocine commented 8 years ago

Is there a way to inject a custom implementation of Refit.ApiException ?

bennor commented 8 years ago

Not at the moment. Why do you need a custom implementation?

geocine commented 8 years ago

I am currently creating a wrapper for a third party api using refit . I would then distribute this as a part of the SDK for our team. I would like to encapsulate the exception into an error response object. I know this is possible using try catch and getting Refit.ApiException. But I wouldn't like to leak the Refit dependency , the dependency should only be reference on the api wrapper (class library). If this is not clear I could post some sample code

bennor commented 8 years ago

If you catch the exception in your wrapper then rethrow as your own exception, you won't be exposing a dependency on Refit.

If you want to submit a PR to make it pluggable you're welcome to but -- given the fairly easy workaround above -- I'm not sure it's worth the effort.

geocine commented 8 years ago

Ok how would I be able to catch it from my wrapper here is a sample code:

Wrapper

namespace WrapperApi.Clients
{
    public interface IUserClient
    {
        [Get("/api/Users/{userId}")]
        Task<User> GetUserDetails(Guid userId);
   }
}
namespace WrapperApi
{
    public static class WrapperService
    {
        private static HttpClient GetClient()
        {
            return new HttpClient(new ApiHttpClientHandler()) {BaseAddress = new Uri(Configuration.ServiceBaseUri)};
        }

        public static IUserClient User {
            get
            {
                return RestService.For<IUserClient>(GetClient());
            }
        }

    }
}

Using the Wrapper

namespace WebAPI.Controllers
{
    [RoutePrefix("api/User")]
    [TokenAuthorize]
    public class UserController : ApiController
    {
        [HttpGet]
        [Route("GetUserDetails")]
        public async Task<User> GetUserDetails()
        {
            var userId = User.Identity.GetUserId().AsGuid();
            return await WrapperService.User.GetUserDetails(userId);
        }
   }
}

Where should I catch the Refit.ApiException and rethrow?

bennor commented 8 years ago

Ah, I see. You'd need to create a class that also implemented the interface, then delegate to our implementation:

public class DelegatingUserClient : IUserClient
{
    private IUserClient _refitClient = RestService.For<IUserClient>(WrapperService.GetClient());

    public async Task<User> GetUserDetails()
    {
        try { return await _refitClient.GetUserDetails(); }
        catch(ApiException ex) { /* do your thing */ }
    }
}

I can see how that's not ideal though -- if you have a lot of different endpoints, there would be a lot of boilerplate.

If you want to submit a PR to do add pluggable exception handling, you'd want to look at something like adding a Func<Uri, HttpMethod, HttpResponseMesage, RefitSettings, Task<Exception>> to RefitSettings, and plugging it in here and here.

ApiService.Create shows how we create the ApiException.

geocine commented 8 years ago

Thanks @bennor I will take a look again later

joseprl89 commented 7 years ago

@geocine You can pass a HttpMessageHandler into the HttpClient Refit uses, and in the implementation of:

protected internal abstract Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken);

Capture any exception and wrap it with your own. That way you just use a single try/catch instead of multiple and avoid the wrappers.

unleashed-jamest commented 4 years ago

@joseprl89 I wound up doing this in a recent project. Given the errors are always in a consistent format, I initially thought I might be able to provide my own model to deserialize to in the case of an error, but like the OP of this issue I found it wasn't pluggable. It took a little while to realize I could just write a DelegatingHandler for it. What I wound up doing was writing a DelegatingHandler that passed in a list of error resolving strategies and uses the context of the request message + exception (or response message if it didn't throw exception but is non 200 response) and decides if it is able to resolve an error message, if not pass to the next resolver in the chain and the first one to resolve an error message wins. Works pretty well, though I would appreciate the pluggability for simpler cases.

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.