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

self.step.accept() not working while [pushViewController animated:true] #60

Closed
yudinm opened this issue Jun 27, 2018 · 8 comments
Closed
Assignees
Labels

Comments

@yudinm
Copy link

yudinm commented Jun 27, 2018

Hi. In our project we are trying to handle network errors in viewModels like that:

             apiClient.getTask()
                .asObservable()
                .catchError{ (error) -> Observable<Task> in
                    guard let error = error as? MoyaError else { return }
                    guard case .statusCode(let response) = error else { return }
                    
                    if response.statusCode == 401 {
                        print("\(response.statusCode) Unauthorized!")
                        invalidateUser()
                        self.step.accept(MySteps.auth)
                    }
                }

In Flow we are using that code for pushing controller:

    private func navigateToTask(withId identifier:String) -> NextFlowItems {
        …
        self.rootViewController.pushViewController(taskViewController, animated: true)
        return NextFlowItems.one(flowItem: NextFlowItem(nextPresentable: taskViewController, nextStepper: taskViewModel))
    }

So, error throws in the middle of animation of pushing view controller and step.accept() did not work. On the other hand, when we are pushing VC without animation (self.rootViewController.pushViewController(taskViewController, animated: false)) – its ok.

What are we doing wrong?

@twittemb
Copy link
Collaborator

twittemb commented Jun 27, 2018

Hi @yudinm

I Have a few questions:

  • In the TaskViewModel, the network call is done in the init function ?
  • Is there an error message or it is just not working ?

My take on this -> the MySteps.auth step is triggered too soon (because the network call is done in the init function of the TaskViewModel) and is catched by your Flow whereas the animation is not finished yet. Then your Flow triggers a new navigateXXX function (because of the MySteps.auth step) and then UIKit cannot display the auth screen because of the animation is not finished on the previous screen.

You should try to put your network call in a dedicated function in your viewModel and call this function in the viewDidLoad of the taskViewController.

Keep me in touch.

@twittemb twittemb self-assigned this Jun 27, 2018
@yudinm
Copy link
Author

yudinm commented Jun 27, 2018

Thank you for quick answer.

In the TaskViewModel, the network call is done in the init function ?
No. Network call done in viewDidLoad.

Is there an error message or it is just not working ?
No error. Just did not go to coordinator.rx.willNavigate/didNavigate and so on. Also i try to subscribe on .step in network error handler and see that:

// animated: true
401 Unauthorized!
step: NoneStep()
step: auth
// animated: false
step: NoneStep()
did navigate to flow=TaskFlow and step=auth
did navigate to flow=DashboardFlow and step=auth
did navigate to flow=AppFlow and step=auth
will navigate to flow=LoginFlow and step=login
did navigate to flow=LoginFlow and step=login
step: auth

