Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to handle Failure while connecting to API #273

Closed
mdawood1991 opened this issue Oct 3, 2016 · 21 comments
Closed

How to handle Failure while connecting to API #273

mdawood1991 opened this issue Oct 3, 2016 · 21 comments

Comments

@mdawood1991
Copy link

How to I handle can't connect to the API exception.

Currently my app crashes due to the following exception not been handled.

{System.Net.WebException: Error: ConnectFailure (Connection refused) ---> System.Net.Sockets.SocketException: Connection refused
  at System.Net.Sockets.Socket.Connect (System.Net.EndPoint remoteEP) [0x000cb] in /Users/builder/data/lanes/3415/7db2aac3/source/mono/mcs/class/System/System.Net.Sockets/Socket.cs:1313 
  at System.Net.WebConnection.Connect (System.Net.HttpWebRequest request) [0x0019b] in /Users/builder/data/lanes/3415/7db2aac3/source/mono/mcs/class/System/System.Net/WebConnection.cs:195 
  --- End of inner exception stack trace ---
  at System.Net.HttpWebRequest.EndGetRequestStream (IAsyncResult asyncResult) [0x00043] in /Users/builder/data/lanes/3415/7db2aac3/source/mono/mcs/class/System/System.Net/HttpWebRequest.cs:882 
  at System.Threading.Tasks.TaskFactory`1[TResult].FromAsyncCoreLogic (IAsyncResult iar, System.Func`2 endFunction, System.Action`1 endAction, System.Threading.Tasks.Task`1 promise, Boolean requiresSynchronization) [0x00014] in /Users/builder/data/lanes/3415/7db2aac3/source/mono/external/referencesource/mscorlib/system/threading/Tasks/FutureFactory.cs:550 
--- End of stack trace from previous location where exception was thrown ---

This is my API Interface

    public interface IUserService
    {
        [Post("/Account/Authenticate")]
        Task<User> Authenticate([Body] User user);
    }

Then this is how I initialize and call the service

            var _userService = RestService.For<IUserService>("http://192.168.0.100:44314/api");
            user = await _userService.Authenticate(user);
@geocine
Copy link

geocine commented Oct 3, 2016

Implement a custom HttpClientHandler

@mdawood1991
Copy link
Author

@geocine is there an example for this somewhere?

@ahmedalejo
Copy link

@mdawood1991 please take a look at the following issues:

@ahmedalejo
Copy link

silly but why isn´t a try/catch block enough?

@mdawood1991
Copy link
Author

@ahmedalejo I am already using a try catch on the part where I am calling the service:

            try
            {
                user = await _userService.Authenticate(user);
                return user;
            }
            catch (ApiException apiException)
            {
                var apiValidation = apiException.GetContentAs<ValidationContainer>();
                validationContainer.AddMessage(MessageTypeEnum.Error, apiValidation.Message);
                return null;
            }

but when the API/server is down the exception is not caught.

@Cheesebaron
Copy link
Contributor

GetContentAs, won't it call the service?

@mdawood1991
Copy link
Author

This is what is happening below.

I have a try catch(ApiException ex) and catch(Exception ex) - The ApiException happens when the server is up and running and the server response HTTPStatusCode != 200, but when the sever is down the Exception ex happens.

I don't want to add two types of catch wherever I make a server/api call - There must be a way to assign a generic exception handler to the refit service.

test

@clairernovotny
Copy link
Member

There are two different things going on though...semantically, they are different exceptions. The ApiException is for when the API itself is throwing and WebException is when the wire/server/network itself is the issue. I'm not sure it really makes sense to combine them as you'd typically want to take different actions based on those.

For WebException you may want to retry a few times. For an ApiException you'd likely simply fail.

@mdawood1991
Copy link
Author

@onovotny yes - but there must be some sort of Exception handler I can do for the refit service - I don't want to add multiple catch statements whenever there is a API service call.

@geocine
Copy link

geocine commented Oct 3, 2016

@mdawood1991 Actually I opened an issue a wihle ago that relates to your issue #272

@mdawood1991
Copy link
Author

@geocine your issue is related to ApiException. ApiException gets called when there is a response other than 200 from the server. What I want to handle is API Access errors/exceptions e.g Http Timeout, Http server refused connection to socket, or website not available etc.

