supertokens / supertokens-ios

iOS SDK for SuperTokens
https://supertokens.io
Other
12 stars 5 forks source link

App crashing on launch #61

Open cameroncooke opened 8 months ago

cameroncooke commented 8 months ago

SwiftUI entry-point App occasionally crashes due to force unwrapping a nil value.

Thread 2: Fatal error: Unexpectedly found nil while unwrapping an Optional value.

Screenshot 2024-02-22 at 15 02 40

I feel the implementation is very fragile and very sensitive to life-cycle behaviour, in my case my app is a pure SwiftUI app with an @main entry point. Sometimes the SuperTokens.doesSessionExist() is called the Delegate adaptor is called.

My entry point looks like this:

import SwiftUI

final class AppDelegate: NSObject, UIApplicationDelegate {
    func application(
        _ application: UIApplication,
        didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]? = nil
    ) -> Bool {

        do {
            try SuperTokens.initialize(
                apiDomain: "http://127.0.0.1:3001",
                apiBasePath: "auth"
            )
        } catch let SuperTokensError.initError(message) {
            print(message)
        } catch {
            print(error)
        }

        URLProtocol.registerClass(SuperTokensURLProtocol.self)

        return true
    }
}

@main
struct MyApp: App {
    @State var isSignedIn = SuperTokens.doesSessionExist()
    @UIApplicationDelegateAdaptor(AppDelegate.self) var appDelegate

    var body: some Scene {
        WindowGroup {
            if isSignedIn {
                MainView()
            } else {
                SignInView()
            }
        }
    }
}
rishabhpoddar commented 8 months ago

The doesSessionExist function relies on the fact that you have called the init function of our SDK. In the snippet above, is it guaranteed that the init function is called before the doesSessionExist is called?

cameroncooke commented 8 months ago

Well, that's kind of my point in an un-documented way the library requires code to be called at run-time in the "correct" sequence, in my case it's Apple's platform code that is non-deterministic leading to this crash. I would argue it's a design flaw of the library to allow this to happen in the first place. I think using static methods (essentially a singleton) is the issue, had this library been instance-based then you would be able to force the instantiation of the class with the appropriate configuration before any methods are called on it that require them.

I understand the aim is convenience but that seems to come at a cost. If the local state (credentials) are the source of truth then there shouldn't be an issue with having multiple instances in a single codebase, the cost would be low, just disk IO. All you are doing is checking tokens are valid or refreshing them right?

Also, I would argue the tokens should be in the keychain not a list file, this would work very similarly to Amplify Cognto iOS SDK which stores its tokens in the keychain which is more secure.

rishabhpoddar commented 8 months ago

Fair enough. It may take us a bit of time to make these changes since we have other priorities at the moment, so you need not use our iOS SDK, and instead manage the tokens yourself: https://supertokens.com/docs/session/quick-setup/handling-session-tokens#if-not-using-our-frontend-sdk (see the method of Header based and not cookie based).

rbranham commented 5 months ago

I had a similar app architecture and was able to come up with a workaround to make sure Supertokens was initialized first. FYI, I am by no means even proficient in Swift, just dabbling on a side project, but I thought I would share this solution for others who come across it:

import SwiftUI
import SuperTokensIOS

@main
struct RouteRecordApp: App {

    // Supertokens initialization, Needs to be done at start of app.
    init() {
        do {
            try SuperTokens.initialize(apiDomain: "http://localhost:4000")
        } catch {
            print("Failed to initialize SuperTokens: " + error.localizedDescription)
        }
        URLProtocol.registerClass(SuperTokensURLProtocol.self)

    }

    var body: some Scene {
        WindowGroup {
            HomeView()
        }
    }
}

struct HomeView: View {

    @StateObject private var viewModel = HomeViewModel()

    var body: some View {
        if viewModel.loggedin {
            CompanySelectScreen(
                signOut: {
                    SuperTokens.signOut { res in
                        viewModel.refresh()
                    }
                }
            )
        } else {
            LoginScreen(
                triggerRecheck: { viewModel.refresh() }
            )
        }

    }
}
import Foundation
import SuperTokensIOS

class HomeViewModel: ObservableObject {

    @Published var loggedin: Bool

    init() {
        loggedin = false
        refresh()
    }

    func refresh() {
        DispatchQueue.main.async {
            self.loggedin = SuperTokens.doesSessionExist()
        }

    }
}

Main idea here is to move the check to a view model so it stopped getting triggered early. I'm honestly not even sure if this is a guarantee to be later (like I said swift is something I'm just dabbling in) But it did solve the above issue for me.

cameroncooke commented 5 months ago

The dispatch queue also throws it onto the next run loop so effectively delaying the execution of the doesSessionExist call. Given you’ve had to define a default value of false there could be a momentary jump between the logged out state and logged in state.

Personally I’m not keen of using GCD to work around issues like this but I’m glad it works for you.

In my case I just rebuilt the entirely library so this the originally problem can’t happen. I might release it at some point but it won’t be feature complete. Just does what I need it to do.

Cameron.

On Wed, 22 May 2024 at 01:22, rbranham @.***> wrote:

I had a similar app architecture and was able to come up with a workaround to make sure Supertokens was initialized first. FYI, I am by no means even proficient in Swift, just dabbling on a side project, but I thought I would share this solution for others who come across it:

import SwiftUI import SuperTokensIOS

@main struct RouteRecordApp: App {

// Supertokens initialization, Needs to be done at start of app.
init() {
    do {
        try SuperTokens.initialize(apiDomain: "http://localhost:4000")
    } catch {
        print("Failed to initialize SuperTokens: " + error.localizedDescription)
    }
    URLProtocol.registerClass(SuperTokensURLProtocol.self)

}

var body: some Scene {
    WindowGroup {
        HomeView()
    }
}

}

struct HomeView: View {

@StateObject private var viewModel = HomeViewModel()

var body: some View {
    if viewModel.loggedin {
        CompanySelectScreen(
            signOut: {
                SuperTokens.signOut { res in
                    viewModel.refresh()
                }
            }
        )
    } else {
        LoginScreen(
            triggerRecheck: { viewModel.refresh() }
        )
    }

}

}

import Foundation import SuperTokensIOS

class HomeViewModel: ObservableObject {

@Published var loggedin: Bool

init() {
    loggedin = false
    refresh()
}

func refresh() {
    DispatchQueue.main.async {
        self.loggedin = SuperTokens.doesSessionExist()
    }

}

}

Main idea here is to move the check to a view model so it stopped getting triggered early. I'm honestly not even sure if this is a guarantee to be later (like I said swift is something I'm just dabbling in) But it did solve the above issue for me.

— Reply to this email directly, view it on GitHub https://github.com/supertokens/supertokens-ios/issues/61#issuecomment-2123631878, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEZ6SOBUSUY6UKRNBP6QWTZDPQL3AVCNFSM6AAAAABDVFCG5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRTGYZTCOBXHA . You are receiving this because you authored the thread.Message ID: @.***>