[google_sign_in_ios] handle restorePreviousSignIn invalid grant exception#11349
[google_sign_in_ios] handle restorePreviousSignIn invalid grant exception#11349felangel wants to merge 3 commits intoflutter:mainfrom
restorePreviousSignIn invalid grant exception#11349Conversation
… `PlatformException`
There was a problem hiding this comment.
Code Review
This pull request enhances the google_sign_in_ios package by adding error handling for restorePreviousSignIn() calls. It now includes a try-catch block to manage exceptions, specifically when tokens are expired or revoked, and attempts a new sign-in in such cases. A corresponding test has been added to validate this new behavior. Feedback suggests refining the exception handling to catch PlatformException instead of a general Exception for better error specificity.
restorePreviousSignIn invalid_grant PlatformExceptionrestorePreviousSignIn invalid grant exception
|
An existing Git SHA, To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with |
| if (userId == null) { | ||
| SignInResult result = await _api.restorePreviousSignIn(); | ||
| SignInResult result; | ||
| try { |
There was a problem hiding this comment.
The intended design of this plugin is that we catch errors on the native side, where we still have full context. What exactly is throwing here? Is this throwing instead of calling the completion handler?
If so, we should catch it there, inspect the error, and turn this specific one into a known error value, and anything else into an unknown error, via the error return system used by all the other native code in this plugin. (We should also file an issue upstream if that's the case, since that would be surprising behavior.)
There was a problem hiding this comment.
The intended design of this plugin is that we catch errors on the native side, where we still have full context. What exactly is throwing here? Is this throwing instead of calling the completion handler?
I believe that is correct 👍
If so, we should catch it there, inspect the error, and turn this specific one into a known error value, and anything else into an unknown error, via the error return system used by all the other native code in this plugin.
Okay I can try to spend some time on that later this week. Feel free to take this over if you get to it before me.
We should also file an issue upstream if that's the case, since that would be surprising behavior.
Where exactly should this issue be filed?
There was a problem hiding this comment.
Where exactly should this issue be filed?
If the Google Sign In SDK is throwing an exception instead of calling the callback, that should be filed here.
Description
Adds exception handling to
_getAuthorizationTokensto handlePlatformExceptionsthrown byrestorePreviousSignIn. This fixes a bug that would causeauthorizeServerto throw aPlatformExceptiondue to an expired or revoked token.restorePreviousSignIncan throwPlatformExceptiondue to invalid grant flutter#184134Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2