2FA SwiftUI crashes in TicTacToe example

[rhys-rant]

Describe the bug
In the TicTacToe example application, when you choose the SwiftUI application flow and opt into the Two Factor Auth. section, when you tap "Next" after typing in 1234, the app with crash with the error:

Fatal error: "LoginAction.twoFactorDismissed" was received by an optional reducer when its state was "nil". This can happen for a few reasons:

with a long list of reasons as to why it's potentially crashed.

This crash appears to be caused by LoginSwiftView.swift line num. 71. where the app sends .twoFactorDismissed to the now nil'd out LoginState, as the NavigationLink receives false for the isActive binding.

To Reproduce

• Open the TicTacToe sample application
• Select SwiftUI version
• Enter 2fa@test.com in the Email field
• Enter password in the Password field
• Tap Log in
• Enter 1234 and tap Submit
• App navigates to the New Game screen, then immediately crashes

Expected behavior
I expected the app to progress to the New Game screen and not crash 😅

Screenshots

Environment

• Xcode 12.4
• Swift 5.3
• Composable Architecture version 0.15.0

Additional context
If you login with the 2FA flow through the UIKit version, it doesn't crash, and retains the login if you navigate backwards and switch to the SwiftUI version.

[mbrandonw]

Hi @rhys-rantmedia, we dug deep this morning and uncovered that it's due to a SwiftUI bug 😭.

It's actually related to this issue: #363. It seems that enabling/disabling NavigationLink is a little buggy, and in this case it causes an unexpected action to be sent into the store, thus causing the assertion failure you see. Disabling this one single line fixes it:

swift-composable-architecture/Examples/TicTacToe/Sources/Views-SwiftUI/LoginSwiftView.swift

Line 80 in 86d6e35

.disabled(viewStore.isLoginButtonDisabled)

I'll be submitting a PR to fix this soon.

