Skip to content

Commit a8528c7

Browse files
authored
catch StateError thrown from Chromium.close (flutter#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.
1 parent 680f33d commit a8528c7

File tree

4 files changed

+74
-33
lines changed

4 files changed

+74
-33
lines changed

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

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -608,20 +608,17 @@ Please provide a valid TCP port (an integer between 0 and 65535, inclusive).
608608
}) async {
609609
if (_chromiumLauncher != null) {
610610
final Chromium chrome = await _chromiumLauncher!.connectedInstance;
611-
ChromeTab? chromeTab;
612-
try {
613-
chromeTab = await getChromeTabGuarded(
614-
chrome.chromeConnection,
615-
(ChromeTab chromeTab) {
616-
return !chromeTab.url.startsWith('chrome-extension');
617-
},
618-
retryFor: const Duration(seconds: 5),
619-
);
620-
} on HttpException catch (error, stackTrace) {
621-
// We were unable to unable to communicate with Chrome.
622-
_logger.printError(error.toString(), stackTrace: stackTrace);
623-
chromeTab = null;
624-
}
611+
final ChromeTab? chromeTab = await getChromeTabGuarded(
612+
chrome.chromeConnection,
613+
(ChromeTab chromeTab) {
614+
return !chromeTab.url.startsWith('chrome-extension');
615+
},
616+
retryFor: const Duration(seconds: 5),
617+
onIoError: (Object error, StackTrace stackTrace) {
618+
// We were unable to unable to communicate with Chrome.
619+
_logger.printError(error.toString(), stackTrace: stackTrace);
620+
}
621+
);
625622
if (chromeTab == null) {
626623
appFailedToStart();
627624
throwToolExit('Failed to connect to Chrome instance.');

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -454,9 +454,6 @@ class FlutterWebPlatform extends PlatformPlugin {
454454
} on FormatException catch (ex) {
455455
_logger.printError('Caught FormatException: $ex');
456456
return shelf.Response.ok('Caught exception: $ex');
457-
} on IOException catch (ex) {
458-
_logger.printError('Caught IOException: $ex');
459-
return shelf.Response.ok('Caught exception: $ex');
460457
}
461458
}
462459
final String? errorMessage = await _testGoldenComparator.compareGoldens(testUri, bytes, goldenKey, updateGoldens);

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

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import 'dart:async';
66

77
import 'package:meta/meta.dart';
88
import 'package:process/process.dart';
9-
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
9+
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'
10+
hide StackTrace;
1011

1112
import '../base/async_guard.dart';
1213
import '../base/common.dart';
@@ -563,12 +564,37 @@ class Chromium {
563564
}
564565
}
565566

566-
// TODO(andrewkolos): Remove when https://github.com/dart-lang/sdk/issues/56566
567-
// is fixed.
567+
568+
/// Wrapper for [ChromeConnection.getTab] that will catch any [IOException] or
569+
/// [StateError], delegate it to the [onIoError] callback, and return null.
570+
///
571+
/// This is useful for callers who are want to retrieve a [ChromeTab], but
572+
/// are okay with the operation failing (e.g. due to an network IO issue or
573+
/// the Chrome process no longer existing).
568574
Future<ChromeTab?> getChromeTabGuarded(
569575
ChromeConnection chromeConnection,
570576
bool Function(ChromeTab tab) accept, {
571577
Duration? retryFor,
572-
}) {
573-
return asyncGuard(() => chromeConnection.getTab(accept, retryFor: retryFor));
578+
void Function(Object error, StackTrace stackTrace)? onIoError,
579+
}) async {
580+
try {
581+
return asyncGuard(
582+
() => chromeConnection.getTab(
583+
accept,
584+
retryFor: retryFor,
585+
),
586+
);
587+
} on IOException catch (error, stackTrace) {
588+
if (onIoError != null) {
589+
onIoError(error, stackTrace);
590+
}
591+
return null;
592+
// The underlying HttpClient will throw a StateError when it tries to
593+
// perform a request despite the connection already being closed.
594+
} on StateError catch (error, stackTrace) {
595+
if (onIoError != null) {
596+
onIoError(error, stackTrace);
597+
}
598+
return null;
599+
}
574600
}

packages/flutter_tools/test/web.shard/chrome_test.dart

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ void main() {
758758
expect(logger.errorText, contains('SocketException'));
759759
});
760760