@bennor
Copy link
Contributor

bennor commented Oct 3, 2016

This has been on my radar for a while. We should probably split ApiException into ApiRequestException and ApiResponseException that derive from a common base. We're not handling request exceptions (e.g. dns errors, etc) at all at the moment, which is why you get the raw WebException.

A workaround in the meantime (if using C#6) is:

catch(Exception ex) when (ex is ApiException || ex is WebException)
{
    // do whatever you need to
}

I've actually used something similar to deal with retries for transient errors (this was before I knew about Polly):

public class TransientExceptionHelper
{
    private static readonly ISet<HttpStatusCode> TransientErrorStatusCodes = new HashSet<HttpStatusCode>(new[]
    {
        HttpStatusCode.BadGateway,
        HttpStatusCode.GatewayTimeout,
        HttpStatusCode.InternalServerError,
        HttpStatusCode.ServiceUnavailable,
        HttpStatusCode.RequestTimeout
    });
    public static bool IsTransient(Exception exception)
    {
        var apiException = exception as ApiException;
        if (apiException != null)
        {
            return TransientErrorStatusCodes.Contains(apiException.StatusCode);
        }
        return exception is HttpRequestException || exception is OperationCanceledException;
    }
}

// Usage
catch (Exception transientEx) when (TransientExceptionHelper.IsTransient(transientEx))
{
    // Handle retries
}

@mdawood1991
Copy link
Author

@bennor I am using Polly like this:

            try
            {
                var userAssets = await Policy
                                   .Handle<Exception>()
                                   //.RetryAsync(retryCount: 1)
                                   .WaitAndRetryAsync(new[]
                                   {
                                     TimeSpan.FromSeconds(1),
                                     TimeSpan.FromSeconds(2),
                                   })
                                   .ExecuteAsync(async () => await _assetService.GetAssetsSummaryList(filter));

                return userAssets;
            }
            catch (ApiException apiException)
            {
                //Some Server error e.g. 500, 401, 403 etc.

                apiException.HandleApiException(validationContainer);
            }
            catch (Exception exception)
            {
                // HTTP RequestTimeout, ServiceUnavailable
                exception.HandleWebException(validationContainer);
            }
            return null;

Is this the best way to do it using Polly?

@viralsavaniIM
Copy link

viralsavaniIM commented Oct 26, 2016

@mdawood1991 I am fairly new at using Polly but I have been using it like this:

Policy
.Handle<Exception>()
.WaitAndRetryAsync
(
  new TimeSpan[]{TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(3)},
  (exception, timespan) =>
      {
       // TODO: Replace Console with log
       if (exception.GetType() == typeof(ApiException))
       {
           // API is throwing
        }
        else
        {
         // any other exceptions
        }

        Console.WriteLine("Exception: " + exception.Message + exception.StackTrace);
      }
)
.ExecuteAsync(async () => await _ApiCache.GetAllPostsCached(userId));

@ghost
Copy link

ghost commented Oct 24, 2018

Is there a way to not throw an exception ? I mean, this will dramatically kill performances. Besides, it's not really an exception since we got a response. An exception would be the server not responding at all.

@rekosko
Copy link

rekosko commented Mar 31, 2020

I still don't get it why this library is throwing ApiException when status code is not successful. I agree with @oooolivierrrr - you got actual response so why treat it as exception? I'm returning e.g. BadRequest respones from the api that are already wrapped into custom WebApiResponse that contains some extras that I may use on the client and if I want to not deal with exceptions in my case I would need to define my api client as: Task<ApiResponse<WebApiResponse<User>>> Authenticate([Body] User user); which is hilarious and wrapping client calls with try catch is out of option because this is not a valid solution and just a hacky way to make it actually work.

@angelru
Copy link

angelru commented Sep 28, 2020

Any solution ???

@rekosko
Copy link

rekosko commented Sep 29, 2020

@angelru new feature appeared - https://github.com/reactiveui/refit#handling-exceptions in RefitSettings. Now you can write your custom ExceptionFactory.

GitHub
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. - reactiveui/refit

@angelru
Copy link

angelru commented Sep 29, 2020

@rekosko It is not very clear to me how to use it

@clairernovotny
Copy link
Member

Closing due to age. Please try Refit v6 and reopen if still an issue.

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants