catch `StateError` thrown from `Chromium.close` by andrewkolos · Pull Request #154366 · flutter/flutter

andrewkolos · 2024-08-29T21:23:59Z

Fixes crasher #154203.

Also does a little refactoring to move exception handling into the getChromeTabGuarded utility function.

I plan on cherry-picking a subset of this change. This would be included in the custom CP patch for #153064.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

andrewkolos · 2024-08-29T21:25:04Z

packages/flutter_tools/lib/src/web/chrome.dart

+}) async {
+  try {
+    return asyncGuard(() => chromeConnection.getTab(accept, retryFor: retryFor));
+  } on IOException catch (error, stackTrace) {


Maybe this should just be a blanket on (e) catch, but I'm being conservative.

christopherfujino · 2024-08-29T21:38:08Z

packages/flutter_tools/lib/src/isolated/resident_web_runner.dart

+          return !chromeTab.url.startsWith('chrome-extension');
+        },
+        retryFor: const Duration(seconds: 5),
+        onError: (Object error, StackTrace stackTrace) {


does this now mean we're swallowing all errors?

oh, it looks like this only gets delegated IOException and StateError?

oh yeah. Anything that's not IOException or StateError will fall through. Maybe it should just catch everything.

Addressed in refactor: add clarifying docs. I decided to just rename the parameter to not imply that any error would be caught.

christopherfujino · 2024-08-29T22:00:07Z

packages/flutter_tools/lib/src/test/flutter_web_platform.dart

        } on FormatException catch (ex) {
          _logger.printError('Caught FormatException: $ex');
          return shelf.Response.ok('Caught exception: $ex');
-        } on IOException catch (ex) {


Why are we not catching this here?

This catch was introduced recently, but before I introduced getChromeTabGuarded. getChromeTabGuarded should already catch this.

packages/flutter_tools/lib/src/web/chrome.dart

christopherfujino

LGTM

…ter#154366)

Fixes crasher flutter#154203. Also does a little refactoring to move exception handling into the `getChromeTabGuarded` utility function. I plan on cherry-picking a subset of this change. This would be included in the custom CP patch for flutter#153064.

…ter#154366)

…ction.getTab` (#154380) This is a custom hotfix patch to fix #154366 and #153064 on the stable channel. The reason for a custom patch rather than a bot-created PR is that the fixes on master were done by two sequential PRs (#153978 and #154366). This CP patch is also a subset of those changes (calls to Chrome when shutting down at the end of `flutter run -d chrome`).

…ter#154366)

andrewkolos added 2 commits August 29, 2024 14:20

catch StateError thrown from Chromium.close

4671dbc

migrate test usages to testWithoutContext

28a5d4d

andrewkolos added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 29, 2024

andrewkolos requested a review from christopherfujino August 29, 2024 21:24

andrewkolos commented Aug 29, 2024

View reviewed changes

andrewkolos marked this pull request as ready for review August 29, 2024 21:26

self-nit

39d12e9

christopherfujino reviewed Aug 29, 2024

View reviewed changes

packages/flutter_tools/lib/src/web/chrome.dart Show resolved Hide resolved

christopherfujino reviewed Aug 29, 2024

View reviewed changes

packages/flutter_tools/lib/src/web/chrome.dart Show resolved Hide resolved

refactor: add clarifying docs

9fd3e84

andrewkolos requested a review from christopherfujino August 29, 2024 23:10

christopherfujino approved these changes Aug 29, 2024

View reviewed changes

andrewkolos added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 29, 2024

auto-submit bot merged commit a8528c7 into flutter:master Aug 30, 2024

andrewkolos mentioned this pull request Aug 30, 2024

[CP-stable] When shutting down chrome, guard the call to ChromeConnection.getTab #154380

Merged

9 tasks

engine-flutter-autoroll mentioned this pull request Aug 30, 2024

Roll Flutter from b8f89f70bf4e to 967286b386b2 (135 revisions) flutter/packages#7550

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

a79fa7a

…ter#154366)

engine-flutter-autoroll mentioned this pull request Aug 30, 2024

Roll Flutter from b8f89f70bf4e to 08c3970f2b7c (136 revisions) flutter/packages#7551

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

27c0a8e

…ter#154366)

engine-flutter-autoroll mentioned this pull request Aug 30, 2024

Roll Flutter from b8f89f70bf4e to 357704aeb247 (137 revisions) flutter/packages#7552

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

ee2b976

…ter#154366)

engine-flutter-autoroll mentioned this pull request Aug 30, 2024

Roll Flutter from b8f89f70bf4e to 995a1da1045e (138 revisions) flutter/packages#7553

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

bd0b00e

…ter#154366)

engine-flutter-autoroll mentioned this pull request Aug 30, 2024

Roll Flutter from b8f89f70bf4e to e48a45ff5e41 (139 revisions) flutter/packages#7554

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

d04981d

…ter#154366)

engine-flutter-autoroll mentioned this pull request Aug 30, 2024

Roll Flutter from b8f89f70bf4e to 01e8ecaea608 (140 revisions) flutter/packages#7555

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

35d7cac

…ter#154366)

engine-flutter-autoroll mentioned this pull request Aug 30, 2024

Roll Flutter from b8f89f70bf4e to 0d8247eb7d9f (141 revisions) flutter/packages#7556

Closed

engine-flutter-autoroll mentioned this pull request Sep 1, 2024

Roll Flutter from b8f89f70bf4e to 6fe09872b12a (150 revisions) flutter/packages#7561

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 1, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

6517455

…ter#154366)

engine-flutter-autoroll mentioned this pull request Sep 3, 2024

Roll Flutter from b8f89f70bf4e to 164ff46f7c3b (151 revisions) flutter/packages#7566

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

1de891f

…ter#154366)

engine-flutter-autoroll mentioned this pull request Sep 3, 2024

Roll Flutter from b8f89f70bf4e to 29af4a174c89 (156 revisions) flutter/packages#7567

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

6bd885b

…ter#154366)

engine-flutter-autoroll mentioned this pull request Sep 3, 2024

Roll Flutter from b8f89f70bf4e to d1385a48271f (157 revisions) flutter/packages#7568

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

848a352

…ter#154366)

engine-flutter-autoroll mentioned this pull request Sep 3, 2024

Roll Flutter from b8f89f70bf4e to 7abeeb196b2a (158 revisions) flutter/packages#7569

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

cb58138

…ter#154366)

engine-flutter-autoroll mentioned this pull request Sep 3, 2024

Roll Flutter from b8f89f70bf4e to 6f19403eb6eb (159 revisions) flutter/packages#7570

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

b2c9537

…ter#154366)

engine-flutter-autoroll mentioned this pull request Sep 3, 2024

Roll Flutter from b8f89f70bf4e to 28a912430499 (162 revisions) flutter/packages#7572

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

1557b2f

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

7231c4f

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

a8f4d96

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

4924b02

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

b19f74a

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

e530f0a

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

9298a2b

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

74d01f6

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

49ba6e9

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

7733ecb

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

6062a72

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

82f5b14

…ter#154366)

andrewkolos mentioned this pull request Sep 10, 2024

Bad state: Client is closed #154203

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

c08102d

…ter#154366)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024

a8528c7 catch StateError thrown from Chromium.close (flutter/flut…

40e5782

…ter#154366)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

catch `StateError` thrown from `Chromium.close`#154366

catch `StateError` thrown from `Chromium.close`#154366
auto-submit[bot] merged 4 commits intoflutter:masterfrom
andrewkolos:catch-state-error-thrown-from-chrome.close

andrewkolos commented Aug 29, 2024

Uh oh!

andrewkolos Aug 29, 2024

Uh oh!

christopherfujino Aug 29, 2024

Uh oh!

christopherfujino Aug 29, 2024

Uh oh!

andrewkolos Aug 29, 2024

Uh oh!

andrewkolos Aug 29, 2024

Uh oh!

christopherfujino Aug 29, 2024

Uh oh!

andrewkolos Aug 29, 2024

Uh oh!

Uh oh!

Uh oh!

christopherfujino left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

andrewkolos commented Aug 29, 2024

Pre-launch Checklist

Uh oh!

andrewkolos Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

christopherfujino Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

christopherfujino Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

andrewkolos Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

andrewkolos Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

christopherfujino Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

andrewkolos Aug 29, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants