Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Expose exception on the signIn method in Google sign in. #1180

Merged

Conversation

cyanglaz
Copy link
Contributor

@cyanglaz cyanglaz commented Feb 7, 2019

Fix flutter/flutter#12734
When users are setting up the google sign in plugin, if for some reason, the <key>CFBundleURLTypes</key> is not properly set, we do not provide a clear enough error message to make their debugging life happier.

We would still like to crash the app since the crash would be a good reminder for the users to set up any necessary properties to use this plugin, and the users should not ship the product without fixing the issue that causes the exception.

After this fix, the error message will be shown as

flutter: PlatformException(google_sign_in, Your app is missing support for the following URL schemes: com.googleusercontent.apps.479882132969-9i9aqik3jfjd7qhci1nqf0bm2g71rm1u, NSInvalidArgumentException)
*** First throw call stack:
(
        0   CoreFoundation                      0x000000011332a1bb __exceptionPreprocess + 331
        1   libobjc.A.dylib                     0x00000001128c8735 objc_exception_throw + 48
        2   CoreFoundation                      0x000000011332a015 +[NSException raise:format:] + 197
        3   Runner                              0x000000010fa9652d -[GIDSignIn signInWithOptions:] + 242
        4   Runner                              0x000000010fa92b22 -[GIDSignIn signIn] + 64
        5   Runner                              0x000000010fa8aeeb -[FLTGoogleSignInPlugin handleMethodCall:result:] + 2251
        6   Flutter                             0x00000001105bffba __45-[FlutterMethodChannel setMethodCallHandler:]_block_invoke + 115
        7   Flutter                             0x00000001105dcc9c _ZNK5shel<…>
Lost connection to device.

@cyanglaz cyanglaz requested a review from mehmetf February 7, 2019 23:29
[[GIDSignIn sharedInstance] signIn];
} @catch (NSException *e) {
result([FlutterError errorWithCode:@"google_sign_in" message:e.reason details:e.name]);
[e raise];
Copy link
Contributor Author

@cyanglaz cyanglaz Feb 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still Debating myself if we should fail more graciously that was requested by the customer in flutter/flutter#12734 or we should still crashing it to just have better error message to force user to fix it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing configuration is fatal. An app like this should never be released and there's no gracious failure for it. Raising FlutterError is the right way to go for this.

@cyanglaz cyanglaz changed the title Google sign in expose exception on the signIn method. Expose exception on the signIn method in Google sign in. Feb 8, 2019
@cyanglaz cyanglaz merged commit 79eeec5 into flutter:master Feb 8, 2019
@cyanglaz cyanglaz deleted the google_signin_better_error_message branch February 8, 2019 00:08
andreidiaconu pushed a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
The app will still crash. Missing configuration is fatal. An app like this should never be released. A crash would be a good reminder for user to provide a proper fix before shipping the product.
After this fix, a proper error message is shown to users.
andreidiaconu added a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
karantanwar pushed a commit to karantanwar/plugins that referenced this pull request Feb 19, 2019
The app will still crash. Missing configuration is fatal. An app like this should never be released. A crash would be a good reminder for user to provide a proper fix before shipping the product.
After this fix, a proper error message is shown to users.
karantanwar pushed a commit to karantanwar/plugins that referenced this pull request Feb 19, 2019
The app will still crash. Missing configuration is fatal. An app like this should never be released. A crash would be a good reminder for user to provide a proper fix before shipping the product.
After this fix, a proper error message is shown to users.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

google_signin plugin crashes app if plist.info is incorrectly configured
3 participants