761-
test('can recover if getTabs throws a connection exception', () async {
761+
testWithoutContext('can recover if getTabs throws a connection exception', () async {
762762
final BufferLogger logger = BufferLogger.test();
763763
final FakeChromeConnection chromeConnection = FakeChromeConnection(maxRetries: 4);
764764
final ChromiumLauncher chromiumLauncher = ChromiumLauncher(
@@ -775,11 +775,11 @@ void main() {
775775
expect(logger.errorText, isEmpty);
776776
});
777777

778-
test('can recover if getTabs throws an HttpException', () async {
778+
testWithoutContext('can recover if getTabs throws an HttpException', () async {
779779
final BufferLogger logger = BufferLogger.test();
780780
final FakeChromeConnection chromeConnection = FakeChromeConnection(
781781
maxRetries: 4,
782-
exception: io.HttpException(
782+
error: io.HttpException(
783783
'Connection closed before full header was received',
784784
uri: Uri.parse('http://localhost:52097/json'),
785785
),
@@ -798,7 +798,27 @@ void main() {
798798
expect(logger.errorText, isEmpty);
799799
});
800800

801-
test('exits if getTabs throws a connection exception consistently', () async {
801+
testWithoutContext('chrome.close can recover if getTab throws a StateError', () async {
802+
final BufferLogger logger = BufferLogger.test();
803+
final FakeChromeConnection chromeConnection = FakeChromeConnection(
804+
maxRetries: 4,
805+
error: StateError('Client is closed.'),
806+
);
807+
final ChromiumLauncher chromiumLauncher = ChromiumLauncher(
808+
fileSystem: fileSystem,
809+
platform: platform,
810+
processManager: processManager,
811+
operatingSystemUtils: operatingSystemUtils,
812+
browserFinder: findChromeExecutable,
813+
logger: logger,
814+
);
815+
final FakeProcess process = FakeProcess();
816+
final Chromium chrome = Chromium(0, chromeConnection, chromiumLauncher: chromiumLauncher, process: process, logger: logger,);
817+
await chrome.close();
818+
expect(logger.errorText, isEmpty);
819+
});
820+
821+
testWithoutContext('exits if getTabs throws a connection exception consistently', () async {
802822
final BufferLogger logger = BufferLogger.test();
803823
final FakeChromeConnection chromeConnection = FakeChromeConnection();
804824
final ChromiumLauncher chromiumLauncher = ChromiumLauncher(
@@ -825,7 +845,7 @@ void main() {
825845
));
826846
});
827847

828-
test('Chromium close sends browser close command', () async {
848+
testWithoutContext('Chromium close sends browser close command', () async {
829849
final BufferLogger logger = BufferLogger.test();
830850
final List<String> commands = <String>[];
831851
void onSendCommand(String cmd) { commands.add(cmd); }
@@ -845,7 +865,7 @@ void main() {
845865
expect(commands, contains('Browser.close'));
846866
});
847867

848-
test('Chromium close handles a SocketException when connecting to Chrome', () async {
868+
testWithoutContext('Chromium close handles a SocketException when connecting to Chrome', () async {
849869
final BufferLogger logger = BufferLogger.test();
850870
final FakeChromeConnectionWithTab chromeConnection = FakeChromeConnectionWithTab();
851871
final ChromiumLauncher chromiumLauncher = ChromiumLauncher(
@@ -863,7 +883,7 @@ void main() {
863883
await chrome.close();
864884
});
865885

866-
test('Chromium close handles a WebSocketException when closing the WipConnection', () async {
886+
testWithoutContext('Chromium close handles a WebSocketException when closing the WipConnection', () async {
867887
final BufferLogger logger = BufferLogger.test();
868888
final FakeChromeConnectionWithTab chromeConnection = FakeChromeConnectionWithTab(throwWebSocketException: true);
869889
final ChromiumLauncher chromiumLauncher = ChromiumLauncher(
@@ -887,8 +907,8 @@ class FakeChromeConnection extends Fake implements ChromeConnection {
887907
/// Create a connection that throws a connection exception on first
888908
/// [maxRetries] calls to [getTabs].
889909
/// If [maxRetries] is `null`, [getTabs] calls never succeed.
890-
FakeChromeConnection({this.maxRetries, Exception? exception}) : _retries = 0 {
891-
this.exception = exception ??
910+
FakeChromeConnection({this.maxRetries, Object? error}) : _retries = 0 {
911+
this.error = error ??
892912
ConnectionException(
893913
formatException: const FormatException('incorrect format'),
894914
responseStatus: 'OK,',
@@ -899,7 +919,7 @@ class FakeChromeConnection extends Fake implements ChromeConnection {
899919
final List<ChromeTab> tabs = <ChromeTab>[];
900920
final int? maxRetries;
901921
int _retries;
902-
late final Exception exception;
922+
late final Object error;
903923

904924
@override
905925
Future<ChromeTab?> getTab(bool Function(ChromeTab tab) accept, {Duration? retryFor}) async {
@@ -910,7 +930,8 @@ class FakeChromeConnection extends Fake implements ChromeConnection {
910930
Future<List<ChromeTab>> getTabs({Duration? retryFor}) async {
911931
_retries ++;
912932
if (maxRetries == null || _retries < maxRetries!) {
913-
throw exception;
933+
// ignore: only_throw_errors -- This is fine for an ad-hoc test.
934+
throw error;
914935
}
915936
return tabs;
916937
}

0 commit comments

Comments
 (0)