android10 / Android-CleanArchitecture

This is a sample app that is part of a series of blog posts I have written about how to architect an android application using Uncle Bob's clean architecture approach.
Apache License 2.0
15.52k stars 3.32k forks source link

What about global error handling? #170

Open ZherebtsovAlexandr opened 8 years ago

ZherebtsovAlexandr commented 8 years ago

In the project error handling is show error message in presentation layer.

But if we need add exception NotAuthorizedException in data layer and handle all error from RestApi with 401 code error. In presentation layer we need handle this error and show AuthActivity.

@Override 
public void onError(Throwable e) {
    UserDetailsPresenter.this.hideViewLoading();    
    if (e instanceof NotAuthorizedException) {
        UserDetailsPresenter.this.showAuthScreen();
    } else {
        UserDetailsPresenter.this.showErrorMessage(new DefaultErrorBundle((Exception) e));
    }
    UserDetailsPresenter.this.showViewRetry();
}  

Can we have a global error handler for each presenter (preferably for the entire application at once) for certain types of errors? Thus we do not have to duplicate code in each subscriber, furthermore, we can have a default subscriber.

Perhaps this approach is a bit contrary to RxJava philosophy, but in this case it seems to me it is justified.

Any ideas on this?

Domain

public interface RestApiErrorHandling {
    void handle (Throwable throwable);
} 

public abstract class UseCase {

    private RestApiErrorHandling restApiErrorHandling;

    publlic void registerErrorHandling(RestApiErrorHandling restApiErrorHandling) {
        this.restApiErrorHandling = restApiErrorHandling;                    
    }

    protected abstract Observable buildUseCaseObservable();

    public void execute(Subscriber UseCaseSubscriber) {
        this.subscription = this.buildUseCaseObservable()        
            .doOnError(throwable -> {
                if (restApiErrorHandling != null) {
                    restApiErrorHandling.handle(throwable);
                }
            })
            .subscribeOn(Schedulers.from(threadExecutor))
            .observeOn(postExecutionThread.getScheduler())
            .subscribe(UseCaseSubscriber);
    }

}

Presentation

public class UserDetailsPresenter implements RestApiErrorHandling {

    private final UseCase getUserDetailsUseCase;
    private final UseCase shareUserUseCase;
    private final UseCase addToFavoriteUserUseCase;

    public class UserDetailsPresenter(@Named("GetUserDetailsUseCase") UseCase getUserDetailsUseCase,
                                      @Named("ShareUserUseCase") UseCase shareUserUseCase,
                                      @Named("AddToFavoriteUserUseCase") UseCase addToFavoriteUserUseCase) {
        this.getUserDetailsUseCase = getUserDetailsUseCase;
        this.shareUserUseCase = shareUserUseCase;
        this.addToFavoriteUserUseCase = addToFavoriteUserUseCase;
        getUserDetailsUseCase.registerErrorHandling(this);
        shareUserUseCase.registerErrorHandling(this);
        addToFavoriteUserUseCase.registerErrorHandling(this);
    }

    private void getUserDetails() {
        this.getUserDetailsUseCase.execute(new UserDetailsSubscriber());
    }

    public void shareUser() {
        this.shareUserUseCase.execute(new DefaultSubscriber());
    }

    public void addToFavoriteUser() {
        this.addToFavoriteUserUseCase.execute(new DefaultSubscriber());
    }

    @Override
    public void handle (Throwable throwable) {
        if (throwable instanceof NotAuthorizedException){
            this.viewDetailsView.showAuthScreen();
        }
    }    
}

P.S. What about the EventBus?

Error handler in the Data layer can sent error via the EventBus error to the Presentation layer?

For example, if an application is built on the one Activity, it is possible to transmit an error on the main presenter and show auth screen.

It will be correct?

MrVilkaman commented 8 years ago

@ZherebtsovAlexandr My looks for global errors handling.

First of all, few words about responses from our server: Datalayer

public class ResponseWrapper<Value extends BaseResponse,Error extends BaseResponse> {

    @SerializedName("Result")
    private Value value;

    @SerializedName("Error")
    private Error error;

    public ResponseWrapper(Value value, Error error) {
        this.value = value;
        this.error = error;
    }

    public Value getValue() {
        return value;
    }

    public Error getError() {
        return error;
    }

    public Boolean isCorrectResponse() {
        return value != null || error != null;
    }

    public boolean isSuccess() {
        return error == null;
    }
}

BaseResponse is simple:

@SerializedName("Code")
    private int code;
    @SerializedName("Message")
    private String message;

i have specific class for process and convert throwable.

public class DpUtilsImpl implements DPUtils {
    private final Bus bus;

    @Inject
    public DpUtilsImpl(Bus bus) {
        this.bus = bus;
    }

    @Override
    public <T extends BaseResponse, R> Observable.Transformer<ResponseWrapper<T, BaseResponse>, R> handleAnswer(Func1<T, R> act) {
        return obs -> obs
                .first(ResponseWrapper::isCorrectResponse) 
                .onErrorResumeNext(getThrowableObservableFunc1()) // Treatment of common errors
                .concatMap(r -> {
                    if (r.isSuccess()) {
                        return Observable.just(act.call(r.getValue()));
                    } else {
                        return handleError(r); // // Treatment of business errors
                    }
                });
    }

