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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

What is the correct way to handle errors? #729

Closed
devxoul opened this issue Jun 7, 2016 · 13 comments
Closed

What is the correct way to handle errors? #729

devxoul opened this issue Jun 7, 2016 · 13 comments

Comments

@devxoul
Copy link
Collaborator

@devxoul devxoul commented Jun 7, 2016

Hi, I'm almost new to Rx and trying to understand the philosophy of reactive programming. 馃槃
I encountered the problem in error handling. I read many articles such as #316, #618 but I could not figure out how to handle errors without nesting flatMap or using Result model.

The code below is very similar to GitHubSignUp example in RxExample project. User inputs are passed to usernameInputDidReturn, passwordInputDidReturn, loginButtonDidTap, and login result will be sended back using didComplete observable.

LoginViewController subscribes didComplete directly, not nesting under self.usernameInput.rx_controlEvent or self.loginButton.rx_tap. How can we handle errors in this case?

Currently I'm using the Result model (as @frogcjn mentioned in #316), but I'd like to know if there is more reactive way.

LoginViewController.swift

// Input
self.usernameInput.rx_controlEvent(.EditingDidEndOnExit)
    .bindTo(self.viewModel.usernameInputDidReturn)
    .addDisposableTo(self.disposeBag)

self.passwordInput.rx_controlEvent(.EditingDidEndOnExit)
    .bindTo(self.viewModel.passwordInputDidReturn)
    .addDisposableTo(self.disposeBag)

self.loginButton.rx_tap
    .bindTo(self.viewModel.loginButtonDidTap)
    .addDisposableTo(self.disposeBag)

// Output
self.viewModel.didComplete
    .catchError { [weak self] error in
        // How can I handle error here? I'd like to handle error instance to provide user feedback.
        // It doesn't work but I'd like to do something like this:
        let message = (error as? LoginError)?.message
        self?.displayErrorLabel(message)
    }
    .subscribeNext { [weak self] in
        self?.startNextViewController()
    }
    .addDisposableTo(self.disposeBag)

LoginViewModel.swift

let usernameAndPassword = Observable
    .combineLatest(self.username.asObservable(), self.password.asObservable()) { username, password in
        return (username, password)
    }

// Observable<User>
self.didComplete = Observable.of(self.usernameInputDidReturn,
                                 self.passwordInputDidReturn,
                                 self.loginButtonDidTap)
    .merge()
    .withLatestFrom(usernameAndPassword)
    .flatMapLatest { username, password in
        // func api.login(...) -> Observable<User>
        return api.login(username: username, password: password) // this can emit error
    }

As @kzaher's comment

button.rx_tap
    .flatMapLatest { _ in
         return doManyThings()
               .catchError { handleErrors($0) }
    }
    .subscribeNext { input in
        // do something
    }

I should use such like this in LoginViewModel.swift:

self.didComplete = Observable.of(self.usernameInputDidReturn,
                                 self.passwordInputDidReturn,
                                 self.loginButtonDidTap)
    .merge()
    .withLatestFrom(usernameAndPassword)
    .flatMapLatest { username, password in
        return api.login(username: username, password: password)
            .catchError { handleErrors($0) } // <- catch errors here
    }

Then how can LoginViewModel tell LoginViewController that login has failed?

@devxoul
Copy link
Collaborator Author

@devxoul devxoul commented Jun 7, 2016

I got an answer from the conversation with @kzaher on Slack. This key idea is: "Treat an API error as a 'failure of sequence' or just an 'error-representing' element."

How I have done is to return Observable<Result<User>> instead of Observable<User> from API function and treat API error as Result.Failure.

API.swift

enum Result<Value> {
    case Success(Value)
    case Failure(ErrorType)
}

func login(username username: String, password: String) -> Observable<Result<User>> {
    return ...
}

LoginViewController.swift

self.didComplete = Observable.of(self.usernameInputDidReturn,
                                 self.passwordInputDidReturn,
                                 self.loginButtonDidTap)
    .merge()
    .withLatestFrom(usernameAndPassword)
    .flatMapLatest { username, password in
        return api.login(username: username, password: password) // Observable<Result<User>>
    }
    .asDriver { error in
        return Driver.just(.Failure(error))
    }

LoginViewModel.swift

self.viewModel.didComplete
    .driveNext { result in
        switch result {
        case .Success(let user):
            self.processNextStep(user)

        case .Failure(let error):
            switch error {
            case LoginError.Username(let message):
                self.showError(message, on: self.usernameInput)

            case LoginError.Password(let message):
                self.showError(message, on: self.passwordInput)

            default:
                self.showError("Unknown error")
            }
        }
    }
    .addDisposableTo(self.disposeBag)

I attach the whole conversation for others 馃槃

