GIDSignIn.sharedInstance.signIn does not return a callback sometimes #378

Activity

1. doctorsaway

   added

   bugSomething isn't workingSomething isn't working

   triageIssues that need to be triagedIssues that need to be triaged

   on Mar 13, 2024
2. mdmathias commented on Mar 13, 2024

   

   mdmathias

   on Mar 13, 2024

   Collaborator

   More actions

   Hi @doctorsaway this isn't reproducible on our end. Please take a look at our example apps to see how you might use this functionality. If there is something that is different, or something that you believe we can address, please reply with more exact reproduction steps (for example, providing context on how your call to -[GIDSignIn signIn...] is made would be helpful).
3. mdmathias

   closed this as completedon Mar 13, 2024
4. mdmathias

   removed

   bugSomething isn't workingSomething isn't working

   triageIssues that need to be triagedIssues that need to be triaged

   on Mar 13, 2024
5. marchy commented on Apr 12, 2024

   

   marchy

   on Apr 12, 2024

   Last edited by marchy

   More actions

   @mdmathias
   ⚠️ P1: Blocking users from logging in!
   We are experiencing the same thing, except that it is not intermittent has overnight stopped working altogether.
   We've had no change to the same code that was working yesterday, but suddenly today there is no callback firing anymore.

   Neither the success nor Cancel scenarios after the Google Sign-in web view is shown fire the callback anymore, however if you click 'Cancel' on the initial alert that is shown prior to the web-view being shown, the callback IS fired (ie: it's definitely a Google SDK issue!)

   

   NOTE #1: We tried both async and callback APIs and neither are continuing past the initial invocation.
   NOTE #2: We tried cleaning/rebuilding the project did not change anything
   NOTE #3: We ensured our client ID matches and resolves correctly at runtime via GIDSignIn.sharedInstance.configuration!.clientID

   Invocation code:

   func initiateGoogleAuth_async() async throws -> User {
   	do {
   		let result:GIDSignInResult = try await GIDSignIn.sharedInstance.signIn( withPresenting: self )
   				
   		// authenticate
   		let user:User = try await self.accountService.authenticate( googleUser: result.user ) //  <--- never gets here
   
   	} catch let error {
   		print( "ERROR: \(error)" ) // <--- never gets here
   		...
   	}
   }		
   
   
   func initiateGoogleAuth_callback( onSuccess:@escaping(User) -> (), onError:@escaping(any Error) -> () ){
   	GIDSignIn.sharedInstance.signIn( withPresenting: self ){ (result:GIDSignInResult?, error:(any Error)?) in
   		guard let result else {
   			print( "ERROR: \(error)" ) // <--- never gets here
   			...
   			return
   		}
   
   		// authenticate
   		let user:User = try await self.accountService.authenticate( googleUser: result.user ) //  <--- never gets here
   	}
   }

   Could this have anything to do with some security/configuration for the project not being correctly set?

   Our OAuth consent screen config in the Developer Console is set to use the publishing status of Production, user types as External and not token grant rate limit has been set.
6. marchy commented on Apr 12, 2024

   

   marchy

   on Apr 12, 2024

   Last edited by marchy

   More actions

   UPDATE: The callbacks started working again after over an hour of observed downtime.
   UPDATE: The callbacks stopped working again as of 6:45pm UTC.

   This is quite concerning as it seems there might be Google back-end issues that the library is not handling. If there are errors they should be surfaced up through the error callback not swallowed and app left in an undetermined state (ie: neither succeeded or failed)

   ASK: Could the team investigate if any back-end issues were experienced on Apr 11, 2014 prior to 3:30pm UTC (time of my prior post). By 5pm UTC the callbacks started resuming again.

   Please keep us posted on this as it is a total blocker to user sign-up / acquisition if this happens intermittently, and we'd like to understand how pervasive this is (ie: % of SLA to expect)
7. marchy commented on Apr 12, 2024

   

   marchy

   on Apr 12, 2024

   Last edited by marchy

   More actions

   Got down to the bottom of it!

   PROBLEM:

   Chased this further down and it seems that within GIDSignIn::addCompletionCallback(..) (line 859) the self->_currentOptions.completion line fails the nil check (in fact _currentOptions itself if nil), so it somehow loses reference to the callback passed into the initial SDK by the time the full auth sequence finishes.

   

   Earlier in the chain it seems _currentOptions is nilled out, as the flow goes into a "non-interactive" branch of the logic
   

   The line on 547 has nothing to invoke since options.completion is nil at that point.

   CAUSE:

   The non-interactive call is coming from a call to try await GIDSignIn.sharedInstance.restorePreviousSignIn() which is happening because when the Google Sign-In web view is showing, it re-invokes the app's SceneDelegate::sceneDidBecomeActive(..) method which is where we refresh data including user data (which itself attempts to restore the prior Google sign-in).

   This in effect was clearing out the callback that was initially passed in at the start of the sign-in flow – and the intermittency of it came from the lag of the user picking their account VS the app sync process finishing earlier in the background.

   QUESTION:

   Is there any way to know when the scene is being displayed due to the Google sign-in web view so that we can opt out the restorePreviousSignIn() check in this scenario?

   SOLUTION:

   Architecturally, the library should prevent separate simultaneous operations from sharing state about their parameters and thus trampling on each other.

   @mdmathias Could you re-open this ticket? It is a library mis-implementation to share parameters among multiple async operations. It's one thing for them to share user/auth state, but the parameters of one operation (such as callback continuation) should never be shared/impacted by a subsequent operation to either the same API or another one as in this case.

   As it stands, we have to implement a ton of extra complexity (such as concurrent operation queues) to make sure we don't call GIDSignIn.sharedInstance.restorePreviousSignIn() while GIDSignIn.sharedInstance.signIn() is in progress and vice-versa.

   ❤️React with ❤️1Reacted by Theo Sementa
8. mdmathias commented on Apr 15, 2024

   

   mdmathias

   on Apr 15, 2024

   Collaborator

   More actions

   Hi @marchy. Thanks for the posts and your analysis. Sorry that you're having trouble.

   First, let me confirm that I understand your issue. From what I gather, it seems like you're experiencing a kind of race condition due to simultaneous calls to restorePreviousSignIn() and signIn(), where the restore call is setting _currentOptions to nil prior to the sign in call's use of _currentOptions.completion to add the completion callback. Therefore, the sign in flow in your app will occasionally not issue a callback due to this race condition. Seems like this line would not pass in this scenario, which would lead to no callback. Is that right?

   If my understanding is correct, then it is unfortunately an error to simultaneously call signIn() and restorePreviousSignIn(). From the library's point of view, it doesn't make sense to restore and sign in at the same time.

   Even if we made changes to GSI to protect against this error [1], apps would still have to implement some logic to choose which response to handle. For example, which response is the truth? The sign in or the restore? The completions will not have the same information.

   The practical solution on your end is to make sure that you are signing in and restoring at different points in your app's lifecycle. Ultimately, it's up to apps to manage this. You can take a look at our Swift sample and ObjC sample for inspiration.

   [1] I haven't fully thought through the feasibility of your request, but I believe it may require significant refactoring of the library. We'd have to (potentially re-) consider GIDSignIn's singleton status, manage multiple async callback queues on behalf of clients wishing to issue simultaneous and duplicative requests, etc. While I can see ways that this would be useful to callers like yourself (given the scenario you outline above), I have to admit that this sort of change is not feasible at the moment. Of course, since the repo is open source, if you believe you have a simple fix to resolve this, then please send us a PR and we would love to review it!
9. Ariandr commented on Dec 17, 2024

   

   Ariandr

   on Dec 17, 2024

   Last edited by Ariandr

   More actions

   Hi @mdmathias
   I have the same issue and I don't simultaneously call signIn() and restorePreviousSignIn(). I call signIn() only after restorePreviousSignIn() doesn't succeed, not in parallel, but the callback from signIn() is never called. I use v. 8.0
10. Ariandr commented on Dec 17, 2024

    

    Ariandr

    on Dec 17, 2024

    Last edited by Ariandr

    More actions

    If anyone needs a workaround for this issue for v.8.0 SDK, you can use this. Works like a charm for me.
    It essentially monitors Google Auth controller disappearing. You don't get the error, but you can get the fact that it was presented and dismissed and check the currentUser after the dismissal is complete.

    Also, you can make the completion be called only once by putting it into a separate optional property and making it nil after the first call. But for my use case it's not needed.

    I tried to use KVO on presentingController?.presentedViewController to monitor it's change, but it doesn't work for me. If you can make it work, please let me know, it would be more elegant that this.

        private weak var presentingController: UIViewController?
        private var dismissalTimer: Timer?
        private var googleAuthWasPresented = false
        
        private func signInAndRequestCalendarPermission(presenting: UIViewController, completion: @escaping (GIDGoogleUser?, Error?) -> Void) {
            let scopes = [kGTLRAuthScopeCalendar]
            
            // It's a workaround, because signIn() doesn't return a callback in most cases
            self.presentingController = presenting
            startMonitoringDismissal(completion: completion)
            
            GIDSignIn.sharedInstance.signIn(withPresenting: presenting,
                                            hint: nil,
                                            additionalScopes: scopes) { result, error in
                self.cleanUpTimer()
                completion(result?.user, error)
            }
        }
        
        private func startMonitoringDismissal(completion: @escaping (GIDGoogleUser?, Error?) -> Void) {
            dismissalTimer?.invalidate()
            dismissalTimer = Timer.scheduledTimer(withTimeInterval: 0.1,
                                                  repeats: true) { [weak self] _ in
                guard let self = self else { return }
                
                if self.presentingController?.presentedViewController != nil {
                    self.googleAuthWasPresented = true
                } else if self.googleAuthWasPresented {
                    print("Presented view controller has been dismissed.")
                    completion(GIDSignIn.sharedInstance.currentUser, nil)
                    
                    self.cleanUpTimer()
                }
            }
        }
    
        private func cleanUpTimer() {
            googleAuthWasPresented = false
            presentingController = nil
            dismissalTimer?.invalidate()
            dismissalTimer = nil
        }
    

    ❤️React with ❤️1Reacted by Marcel Bradea