Skip to content

Commit c8b7441

Browse files
[CP-stable][Tool Robustness] Gracefully handle asynchronous subprocess crashes and connection timeouts (flutter#187118)
This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request. ### Issue Link: What is the link to the issue this cherry-pick is addressing? flutter#186962 flutter#186963 ### Impact Description: What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping of production apps (the app crashes on launch). This information is for domain experts and release engineers to understand the consequences of saying yes or no to the cherry pick. The Flutter Tool crashes instead of exiting gracefully. These two issues are top-10 crashers for the tool on 3.44.0. ### Changelog Description: Explain this cherry pick: * In one line that is accessible to most Flutter developers. * That describes the state prior to the fix. * That includes which platforms are impacted. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples. [flutter/186962] When the analysis server exits unexpectedly, the `flutter` tool can crash instead of outputting a descriptive error message. [flutter/186963] When failing to connect to a Chrome instance on Windows, the `flutter` tool can crash instead of outputting a descriptive error message. ### Workaround: Is there a workaround for this issue? No. ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: What are the steps to validate that this fix works? These crashes are flaky and hard to reproduce. Will need to monitor tool crash reports for the next stable release to verify.
1 parent c416acf commit c8b7441

4 files changed

Lines changed: 86 additions & 2 deletions

File tree

packages/flutter_tools/lib/src/commands/analyze_once.dart

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,9 @@ class AnalyzeOnce extends AnalyzeBase {
9595
if (!analysisCompleter.isCompleted) {
9696
analysisCompleter.completeError(
9797
// Include the last 20 lines of server output in exception message
98-
Exception(
98+
_AnalysisServerExitException(
9999
'analysis server exited with code $exitCode and output:\n${server.getLogs(20)}',
100+
exitCode,
100101
),
101102
);
102103
}
@@ -112,7 +113,11 @@ class AnalyzeOnce extends AnalyzeBase {
112113
? logger.startProgress('Analyzing $message...')
113114
: null;
114115

115-
await analysisCompleter.future;
116+
try {
117+
await analysisCompleter.future;
118+
} on _AnalysisServerExitException catch (error) {
119+
throwToolExit(error.message, exitCode: error.exitCode);
120+
}
116121
} finally {
117122
await server.dispose();
118123
progress?.cancel();
@@ -173,3 +178,9 @@ class AnalyzeOnce extends AnalyzeBase {
173178
return false;
174179
}
175180
}
181+
182+
class _AnalysisServerExitException implements Exception {
183+
_AnalysisServerExitException(this.message, this.exitCode);
184+
final String message;
185+
final int? exitCode;
186+
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,13 @@ class ResidentWebRunner extends ResidentRunner {
392392
appFailedToStart();
393393
_logger.printError(error.toString(), stackTrace: stackTrace);
394394
throwToolExit(kExitMessage);
395+
} on TimeoutException catch (error, stackTrace) {
396+
appFailedToStart();
397+
_logger.printError(
398+
'Failed to establish connection with the web debug service: $error',
399+
stackTrace: stackTrace,
400+
);
401+
throwToolExit('Failed to connect to the web debug service.');
395402
} on DartDevelopmentServiceException catch (error) {
396403
// The application may have started shutting down before DDS was able to finish establishing
397404
// its connection to DWDS. Don't treat this as an unhandled exception.

packages/flutter_tools/test/commands.shard/hermetic/analyze_test.dart

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import 'package:args/command_runner.dart';
99
import 'package:file/file.dart';
1010
import 'package:file/memory.dart';
1111
import 'package:flutter_tools/src/artifacts.dart';
12+
import 'package:flutter_tools/src/base/common.dart';
1213
import 'package:flutter_tools/src/base/logger.dart';
1314
import 'package:flutter_tools/src/base/platform.dart';
1415
import 'package:flutter_tools/src/base/terminal.dart';
@@ -114,6 +115,44 @@ void main() {
114115
},
115116
);
116117

118+
testUsingContext(
119+
'Analysis server premature exit with 255 throws ToolExit',
120+
() async {
121+
const stderr = 'Fatal error in analyzer';
122+
processManager.addCommands(<FakeCommand>[
123+
const FakeCommand(
124+
command: <String>[
125+
'Artifact.engineDartSdkPath/bin/dart',
126+
'language-server',
127+
'--dart-sdk',
128+
'Artifact.engineDartSdkPath',
129+
'--disable-server-feature-completion',
130+
'--disable-server-feature-search',
131+
'--suppress-analytics',
132+
],
133+
exitCode: 255,
134+
stderr: stderr,
135+
),
136+
]);
137+
await expectLater(
138+
runner.run(<String>['analyze']),
139+
throwsA(
140+
isA<ToolExit>()
141+
.having(
142+
(ToolExit e) => e.message,
143+
'message',
144+
contains('analysis server exited with code 255 and output:\n[stderr] $stderr'),
145+
)
146+
.having((ToolExit e) => e.exitCode, 'exitCode', equals(255)),
147+
),
148+
);
149+
},
150+
overrides: <Type, Generator>{
151+
FileSystem: () => fileSystem,
152+
ProcessManager: () => processManager,
153+
},
154+
);
155+
117156
testUsingContext(
118157
'--flutter-repo analyzes everything in the flutterRoot',
119158
() async {

packages/flutter_tools/test/general.shard/resident_web_runner_test.dart

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1963,6 +1963,33 @@ flutter:
19631963
},
19641964
);
19651965

1966+
testUsingContext(
1967+
'ResidentWebRunner throws ToolExit when DWDS debug connection times out',
1968+
() async {
1969+
final logger = BufferLogger.test();
1970+
final ResidentRunner residentWebRunner = setUpResidentRunner(flutterDevice, logger: logger);
1971+
fakeVmServiceHost = FakeVmServiceHost(requests: <VmServiceExpectation>[]);
1972+
setupMocks();
1973+
webDevFS.exception = TimeoutException('Connection timed out');
1974+
1975+
await expectLater(
1976+
residentWebRunner.run,
1977+
throwsToolExit(message: 'Failed to connect to the web debug service.'),
1978+
);
1979+
expect(
1980+
logger.errorText,
1981+
contains(
1982+
'Failed to establish connection with the web debug service: TimeoutException: Connection timed out',
1983+
),
1984+
);
1985+
},
1986+
overrides: <Type, Generator>{
1987+
FileSystem: () => fileSystem,
1988+
ProcessManager: () => processManager,
1989+
Pub: ThrowingPub.new,
1990+
},
1991+
);
1992+
19661993
testUsingContext(
19671994
'throws when port is an integer outside the valid TCP range',
19681995
() async {

0 commit comments

Comments
 (0)