Skip to content

[google_sign_in] Don't crash a misconfigured iOS app #9486

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 8, 2025
Merged

[google_sign_in] Don't crash a misconfigured iOS app #9486

merged 1 commit into from
Jul 8, 2025
+20 −16

Conversation

stuartmorgan-g
Copy link
Contributor

In flutter/plugins#1180 the plugin was deliberately made to crash when misconfigured. The reasoning was that the app shouldn't be released this way, which is true, but crashing the entire app means that a developer has to use Xcode to debug the app to understand what's happening, which is a very poor user experience.

The PR indicated that the error would be printed from the Dart side after that PR, but that's not actually what happens; method channel returns are asynchronous, whereas crashing the app on the native side with an unhandled exception happens immediately, so even though the native side does call the completion, the async process of that completion being received on the Dart side as a PlatformException was never happening, and the crash was silent outside the context of a native debugger.

This removes the throw, so that it can be received as a Dart exception, which is much easier for most developers to debug.

Pre-Review Checklist

Footnotes

  1. 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

@stuartmorgan-g
[google_sign_in] Don't crash a misconfigured iOS app
499537d 
In flutter/plugins#1180 the plugin was
deliberately designed to crash when misconfigured. The reasoning was
that the app shouldn't be released this way, which is true, but crashing
the entire app means that a developer has to use Xcode to debug the app
to understand what's happening, which is a very poor user experience.

This removes the throw, so that it can be received as a Dart exception,
which is much easier for most developers to debug.
@stuartmorgan-g
Copy link
Contributor Author

There's no issue linked because this is something I just happened to hit while testing #9484 locally, and had messed up the configuration in my test app.

@stuartmorgan-g stuartmorgan-g added the triage-ios Should be looked at in iOS triage label Jun 24, 2025
LongCatIsLooong
LongCatIsLooong approved these changes Jul 7, 2025
View reviewed changes
Copy link
Contributor