Checked calls in VC. Found, that network call triggered by let viewWillAppear = rx.sentMessage(#selector(UIViewController.viewWillAppear(_:))).mapToVoid().asDriverOnErrorJustComplete(). Fixed by replacing viewWillAppear to viewDidAppear.

However, problem confuse me. Why Stepper or Coordinator work depends from animation and VC state? In my case we have access token that sends in http headers.

  1. User get tasks list (first network call)
  2. Access token invalidates on server
  3. User taps on task item. Its pushes details VC (second network call)
  4. Getting network error with status code 401

On this error we want navigate user back to login flow. That kind of error can perhaps in very different cases: on background requests, when user quickly taps on different items and switch screens. How we can assured redirect user to login (or something other) flow without any knowledge about current view state, transitions?

@twittemb
Copy link
Collaborator

twittemb commented Jul 2, 2018

Hi @yudinm

In order to help you efficiently, It would be great if I could see a bigger part of your code base ... but I assume it is not open source :-( so I will try to give you some insights:

Why VC lifecycle matters ?
The only reason RxFlow "listens" to your ViewControllers lifecycle is to be able to restrain the effects of a Stepper to the VC (and the Flow) that is currently displayed. To do so, RxFlow needs to know if the VC associated to the Stepper that has triggered a new Step is displayed or not, and if yes -> the Step can be given to the Flow, if not -> the Step is ignored.

RxFlow does this to avoid performing a navigation coming from a VC that is not displayed (for instance from a VC that is lower than the current VC in your navigation stack), because that could lead to UIKit errors (by making a VC present a another VC whereas it is not actually the currently displayed VC).

RxFlow also performs such checks at a Flow level.

Handling user connection state

I personally had to deal with "user authentication state" in previous applications. In that case, I like to have a "UserState" value exposed through a Rx Observable by a low level layer (perhaps a UserService or something). This UserState can be an enum representing the current user, for instance:

enum UserState {
    case unauthenticated
    case authenticated (User)
}

If for some reason, a request returns a code 401, you could mutate the UserState to ".unauthenticated". As it is an Observable, you could react to this mutation.

For instance, the Stepper associated with the root Flow could subscribe to this UserState Observable and and trigger a MyStep.auth step if the state equals .unauthenticated. In that case the root Flow could display a login popup to authenticate the user. Doing so, your app is able to prompt the user to login each time the token is expired.

Could this match your requirements ?

@twittemb
Copy link
Collaborator

twittemb commented Jul 4, 2018

@yudinm how is it going with yours flows ?

@yudinm
Copy link
Author

yudinm commented Jul 6, 2018

Hi. Sorry, appeared tasks not related to the Flows.

With VC lifecycle – it's clear.

With connection state we'll use your way, probably. Thank you.

For now we just added extension to ObservableType:

import RxSwift

protocol ErrorHandler {
    func handle(_ error: Error)
}

extension ObservableType {
    func handleError(_ errorHandler: ErrorHandler) -> Observable<E> {
        return Observable.create({ observer in
            let subscription = self.subscribe { e in
                switch e {
                case .next(let value):
                    observer.on(.next(value))
                case .error(let error):
                    errorHandler.handle(error)
                    observer.on(.error(error))
                case .completed:
                    observer.on(.completed)
                }
            }
            
            return subscription
        })
    }
}

And in ViewModels implement protocol:

extension TaskListViewModel : ErrorHandler {
    func handle(_ error: Error) {
        guard let error = error as? MoyaError else { return }
        guard case .statusCode(let response) = error else { return }
        
        if response.statusCode == 401 {
            print("\(response.statusCode) Unauthorized!")
            self.services.preferencesService.invalidateUser()
            self.step.accept(Steps.auth)
        } else {
            print("Error statusCode: \(response.statusCode)")
        }
    }
}

Anyway error handling is a future task in current app.

I would like to add, that ViewControllers as a Stepper mixed with ViewModels as a Stepper produces some confusion. In our case we have agreed to use only ViewModels as a Steppers.

Thank you. Good stuff.

@twittemb
Copy link
Collaborator

twittemb commented Jul 6, 2018

@yudinm

Thanks for your feedback. You're totally right about ViewModels being the best candidate for Stepper.

If i may make a suggestion, it feels like your extension to ObservableType is doing something very similar to the built-in do(onError) function. Is there a reason you don't use it ?

@twittemb
Copy link
Collaborator

@yudinm

Can I close this issue ?

Thanks.

@yudinm
Copy link
Author

yudinm commented Jul 16, 2018

Yes. Thanks! You may close.

We like to make extension, because we do not want include "big" block in chain. We will call .handleError(self) in chain. And implement enum of error handlers somewhere outside of ViewModel.

        let tasks = input.trigger.flatMapLatest { _ -> Driver<[TaskListViewModelItem]> in
            return apiClient.getTasks()
                .asObservable()
                .handleError(self)
                .map{ return $0.map(TaskListViewModelItem.init) }
                .asObservable()
                .trackActivity(activityIndicator)
                .asDriver(onErrorJustReturn: [])
        }

@yudinm yudinm closed this as completed Jul 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants