[google_sign_in] allow developer to handle platform exception when user cancels google sign in flow #6098
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| await expectLater( | ||
| googleSignIn.signIn(), | ||
| throwsA(isA<PlatformException>().having( | ||
| (e) => e.code, |
There was a problem hiding this comment.
info - test/google_sign_in_test.dart:155:16 - Missing type annotation. Try specifying the type 'PlatformException'. - always_specify_types
error is from Luci, you can add the datatype here for e to fix that.
There was a problem hiding this comment.
Thank you @piedcipher for this fix. 👍
There was a problem hiding this comment.
happy to help!
Linux repo_checks is failing because of:
--packages-for-branch: running on branch "HEAD".
Running for all packages that have diffs relative to "0a692590a8853c6b3ea7b8e5ef16d1b746b2d39a"
Changed packages: google_sign_in/google_sign_in
[0:00] Running for packages/google_sign_in/google_sign_in...
No version change.
Found NEXT; validating next version in the CHANGELOG.
No version change found, but the change to this package could not be verified to be exempt
from version changes according to repository policy.
If this is a false positive, please comment in the PR to explain why the PR
is exempt, and add (or ask your reviewer to add) the "override: no versioning needed" label.
The following packages had errors:
packages/google_sign_in/google_sign_in:
Missing version change
See above for full details.
it seems a reviewer (part of the team) can fix it. congrats on your first pr!
There was a problem hiding this comment.
Thanks @piedcipher for your support in getting this PR rolling 🎉
There was a problem hiding this comment.
I'm glad I could, great work! 🙌🎉
There was a problem hiding this comment.
For future reference:
it seems a reviewer (part of the team) can fix it.
Per the error message, this would be true only for:
If this is a false positive, please comment in the PR to explain why the PR
is exempt
There is no such comment posted here (and it would be difficult for there to be, as this PR is not exempt for any of the reasons listed in the link from the PR checklist).
The actual fix for the CI failure here would be to complete the PR checklist.
packages/google_sign_in/google_sign_in/test/google_sign_in_test.dart
Outdated
Show resolved
Hide resolved
Co-authored-by: Tirth <[email protected]>
|
Thanks for the contribution, but this is not a change we want to make to the plugin. The documented behavior is to return If you are interested in fixing the referenced issue, we'd be glad to review a PR that properly catches exceptions and returns null in the cases that are currently failing to do so. |
This PR will pass PlatformException for kSignInCanceledError to App Level to provide flexibility to handle those as per the requirement of the development team without hindering catching the error inside the plugin.
flutter/flutter#85785
Reference: flutter/flutter#85785 (comment)
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.