Retrofit error handling

Question

I wrapped my Retrofit code in a class like below. If it's not clear from the code I'm posting it's interacting with a restful service with OAuth.

What would be a good way to do error handling? The REST server returns an error message in json format. I would like to act on that message by throwing some exceptions from my class. I'm trying to do something like below. But is this good design? Is mixing callbacks and exception throwing a good idea? Is there a better way?

With the approach below I could get i18l messages from within my custom exceptions and toast them to the user.

public class RestClient implements IRestClient {
    private IRestAPI api;

    /**
     *
     * @param accessToken
     */
    public RestClient(final String accessToken)
    {
        RequestInterceptor requestInterceptor = new RequestInterceptor()
        {
            @Override
            public void intercept(RequestFacade request) {
                request.addHeader("Authorization", "Bearer " + accessToken);
            }
        };

        RestAdapter restAdapter = new RestAdapter.Builder()
                .setEndpoint(Config.ENDPOINT)
                .setRequestInterceptor(requestInterceptor)
                .build();
        api = restAdapter.create(IRestAPI.class);
    }

    @Override
    public void requestSomething(final Callback callback) {
        api.getSomething(new Callback<Something>() {
            @Override
            public void success(Something something, Response response) {
                callback.success(something, response);
            }

            @Override
            public void failure(RetrofitError error) {
                if(error.getMessage().getId().euqals(ACCESS_TOKEN_EXPIRED))
                {
                    throw new AccessTokenExpired();
                }
                else if(error.getMessage().getId().euqals(USER_NOT_FOUND))
                {
                    throw new UsernamePasswordNotFound();
                }
                else // something else happened...
                {
                    throw error;
                }
            }
        });
    }

    @Override
    public void deleteSomething(final Callback callback) {
        api.deleteSomething(new Callback<Something>() {
            @Override
            public void success(Something something, Response response) {
                callback.success(something, response);
            }

            @Override
            public void failure(RetrofitError error) {
                if(error.getMessage().getId().euqals(SOMETHING_NOT_FOUND))
                {
                    ...
                    ...
                    Different exceptions
                }
                ...
            }
        });
    }

}

Naturally I would have to create my own call back interface with only a success method.

Answer 1

When you build the RestAdapter, you can provide an error handler that maps out to your custom exceptions, it bypasses the call to failure in the Callback<T> on anything 4xx/5xx. As a really contrived example:

public class Scratch {
    public static void main(String[] args) {
        Endpoints e = new RestAdapter.Builder()
                .setEndpoint("http://google.com")
                .setLogLevel(RestAdapter.LogLevel.FULL)
                .setErrorHandler(new ErrorHandler() {
                    @Override
                    public Throwable handleError(RetrofitError cause) {
                        switch (cause.getResponse().getStatus()) {
                            case 400:
                                /* Handle the expected body format */
                                cause.getBody();
                                throw new RuntimeException("Bad Request");
                            default:
                                /* Things and stuff */
                                throw new RuntimeException("");
                        }
                    }
                })
                .build()
                .create(Endpoints.class);

        e.getGoogle(new Callback<Response>() {
            @Override
            public void success(Response response, Response response2) {
                System.out.println("Got it");
            }

            @Override
            public void failure(RetrofitError error) {
                System.err.println("This won't ever be seen due to the error handler.");
            }
        });
    }

    private static interface Endpoints {
        @GET("/foo/bar")
        void getGoogle(Callback<Response> callback);
    }
}

edit: By doing this, however, you're potentially sacrificing a big reason why you'd want to use the Callback interface to begin with. If this is a common use you will need, it may make more sense to use the sync calls and return your object type. I don't fully know your use to say that's necessary, but it seems as though it may be more appropriate.