@LongCatIsLooong LongCatIsLooong left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation!

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 8, 2025
@auto-submit auto-submit bot merged commit 337b1ad into flutter:main Jul 8, 2025
78 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 9, 2025
@engine-flutter-autoroll
337b1adc2 [google_sign_in] Don't crash a misconfigured iOS app (flutt…
6ef52e8 
…er/packages#9486)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 9, 2025
Merged
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Jul 9, 2025
@engine-flutter-autoroll
Roll Packages from cba2e9028121 to 4a231ae8546d (5 revisions) (#171879)
ac12f66 
flutter/packages@cba2e90...4a231ae

2025-07-08 [email protected] [video_player] Adds platform view
support on macOS (flutter/packages#9576)
2025-07-08 [email protected] [camera] fix `CameraLensType` export
(flutter/packages#9536)
2025-07-08 [email protected] [video_player] Move iOS/macOS to
per-player-instance Pigeon APIs (flutter/packages#9529)
2025-07-08 [email protected] Roll Flutter from
28a4e85 to adffe24 (17 revisions) (flutter/packages#9580)
2025-07-08 [email protected] [google_sign_in] Don't crash a
misconfigured iOS app (flutter/packages#9486)

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
@auto-submit auto-submit bot mentioned this pull request Jul 9, 2025
Merged
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Jul 9, 2025
@auto-submit
Reverts "Roll Packages from cba2e9028121 to 4a231ae8546d (5 revisions) (
5434b98 
#171879)" (#171897)

<!-- start_original_pr_link -->
Reverts: #171879
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: chinmaygarde
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: #171896
<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: engine-flutter-autoroll
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {fluttergithubbot}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:

flutter/packages@cba2e90...4a231ae

2025-07-08 [email protected] [video_player] Adds platform view
support on macOS (flutter/packages#9576)
2025-07-08 [email protected] [camera] fix `CameraLensType` export
(flutter/packages#9536)
2025-07-08 [email protected] [video_player] Move iOS/macOS to
per-player-instance Pigeon APIs (flutter/packages#9529)
2025-07-08 [email protected] Roll Flutter from
28a4e85 to adffe24 (17 revisions) (flutter/packages#9580)
2025-07-08 [email protected] [google_sign_in] Don't crash a
misconfigured iOS app (flutter/packages#9486)

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

<!-- end_revert_body -->

Co-authored-by: auto-submit[bot] <[email protected]>
@auto-submit auto-submit bot mentioned this pull request Jul 9, 2025
Merged
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Jul 9, 2025
@auto-submit
Reverts "Reverts "Roll Packages from cba2e9028121 to 4a231ae8546d (5 …
721ae43 
…revisions) (#171879)" (#171897)" (#171910)

<!-- start_original_pr_link -->
Reverts: #171897
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: matanlurey
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: Was not the reason for the test failure, it was an
OOB tag change.
<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: auto-submit[bot]
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {fluttergithubbot}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:
<!-- start_original_pr_link -->
Reverts: #171879
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: chinmaygarde
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: #171896
<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: engine-flutter-autoroll
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {fluttergithubbot}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:

flutter/packages@cba2e90...4a231ae

2025-07-08 [email protected] [video_player] Adds platform view
support on macOS (flutter/packages#9576)
2025-07-08 [email protected] [camera] fix `CameraLensType` export
(flutter/packages#9536)
2025-07-08 [email protected] [video_player] Move iOS/macOS to
per-player-instance Pigeon APIs (flutter/packages#9529)
2025-07-08 [email protected] Roll Flutter from
28a4e85 to adffe24 (17 revisions) (flutter/packages#9580)
2025-07-08 [email protected] [google_sign_in] Don't crash a
misconfigured iOS app (flutter/packages#9486)

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

<!-- end_revert_body -->

<!-- end_revert_body -->

Co-authored-by: auto-submit[bot] <[email protected]>
azatech pushed a commit to azatech/flutter that referenced this pull request Jul 10, 2025
@engine-flutter-autoroll @azatech
Roll Packages from cba2e9028121 to 4a231ae8546d (5 revisions) (flutte…
da195a9 
…r#171879)

flutter/packages@cba2e90...4a231ae

2025-07-08 [email protected] [video_player] Adds platform view
support on macOS (flutter/packages#9576)
2025-07-08 [email protected] [camera] fix `CameraLensType` export
(flutter/packages#9536)
2025-07-08 [email protected] [video_player] Move iOS/macOS to
per-player-instance Pigeon APIs (flutter/packages#9529)
2025-07-08 [email protected] Roll Flutter from
28a4e85 to adffe24 (17 revisions) (flutter/packages#9580)
2025-07-08 [email protected] [google_sign_in] Don't crash a
misconfigured iOS app (flutter/packages#9486)

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
azatech pushed a commit to azatech/flutter that referenced this pull request Jul 10, 2025
@auto-submit @azatech
Reverts "Roll Packages from cba2e9028121 to 4a231ae8546d (5 revisions) (
fde371e 
flutter#171879)" (flutter#171897)

<!-- start_original_pr_link -->
Reverts: flutter#171879
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: chinmaygarde
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: flutter#171896
<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: engine-flutter-autoroll
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {fluttergithubbot}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:

flutter/packages@cba2e90...4a231ae

2025-07-08 [email protected] [video_player] Adds platform view
support on macOS (flutter/packages#9576)
2025-07-08 [email protected] [camera] fix `CameraLensType` export
(flutter/packages#9536)
2025-07-08 [email protected] [video_player] Move iOS/macOS to
per-player-instance Pigeon APIs (flutter/packages#9529)
2025-07-08 [email protected] Roll Flutter from
28a4e85 to adffe24 (17 revisions) (flutter/packages#9580)
2025-07-08 [email protected] [google_sign_in] Don't crash a
misconfigured iOS app (flutter/packages#9486)

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

<!-- end_revert_body -->

Co-authored-by: auto-submit[bot] <[email protected]>
azatech pushed a commit to azatech/flutter that referenced this pull request Jul 10, 2025
@auto-submit @azatech
Reverts "Reverts "Roll Packages from cba2e9028121 to 4a231ae8546d (5 …
d4e75a7 
…revisions) (flutter#171879)" (flutter#171897)" (flutter#171910)

<!-- start_original_pr_link -->
Reverts: flutter#171897
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: matanlurey
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: Was not the reason for the test failure, it was an
OOB tag change.
<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: auto-submit[bot]
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {fluttergithubbot}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:
<!-- start_original_pr_link -->
Reverts: flutter#171879
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: chinmaygarde
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: flutter#171896
<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: engine-flutter-autoroll
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {fluttergithubbot}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:

flutter/packages@cba2e90...4a231ae

2025-07-08 [email protected] [video_player] Adds platform view
support on macOS (flutter/packages#9576)
2025-07-08 [email protected] [camera] fix `CameraLensType` export
(flutter/packages#9536)
2025-07-08 [email protected] [video_player] Move iOS/macOS to
per-player-instance Pigeon APIs (flutter/packages#9529)
2025-07-08 [email protected] Roll Flutter from
28a4e85 to adffe24 (17 revisions) (flutter/packages#9580)
2025-07-08 [email protected] [google_sign_in] Don't crash a
misconfigured iOS app (flutter/packages#9486)

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

<!-- end_revert_body -->

<!-- end_revert_body -->

Co-authored-by: auto-submit[bot] <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LongCatIsLooong LongCatIsLooong LongCatIsLooong approved these changes

@cbracken cbracken Awaiting requested review from cbracken cbracken is a code owner

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: google_sign_in platform-ios platform-macos triage-ios Should be looked at in iOS triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stuartmorgan-g @LongCatIsLooong