@kzaher
I think, it鈥檚 about definition. This might sound weird at first but there is no such thing as universal error. You can probably just define error in a particular context. So if we are saying error in context of observable sequence, then yes, you obviously don鈥檛 want to terminate sequence.

What you want is an enum value that expresses that condition as sequence element. Result can emulate that ofc
but it鈥檚 not expandable. What you probably want is:

enum {
    case NormalCase
    case PresentErrorBecauseOf
    case PresentSomething3
}

And yes, if you have only kind of 2 cases, then you could abuse Result for this. So what we're doing is actually 鈥媉FSP_鈥 (Functional Sequential Programming) :)

@devxoul
Does it mean that it is not always needed to use sequence's Error, but we can use element as an error case? Such as Result model or enum model as you mentioned.

@kzaher
I think this is more of o philosophical question :) I wouldn鈥檛 call this an error case. You just need to present a special case

@devxoul
Oh I got it. It is more similar to: Should REST API return 4xx status code on error? Someone says 'HTTP request has succeeded', other says 'HTTP request has succeeded but execution has failed'
Ummmm I cannot explain what I'm thinking in English 馃槥 But I think you're correct.

@kzaher
It鈥檚 is similar in that it also asks question, error in what layer. Error in what context, then yes :)
In this case, it鈥檚 error in your 鈥渁pplicative layer鈥 and observable sequence would be HTTP layer :)
So the real problem is referring to both of these concepts as just error instead of error in observable sequence context (HTTP context) Error in my application context. Hope this clears things up :)

@devxoul
Cool. It made my brain open. Thanks! I can attach this conversation in #729 for others

@kzaher
thnx

@devxoul devxoul closed this Jun 7, 2016
This was referenced Mar 30, 2017
@amirpervaiz086
Copy link

@amirpervaiz086 amirpervaiz086 commented Nov 17, 2017

@devxoul why not we just catch error in "catchError" and use error observable in our ViewModel and subscribe that to view controller so when ever error occurs, it will fall into catch and emit error object to view controller. does it make sense?

@devxoul
Copy link
Collaborator Author

@devxoul devxoul commented Nov 17, 2017

@amirpervaiz086, Yeah that can be another solution. But I think I wanted to do that with a single observable at that time :)

@wongzigii
Copy link

@wongzigii wongzigii commented Dec 7, 2017

@devxoul Let's say I finally got an Observable<Result<User>> from network request, How can I unwrap this into Observable<User> or Driver<User> ?

@devxoul
Copy link
Collaborator Author

@devxoul devxoul commented Dec 7, 2017

@wongzigii you can use pattern matching.

@wongzigii
Copy link

@wongzigii wongzigii commented Dec 7, 2017

@devxoul I finally found dematerialize

@Colafroth
Copy link

@Colafroth Colafroth commented Feb 20, 2019

@devxoul if you use Result, how do you trigger retry without using the onError?

@devxoul
Copy link
Collaborator Author

@devxoul devxoul commented Feb 21, 2019

I haven't tried yet but you may use flatMap to map the result to an error.

@freak4pc
Copy link
Collaborator

@freak4pc freak4pc commented Apr 30, 2020

There was a comment here by @kean which was deleted. Went as follows:

I'm really struggling with error handling. RxSwift recommends to use Single for representing network requests and model errors as sequence errors.

func getRepo(_ repo: String) -> Single<[String: Any]>

But based on this thread, it turns out, this is incompatible with the idea of how to do error handling, which is to never allow sequences to fail. What's the point of using Single then? Wouldn't it make more sense to model Single such that it never fails in the first place (and maybe model all of the rest of the traits/subjects this way)? Has anyone explored this idea?

It was important for me to answer your question because I think you got the wrong idea. There is no idea of "you should never allow sequences to fail" that you should follow.

You need to handle and hone your errors, though. What does that mean?

  • If your stream feeds a UI Element, you wouldn't want it to ever emit an error, because UI Elements have no idea what to do with errors, and also UI elements should always have something on them. This is why things like Driver and Signal exist. You could easily achieve the same with a regular Observable obviously, which is the base for everything. You don't have to use traits or any other fancy types, they are just things that provide type-safe guarantees which make them easier for consumers to make assumptions about, but they're not a must.

  • If you have something that may have an error, and used by a different piece of you code, like a network request, I personally would have it throw that error. The consumer that uses your network request should decide what to do with the errors. Catch them? Materialize? etc. So that means, the inner units of your app may and should usually throw their errors but once you pass the data on to consumers, you should decide what to do in that error case.

  • When you need to handle errors as a user-facing event, the two most common ways are using Observable<Result<[String: Any], SomeError>>, or using materialize() on a regular Observable<[String: Any]> and splitting errors and elements. Me and my team personally prefer the latter as it provides more control (and basically an Event has the same shape of a Result, minus the typed error).