    @Override
    @NonNull
    public <T> Func1<Throwable, Observable<T>> getThrowableObservableFunc1() { // Treatment of common errors
        return throwable -> {
            if (throwable instanceof HttpException) {
                HttpException httpException = (HttpException) throwable;
                Response response = httpException.response();
                return Observable.error(getThrowable(response.message(), response.code(),throwable));
//                  RetrofitException.httpError(response.raw().request().url().toString(), response, retrofit);
            }else if (throwable instanceof IOException ) {
                return Observable.error(new InternetConnectionException());
            }else if (throwable instanceof NetworkErrorException) {
                return Observable.error(new InternetConnectionException());
            }
            return Observable.error(throwable);
        };
    }

    @NonNull
    private <T> Observable<T> handleError(ResponseWrapper<? extends BaseResponse, BaseResponse> r) {
        BaseResponse error = r.getError();
        Throwable exception = getThrowable(error.getMessage(), error.getCode(), null);
        return Observable.error(exception);
    }

    @NonNull
    private Throwable getThrowable(String message, int code, Throwable throwable) {
        Throwable exception;
        switch (code) {
            case 404:
                exception = new NotFoundException();
                break;
            case 401:
                exception = new UncheckedException(message);
                bus.publish(QueriesBus.USER_STATUS_QUEUE, UserStatus.BAD_TOKEN);
                break;
            case 500:
            case 501:
            case 502:
            case 503:
            case 504:
                exception = new ServerException(throwable);
                break;
            default:
                exception = new UncheckedException(message);
                break;
        }
        return exception;
    }
}

So this class convert external exception into my internal exception. It include both retrofit exception and our server http errors;

Also it is point for global notify app about some event (as well as NotAuthorizedException) through EventBus

And now we can simple use it. For example:

@Override
    public Observable<ConfirmResponse> login(String email, String pass) {
        return restApi.login(new LoginRequest(email, pass))
                .compose(dpUtils.handleAnswer(confirmResponse -> confirmResponse));
    }

A good supplement is coverage tests and to be sure that error handling will work equally for all network requests

Presentationlayer

Aslo I have one class for handle erros by default:

public class ThrowableResolverImpl implements ThrowableResolver {

    private UIResolver uiResolver;

    //  @Inject
    public ThrowableResolverImpl(UIResolver uiResolver) {
        this.uiResolver = uiResolver;
    }

    @Override
    public void handleError(Throwable throwable) {
        if (throwable instanceof ServerException) {
            uiResolver.showMessage(R.string.dialog_server_error);
        } else if (throwable instanceof ServerNotAvailableException) {
            uiResolver.showMessage(R.string.dialog_server_notavailable_error);
        } else if (throwable instanceof InternetConnectionException) {
            uiResolver.showSnackbar(R.string.dialog_internet_error);
        } else if (throwable instanceof NotFoundException) {
            uiResolver.showMessage(R.string.dialog_default_404_error);
        } else if (throwable instanceof UncheckedException) {
            uiResolver.showMessage(R.string.dialog_default_error, throwable.getMessage());
        } else {
            uiResolver.showMessage(R.string.dialog_default_error, throwable.getMessage());
        }
    }
}

By the way, if you need change behavior for some request or\and some exception type you can do it in presenter in subscriber

    private static class UserLoginSubscriber extends LoadSubscriber<LoginView,Void> {
        public UserLoginSubscriber(LoginView view) {
            super(view);
        }

        @Override
        public void onCompleted() {
            super.onCompleted();
            view().successLogin();
        }

        @Override
        public void onError(Throwable e) {
            if (e instanceof NotFoundException) {
                view().userNonFoundError(); // New behavior
                skipNextError(); // Remove old behavior
            }
            super.onError(e);
        }

    }

At the time of data such decision completely suits me. What can you say about this?

ZherebtsovAlexandr commented 8 years ago

@MrVilkaman thank u for sharing this!

I like this error handling. I will refactor the code to follow Rx transforms approach.

This code and DPUtils interface from domain layer?

@Override
     public Observable<ConfirmResponse> login(String email, String pass) {
            return restApi.login(new LoginRequest(email, pass))
                    .compose(dpUtils.handleAnswer(confirmResponse -> confirmResponse));
     }

And other questions how you subscribe to Bus event?

MrVilkaman commented 8 years ago

@ZherebtsovAlexandr

This code and DPUtils interface from domain layer?

Now it is part of data provider, but I feel that is not good. Maybe it need move to domain layer and use it in usecase. It will be correct?

And other questions how you subscribe to Bus event? For global event I subscribe to Bus in special class that inject in activity:

public class NavigationAssistantImpl implements NavigationAssistant {
    private final Bus rxBus;
    private final BaseActivityPresenter presenter;
    private final SessionDP sessionDP;
    private Subscription subscribe;

    public NavigationAssistantImpl(Bus rxBus, BaseActivityPresenter presenter, SessionDP sessionDP) {
        this.rxBus = rxBus;
        this.presenter = presenter;
        this.sessionDP = sessionDP;
    }

    @Override
    public void onStop() {
        if (subscribe != null) {
            subscribe.unsubscribe();
        }
    }

    @Override
    public void onStart() {
        subscribe = rxBus.subscribe(QueriesBus.USER_STATUS_QUEUE, new UserStatusObserver());
    }

    private class UserStatusObserver implements Observer<UserStatus> {

        @Override
        public void onNext(UserStatus userStatus) {
            switch (userStatus) {
                case USER_LOGOUT:
                case BAD_TOKEN:
                    sessionDP.logout();
                    presenter.openActivity(MainActivity.class);
                    break;
                case UNKNOWN:
                    break;
            }
        }

        @Override
        public void onCompleted() {

        }

        @Override
        public void onError(Throwable e) {

        }
    }

This code need refactor and I known it =(

Also, for local screen event you can do it in presenter for current screen.

I will refactor the code to follow Rx transforms approach.

Please, notify me when you do this. ok?