It's also worth noting that this isn't a crash but rather an assertionFailure, so it only happens in DEBUG, not RELEASE. We do this because it can help catch programmer errors and silent failures. However, we do have a PR (#296) where we plan to turn the assertionFailure into a breakpoint so that you can continue execution after.

Thanks for reporting this! We're glad you caught it so that we can fix it. We're pretty sure this worked in Xcode 11.something but who knows when things changed 😬

[mbrandonw]

Actually the above is incorrect... I accidentally had some code commented out that led me to believe things were working when they were not 🙁

But, we did find the real culprit. It appears to be the Form used in LoginView. If you comment out this form everything works as expected:

swift-composable-architecture/Examples/TicTacToe/Sources/Views-SwiftUI/LoginSwiftView.swift

Line 37 in 86d6e35

Form {

We've seen other weird issues with Forms, such as .onDisappear getting called early, so perhaps it's all related. I filed this feedback awhile ago, and if I can ever reproduce this problem in a vanilla SwiftUI app I'll file another one: https://gist.github.com/mbrandonw/2b1eefef3d939074fa040a7a0c7ab377

[rhys-rant]

Ahhh, that's such odd behaviour from SwiftUI!

I did see some of the other ones related to sending .dismiss actions from a .onDisappear closure, and wondered if it was similar...

Great to hear about #296! Is there any sort of timescale for when that PR might be merged?

Thanks for responding so quickly!

[mbrandonw]

It's going to take us a little more time to prepare the PR because we need to get rid of Form which means we have to style the view ourselves. We may not be able to do that for a few days or a week, but if you wanted to take a stab at removing Form and applying some basic styling to look reasonable then we would happily accept a PR!

[ericlewis]

@mbrandonw I also have ran into a similar issue in a similar fashion, but wasn't related to a form. It seems when unwinding some views that they call viewDidAppear as they are torn down.

[mbrandonw]

@ericlewis do you have any code you could show that reproduces the problem?

[ericlewis]

@mbrandonw I will TRY to create a repro project with minimal change, but it's in our production app currently. I do have a solution for it, but less than optimal.

[ericlewis]

@mbrandonw this is the most minimal example I can make that reproduces the issue.

ReproLogoutIssue.zip

edit: uploaded an even simpler version that still repros the issue, I was reflecting our setup with this one.
edit 2: it should actually fit right in a comment:

import SwiftUI
import ComposableArchitecture

struct AppState: Equatable {
    public var loggedOut: LoggedOutState? = LoggedOutState()
    public var loggedIn: LoggedInState?
}

enum AppAction: Equatable {
    case loggedOut(LoggedOutAction)
    case loggedIn(LoggedInAction)
    case logout
}

struct AppEnvironment {}

let appReducer = Reducer<AppState, AppAction, AppEnvironment>.combine(
    loggedOutReducer.optional().pullback(
        state: \.loggedOut,
        action: /AppAction.loggedOut,
        environment: { _ in
            LoggedOutEnvironment()
        }
    ),
    loggedInReducer.optional().pullback(
        state: \.loggedIn,
        action: /AppAction.loggedIn,
        environment: { _ in
            LoggedInEnvironment()
        }
    ),
    Reducer<AppState, AppAction, AppEnvironment> {
        state, action, _ in
        switch action {
        case .loggedOut(.login):
            state.loggedOut = nil
            state.loggedIn = .init()
            return .none
        case .loggedIn(.accountTab(.logout)):
            return Effect(value: .logout)
        case .logout:
            state.loggedOut = .init()
            state.loggedIn = nil
            return .none
        default:
            return .none
        }
    }
)

struct LoggedInState: Equatable {
    var homeTab = HomeTabState()
    var accountTab = AccountTabState()
}

enum LoggedInAction: Equatable {
    case homeTab(HomeTabAction)
    case accountTab(AccountTabAction)
}

struct LoggedInEnvironment {}

let loggedInReducer = Reducer<LoggedInState, LoggedInAction, LoggedInEnvironment>.combine(
    homeTabReducer.pullback(
        state: \.homeTab,
        action: /LoggedInAction.homeTab,
        environment: { _ in
            HomeTabEnvironment()
        }
    )
)

struct LoggedOutState: Equatable {}

enum LoggedOutAction: Equatable {
    case login
    case loggedIn
}

struct LoggedOutEnvironment {}

let loggedOutReducer = Reducer<LoggedOutState, LoggedOutAction, LoggedOutEnvironment> {
    _, action, _ in
    switch action {
    case .login:
        return .none
    case .loggedIn:
        return .none
    }
}

struct HomeTabState: Equatable {
    var number: Int?
}

enum HomeTabAction: Equatable {
    case fetch
    case fetchResponse
}

struct HomeTabEnvironment {}

let homeTabReducer = Reducer<HomeTabState, HomeTabAction, HomeTabEnvironment> {
    state, action, _ in
    switch action {
    case .fetch:
        return Effect(value: .fetchResponse)
    case .fetchResponse:
        state.number = Int.random(in: 0...1337)
        return .none
    }
}

struct AccountTabState: Equatable {}

enum AccountTabAction: Equatable {
    case logout
}

struct AccountTabEnvironment {}

let accountTabReducer = Reducer<AccountTabState, AccountTabAction, AccountTabEnvironment> {
    _, action, _ in
    switch action {
    case .logout:
        return .none
    }
}

struct AccountTabView: View {
    let store: Store<AccountTabState, AccountTabAction>
    
    var body: some View {
        WithViewStore(store) { viewStore in
            Button("Logout") {
                viewStore.send(.logout)
            }
        }
    }
}

struct HomeTabView: View {
    let store: Store<HomeTabState, HomeTabAction>
    
    var body: some View {
        WithViewStore(store) { viewStore in
            Text("Home \(viewStore.number ?? -1337)")
                .onAppear {
                    viewStore.send(.fetch)
                }
        }
    }
}

struct ContentView: View {
    let store: Store<AppState, AppAction>
    
    var body: some View {
        IfLetStore(self.store.scope(state: { $0.loggedOut }, action: AppAction.loggedOut),
                   then: { store in
                    NavigationView {
                        WithViewStore(store) { viewStore in
                            Button("Login") {
                                viewStore.send(.login)
                            }
                        }
                    }
                   },
                   else:
                    IfLetStore(
                        self.store.scope(state: { $0.loggedIn }, action: AppAction.loggedIn)
                    ) { loggedInStore in
                        TabView {
                            IfLetStore(
                                loggedInStore.scope(state: { $0.homeTab }, action: LoggedInAction.homeTab)
                            ) { store in
                                HomeTabView(store: store)
                                    .tabItem {
                                        Label("Home", systemImage: "house.circle")
                                    }
                            }
                            IfLetStore(
                                loggedInStore.scope(state: { $0.accountTab }, action: LoggedInAction.accountTab)
                            ) { store in
                                AccountTabView(store: store)
                                    .tabItem {
                                        Label("Account", systemImage: "person.circle")
                                    }
                            }
                        }
                    }
        )
    }
}

error:

2021-02-24 14:45:41.858363-0500 ReproLogoutIssue[64509:2967437] /Users/ericlewis/Developer/ReproLogoutIssue/ReproLogoutIssue/ContentView.swift:32: Fatal error: "LoggedInAction.homeTab(.fetch)" was received by an optional reducer when its state was "nil". This can happen for a few reasons:

* The optional reducer was combined with or run from another reducer that set "LoggedInState" to "nil" before the optional reducer ran. Combine or run optional reducers before reducers that can set their state to "nil". This ensures that optional reducers can handle their actions while their state is still non-"nil".

* An active effect emitted this action while state was "nil". Make sure that effects for this optional reducer are canceled when optional state is set to "nil".

* This action was sent to the store while state was "nil". Make sure that actions for this reducer can only be sent to a view store when state is non-"nil". In SwiftUI applications, use "IfLetStore".

[ericlewis]

I have a work around for the above code too, it is quite odd.

[rhys-rant]

It's going to take us a little more time to prepare the PR because we need to get rid of Form which means we have to style the view ourselves. We may not be able to do that for a few days or a week, but if you wanted to take a stab at removing Form and applying some basic styling to look reasonable then we would happily accept a PR!

Oops, sorry, I meant the #296 PR, for the breakpoint version of optional()

[mbrandonw]

Ahhh, yes, we plan on discussing that very soon.

[ferologics]

@ericlewis care sharing the workaround? I'm being blocked by this very issue.

EDIT: looks like the 0.16.0 release contains a "fix" where the assertionFailure is replaced by a breakpoint in DEBUG builds.