Hope this helps clear up some of the ideas (my personal thoughts, at least) about how to leverage error handling.

@kean
Copy link

@kean kean commented May 1, 2020

Thanks for the clear explanation, @freak4pc. I think this is going to be helpful for people. But the reason I asked it was different. Let me clarify what I meant, I wasn't clear in the original message because I didn't take enough time to compose it, my bad.

I wasn't fully satisfied with the need to provide an error handling mechanism when casting to Driver when I already caught the errors in the upstream sequences, or never produced errors in the first place (by using Single<Result<T>>, which yes, doesn't actually communicate to the system that there are no errors possible).

I was wondering whether there is any way to make it more ergonomic. I can see how this could be achieved with typed errors and Never type. I was wondering whether someone investigates whether it is possible with RxSwift traits.

If you look at the example in this issue, there is this closure which is supposably never going to be executed.

.asDriver { error in
    return Driver.just(.Failure(error)) // This is never going to be called, not errors are produced.
}

And, in reality, these error recovery methods for Driver can sometimes be confusing. Here is a (bad) example:

init(searchTerm: ControlProperty<String>, searchService: AccountSearchServiceProtocol) {
    self.accounts = searchTerm.flatMapLatest { searchTerm in
        searchService.searchAccounts(name: searchTerm)
    }.asDriver(onErrorJustReturn: [])
}

This doesn't achieve what you want as it simply replaces the entire flatMapLatest sequence with an empty value as soon as it errors out for the first time. Then it's just going to continue using this empty sequence forever.

This is the correct placement, but the resulting sequence isn't a Driver:

init(searchTerm: ControlProperty<String>, searchService: AccountSearchServiceProtocol) {
    self.accounts = searchTerm.flatMapLatest { searchTerm in
        searchService
           .searchAccounts(name: searchTerm)
           .asDriver(onErrorJustReturn: [])
        }
    }

I found this to work best, but I find it counterintuitive to cast input to Driver.

init(searchTerm: ControlProperty<String>, searchService: AccountSearchServiceProtocol) {
    self.accounts = searchTerm.asDriver().flatMapLatest { searchTerm in
        searchService
            .searchAccounts(name: searchTerm)
            .asDriver(onErrorJustReturn: [])
    }
}
@freak4pc
Copy link
Collaborator

@freak4pc freak4pc commented May 4, 2020

Since RxSwift doesn't use typed errors, we have no way to know "you handle the error upstream". As far as Rx is concerned, whatever you feed into the Driver may emit an error.

Overloads around Result, etc, are opinionated, and not specifically in the scope of RxSwift IMO.

You could easily create your own overloads around your specific use-cases:

For example, I personally use this in cases I'm absolutely positive I've handled the error:

extension ObservableConvertibleType {
    func asDriver() -> Driver<Element> {
         return asDriver(onErrorDriveWith: .never())
    }
}

You can also deal with Result, specifically, in a similar way:

protocol ResultConvertible {
    associatedtype Success
    associatedtype Failure: Swift.Error
}

extension Result: ResultConvertible {}

extension ObservableConvertibleType where Element: ResultConvertible {
    func asDriver() -> Driver<Element> {
        return asDriver(onErrorDriveWith: .never())
    }
}

In regards to where you place error handling, obviously scoping is important :) This is true in RxSwift like it is in Combine, and like it is even in Swift (where the position of the do/catch obviously matters).

Eventually traits are a means to let you communicate to outside consumers that you provide some guarantees. If you are absolutely sure you've dealt with the error, you can use one of the above syntactic sugar options to "silence" failures.

You might want to instead have a regular asDriver() that also fataLErrors in DEBUG, just to catch mistakes early on:

func asDriver() -> Driver<Element> {
    return asDriver { error in
        #if DEBUG
        fatalError("An error was thrown into an infallible Driver")
        #else
        return .never()
        #endif
    }
}
@kean
Copy link

@kean kean commented May 4, 2020

Thanks, I was thinking about adding just this asDriver() variant. But I'm also satisfied with casting inputs to drivers. It might be worth including asDriver in the framework and just rely on the runtime errors since there is not enough information at compile time; similar to how bind works for Observable. I typically expect frameworks to have most of the common use cases covered without the need for extensions.

@freak4pc
Copy link
Collaborator

@freak4pc freak4pc commented May 9, 2020

I'd say generally I disagree, because it's just an opinionated way of "silencing" the error. We have no type-safe way to know you've silenced the error up in the chain, so this is exactly where you need to explicitly say "I'm a developer, and I've already handled this error, so going into a .never() is not a problem" - Much like you wouldn't necessarily put a fatalError in that path, but expect a developer to make the decision.

Thanks for the notes again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can鈥檛 perform that action at this time.