-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[local_auth] Adopt structured errors - platform interface #10023
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[local_auth] Adopt structured errors - platform interface #10023
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces structured error handling to the local_auth platform interface by adding a new LocalAuthException class and a LocalAuthExceptionCode enum. This is a valuable improvement as it allows for more granular and consistent error handling across platforms. The contract of the authenticate method has been clarified in the documentation to reflect these changes. Additionally, the useErrorDialogs option in AuthenticationOptions is now deprecated. The changes are well-structured and include necessary updates to tests, the changelog, and pubspec. I have one minor suggestion to enhance the new exception class for better debuggability.
| final Object? details; | ||
|
|
||
| @override | ||
| String toString() => 'LocalAuthException(code $code, $description, $details)'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better readability and consistency with Dart conventions, it's good practice for toString() to return a string that resembles a constructor call with named parameters. Using code.name for the enum value will also produce a cleaner output, which is helpful for debugging.
| String toString() => 'LocalAuthException(code $code, $description, $details)'; | |
| String toString() => 'LocalAuthException(code: ${code.name}, description: $description, details: $details)'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I adjusted to follow the Flutter style for toString, using objectRuntimeType, which I had forgotten to do. Flutter style doesn't do named arguments for everything, but I did add the .name for code.
tarrinneal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one typo and lgtm
| /// such as no biometric sensor on device, will still result in | ||
| /// a [PlatformException]. | ||
| // This parameter still exists for backwards compatibility with local_auth | ||
| // 2.x, but implementers targetting local_auth 3.x or later should ignore it, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
targeting
flutter/packages@287739d...321a584 2025-09-30 [email protected] [local_auth] Adopt structured errors - platform interface (flutter/packages#10023) 2025-09-30 [email protected] Revert hardcoded compilesdk back to `flutter.compileSdkVersion` in `camera_android` (flutter/packages#9668) 2025-09-30 [email protected] [file_selector] updates build files to use JVM 17 (flutter/packages#10097) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…r#176357) flutter/packages@287739d...321a584 2025-09-30 [email protected] [local_auth] Adopt structured errors - platform interface (flutter/packages#10023) 2025-09-30 [email protected] Revert hardcoded compilesdk back to `flutter.compileSdkVersion` in `camera_android` (flutter/packages#9668) 2025-09-30 [email protected] [file_selector] updates build files to use JVM 17 (flutter/packages#10097) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Platform interface portion of #9981
LocalAuthExceptionclass for structured errors.AuthenticationOptions.useErrorDialogsPart of:
local_authplugin flutter#113687useErrorDialogsflutter#175125Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).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 ↩3