Catch exceptions thrown by runChecked* when possible (#36109)
diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index ef6eb69..fba8408 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart
@@ -378,10 +378,14 @@ printError('$installResult'); return false; } - - await runAdbCheckedAsync(<String>[ - 'shell', 'echo', '-n', _getSourceSha1(app), '>', _getDeviceSha1Path(app), - ]); + try { + await runAdbCheckedAsync(<String>[ + 'shell', 'echo', '-n', _getSourceSha1(app), '>', _getDeviceSha1Path(app), + ]); + } on ProcessException catch (error) { + printError('adb shell failed to write the SHA hash: $error.'); + return false; + } return true; } @@ -390,7 +394,13 @@ if (!await _checkForSupportedAdbVersion() || !await _checkForSupportedAndroidVersion()) return false; - final String uninstallOut = (await runCheckedAsync(adbCommandForDevice(<String>['uninstall', app.id]))).stdout; + String uninstallOut; + try { + uninstallOut = (await runCheckedAsync(adbCommandForDevice(<String>['uninstall', app.id]))).stdout; + } catch (error) { + printError('adb uninstall failed: $error'); + return false; + } final RegExp failureExp = RegExp(r'^Failure.*$', multiLine: true); final String failure = failureExp.stringMatch(uninstallOut); if (failure != null) { @@ -615,13 +625,18 @@ static final RegExp _timeRegExp = RegExp(r'^\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d{3}', multiLine: true); - /// Return the most recent timestamp in the Android log or null if there is + /// Return the most recent timestamp in the Android log or [null] if there is /// no available timestamp. The format can be passed to logcat's -T option. String get lastLogcatTimestamp { - final String output = runAdbCheckedSync(<String>[ - 'shell', '-x', 'logcat', '-v', 'time', '-t', '1', - ]); - + String output; + try { + output = runAdbCheckedSync(<String>[ + 'shell', '-x', 'logcat', '-v', 'time', '-t', '1', + ]); + } catch (error) { + printError('Failed to extract the most recent timestamp from the Android log: $error.'); + return null; + } final Match timeMatch = _timeRegExp.firstMatch(output); return timeMatch?.group(0); } @@ -940,9 +955,15 @@ List<ForwardedPort> get forwardedPorts { final List<ForwardedPort> ports = <ForwardedPort>[]; - final String stdout = runCheckedSync(device.adbCommandForDevice( - <String>['forward', '--list'] - )); + String stdout; + try { + stdout = runCheckedSync(device.adbCommandForDevice( + <String>['forward', '--list'] + )); + } catch (error) { + printError('Failed to list forwarded ports: $error.'); + return ports; + } final List<String> lines = LineSplitter.split(stdout).toList(); for (String line in lines) {
diff --git a/packages/flutter_tools/lib/src/application_package.dart b/packages/flutter_tools/lib/src/application_package.dart index ad273fd..a032cfd 100644 --- a/packages/flutter_tools/lib/src/application_package.dart +++ b/packages/flutter_tools/lib/src/application_package.dart
@@ -114,16 +114,21 @@ return null; } - final List<String> aaptArgs = <String>[ - aaptPath, - 'dump', - 'xmltree', - apk.path, - 'AndroidManifest.xml', - ]; + String apptStdout; + try { + apptStdout = runCheckedSync(<String>[ + aaptPath, + 'dump', + 'xmltree', + apk.path, + 'AndroidManifest.xml', + ]); + } catch (error) { + printError('Failed to extract manifest from APK: $error.'); + return null; + } - final ApkManifestData data = ApkManifestData - .parseFromXmlDump(runCheckedSync(aaptArgs)); + final ApkManifestData data = ApkManifestData.parseFromXmlDump(apptStdout); if (data == null) { printError('Unable to read manifest info from ${apk.path}.');
diff --git a/packages/flutter_tools/lib/src/commands/upgrade.dart b/packages/flutter_tools/lib/src/commands/upgrade.dart index 583a701..51c7ee8 100644 --- a/packages/flutter_tools/lib/src/commands/upgrade.dart +++ b/packages/flutter_tools/lib/src/commands/upgrade.dart
@@ -8,6 +8,7 @@ import '../base/common.dart'; import '../base/file_system.dart'; +import '../base/io.dart'; import '../base/os.dart'; import '../base/process.dart'; import '../cache.dart'; @@ -98,8 +99,14 @@ 'git', 'status', '-s' ], workingDirectory: Cache.flutterRoot); return result.stdout.trim().isNotEmpty; - } catch (e) { - throwToolExit('git status failed: $e'); + } on ProcessException catch (error) { + throwToolExit( + 'The tool could not verify the status of the current flutter checkout. ' + 'This might be due to git not being installed or an internal error.' + 'If it is okay to ignore potential local changes, then re-run this' + 'command with --force.' + '\nError: $error.' + ); } return false; } @@ -132,11 +139,17 @@ } else { tag = 'v${gitTagVersion.x}.${gitTagVersion.y}.${gitTagVersion.z}'; } - final RunResult runResult = await runCheckedAsync(<String>[ - 'git', 'reset', '--hard', tag, - ], workingDirectory: Cache.flutterRoot); - if (runResult.exitCode != 0) { - throwToolExit('Failed to restore branch from hotfix.'); + try { + await runCheckedAsync(<String>[ + 'git', 'reset', '--hard', tag, + ], workingDirectory: Cache.flutterRoot); + } on ProcessException catch (error) { + throwToolExit( + 'Unable to upgrade Flutter: The tool could not update to the version $tag. ' + 'This may be due to git not being installed or an internal error.' + 'Please ensure that git is installed on your computer and retry again.' + '\nError: $error.' + ); } }
diff --git a/packages/flutter_tools/lib/src/commands/version.dart b/packages/flutter_tools/lib/src/commands/version.dart index f3c8d32..ed71c55 100644 --- a/packages/flutter_tools/lib/src/commands/version.dart +++ b/packages/flutter_tools/lib/src/commands/version.dart
@@ -6,6 +6,7 @@ import '../base/common.dart'; import '../base/file_system.dart'; +import '../base/io.dart'; import '../base/os.dart'; import '../base/process.dart'; import '../base/version.dart'; @@ -34,10 +35,19 @@ Version minSupportedVersion = Version.parse('1.2.1'); Future<List<String>> getTags() async { - final RunResult runResult = await runCheckedAsync( - <String>['git', 'tag', '-l', 'v*', '--sort=-creatordate'], - workingDirectory: Cache.flutterRoot, - ); + RunResult runResult; + try { + runResult = await runCheckedAsync( + <String>['git', 'tag', '-l', 'v*', '--sort=-creatordate'], + workingDirectory: Cache.flutterRoot, + ); + } on ProcessException catch (error) { + throwToolExit( + 'Unable to get the tags. ' + 'This might be due to git not being installed or an internal error' + '\nError: $error.' + ); + } return runResult.toString().split('\n'); }
diff --git a/packages/flutter_tools/lib/src/ios/code_signing.dart b/packages/flutter_tools/lib/src/ios/code_signing.dart index 90cca80..bbefcbf 100644 --- a/packages/flutter_tools/lib/src/ios/code_signing.dart +++ b/packages/flutter_tools/lib/src/ios/code_signing.dart
@@ -120,7 +120,16 @@ const List<String> findIdentityCommand = <String>['security', 'find-identity', '-p', 'codesigning', '-v']; - final List<String> validCodeSigningIdentities = runCheckedSync(findIdentityCommand) + + String findIdentityStdout; + try { + findIdentityStdout = runCheckedSync(findIdentityCommand); + } catch (error) { + printTrace('Unexpected failure from find-identity: $error.'); + return null; + } + + final List<String> validCodeSigningIdentities = findIdentityStdout .split('\n') .map<String>((String outputLine) { return _securityFindIdentityDeveloperIdentityExtractionPattern @@ -148,12 +157,18 @@ if (signingCertificateId == null) return null; - final String signingCertificate = runCheckedSync( - <String>['security', 'find-certificate', '-c', signingCertificateId, '-p'] - ); + String signingCertificateStdout; + try { + signingCertificateStdout = runCheckedSync( + <String>['security', 'find-certificate', '-c', signingCertificateId, '-p'] + ); + } catch (error) { + printTrace('Couldn\'t find the certificate: $error.'); + return null; + } final Process opensslProcess = await runCommand(const <String>['openssl', 'x509', '-subject']); - await (opensslProcess.stdin..write(signingCertificate)).close(); + await (opensslProcess.stdin..write(signingCertificateStdout)).close(); final String opensslOutput = await utf8.decodeStream(opensslProcess.stdout); // Fire and forget discard of the stderr stream so we don't hold onto resources.
diff --git a/packages/flutter_tools/lib/src/ios/xcodeproj.dart b/packages/flutter_tools/lib/src/ios/xcodeproj.dart index 5b7a91f..7be0ef1 100644 --- a/packages/flutter_tools/lib/src/ios/xcodeproj.dart +++ b/packages/flutter_tools/lib/src/ios/xcodeproj.dart
@@ -174,15 +174,20 @@ } Map<String, String> getBuildSettings(String projectPath, String target) { - final String out = runCheckedSync(<String>[ - _executable, - '-project', - fs.path.absolute(projectPath), - '-target', - target, - '-showBuildSettings', - ], workingDirectory: projectPath); - return parseXcodeBuildSettings(out); + try { + final String out = runCheckedSync(<String>[ + _executable, + '-project', + fs.path.absolute(projectPath), + '-target', + target, + '-showBuildSettings', + ], workingDirectory: projectPath); + return parseXcodeBuildSettings(out); + } catch(error) { + printTrace('Unexpected failure to get the build settings: $error.'); + return const <String, String>{}; + } } Future<XcodeProjectInfo> getInfo(String projectPath) async {
diff --git a/packages/flutter_tools/test/general.shard/android/android_device_test.dart b/packages/flutter_tools/test/general.shard/android/android_device_test.dart index c439573..96c5632 100644 --- a/packages/flutter_tools/test/general.shard/android/android_device_test.dart +++ b/packages/flutter_tools/test/general.shard/android/android_device_test.dart
@@ -10,6 +10,7 @@ import 'package:flutter_tools/src/android/android_console.dart'; import 'package:flutter_tools/src/android/android_device.dart'; import 'package:flutter_tools/src/android/android_sdk.dart'; +import 'package:flutter_tools/src/application_package.dart'; import 'package:flutter_tools/src/base/config.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/io.dart'; @@ -365,7 +366,7 @@ testUsingContext('returns the generated host port from stdout', () async { when(mockProcessManager.run(argThat(contains('forward')))) - .thenAnswer((_) async => ProcessResult(0, 0, '456', '')); + .thenAnswer((_) async => ProcessResult(0, 0, '456', '')); expect(await forwarder.forward(123), equals(456)); }, overrides: <Type, Generator>{ @@ -374,7 +375,7 @@ testUsingContext('returns the supplied host port when stdout is empty', () async { when(mockProcessManager.run(argThat(contains('forward')))) - .thenAnswer((_) async => ProcessResult(0, 0, '', '')); + .thenAnswer((_) async => ProcessResult(0, 0, '', '')); expect(await forwarder.forward(123, hostPort: 456), equals(456)); }, overrides: <Type, Generator>{ @@ -383,7 +384,7 @@ testUsingContext('returns the supplied host port when stdout is the host port', () async { when(mockProcessManager.run(argThat(contains('forward')))) - .thenAnswer((_) async => ProcessResult(0, 0, '456', '')); + .thenAnswer((_) async => ProcessResult(0, 0, '456', '')); expect(await forwarder.forward(123, hostPort: 456), equals(456)); }, overrides: <Type, Generator>{ @@ -392,12 +393,34 @@ testUsingContext('throws an error when stdout is not blank nor the host port', () async { when(mockProcessManager.run(argThat(contains('forward')))) - .thenAnswer((_) async => ProcessResult(0, 0, '123456', '')); + .thenAnswer((_) async => ProcessResult(0, 0, '123456', '')); expect(forwarder.forward(123, hostPort: 456), throwsA(isInstanceOf<ProcessException>())); }, overrides: <Type, Generator>{ ProcessManager: () => mockProcessManager, }); + + testUsingContext('forwardedPorts returns empty list when forward failed', () { + when(mockProcessManager.runSync(argThat(contains('forward')))) + .thenReturn(ProcessResult(0, 1, '', '')); + + expect(forwarder.forwardedPorts, equals(const <ForwardedPort>[])); + }, overrides: <Type, Generator>{ + ProcessManager: () => mockProcessManager, + }); + }); + + group('lastLogcatTimestamp', () { + final ProcessManager mockProcessManager = MockProcessManager(); + final AndroidDevice device = AndroidDevice('1234'); + + testUsingContext('returns null if shell command failed', () async { + when(mockProcessManager.runSync(argThat(contains('logcat')))) + .thenReturn(ProcessResult(0, 1, '', '')); + expect(device.lastLogcatTimestamp, isNull); + }, overrides: <Type, Generator>{ + ProcessManager: () => mockProcessManager, + }); }); } @@ -601,3 +624,10 @@ @override void add(List<int> data) {} } + +class AndroidPackageTest extends ApplicationPackage { + AndroidPackageTest() : super(id: 'app-id'); + + @override + String get name => 'app-package'; +}
diff --git a/packages/flutter_tools/test/general.shard/application_package_test.dart b/packages/flutter_tools/test/general.shard/application_package_test.dart index 620ee3f..a531335 100644 --- a/packages/flutter_tools/test/general.shard/application_package_test.dart +++ b/packages/flutter_tools/test/general.shard/application_package_test.dart
@@ -129,6 +129,15 @@ ), ); }, overrides: overrides); + + testUsingContext('returns null when failed to extract manifest', () async { + final AndroidSdkVersion sdkVersion = MockitoAndroidSdkVersion(); + when(sdk.latestVersion).thenReturn(sdkVersion); + when(mockProcessManager.runSync(argThat(contains('logcat')))) + .thenReturn(ProcessResult(0, 1, '', '')); + + expect(AndroidApk.fromApk(null), isNull); + }, overrides: overrides); }); group('ApkManifestData', () {
diff --git a/packages/flutter_tools/test/general.shard/commands/version_test.dart b/packages/flutter_tools/test/general.shard/commands/version_test.dart index 1e48004..304e3e0 100644 --- a/packages/flutter_tools/test/general.shard/commands/version_test.dart +++ b/packages/flutter_tools/test/general.shard/commands/version_test.dart
@@ -6,6 +6,7 @@ import 'dart:convert'; import 'dart:io'; +import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/version.dart'; @@ -59,12 +60,29 @@ }, overrides: <Type, Generator>{ ProcessManager: () => MockProcessManager(), }); + + testUsingContext('exit tool if can\'t get the tags', () async { + final VersionCommand command = VersionCommand(); + + try { + await command.getTags(); + fail('ToolExit expected'); + } catch(e) { + expect(e, isInstanceOf<ToolExit>()); + } + }, overrides: <Type, Generator>{ + ProcessManager: () => MockProcessManager(failGitTag: true), + }); }); } class MockProcessManager extends Mock implements ProcessManager { + MockProcessManager({ this.failGitTag = false }); + String version = ''; + bool failGitTag; + @override Future<ProcessResult> run( List<dynamic> command, { @@ -76,6 +94,9 @@ Encoding stderrEncoding = systemEncoding, }) async { if (command[0] == 'git' && command[1] == 'tag') { + if (failGitTag) { + return ProcessResult(0, 1, '', ''); + } return ProcessResult(0, 0, 'v10.0.0\r\nv20.0.0', ''); } if (command[0] == 'git' && command[1] == 'checkout') {
diff --git a/packages/flutter_tools/test/general.shard/ios/code_signing_test.dart b/packages/flutter_tools/test/general.shard/ios/code_signing_test.dart index 57a2242..a90b9b2 100644 --- a/packages/flutter_tools/test/general.shard/ios/code_signing_test.dart +++ b/packages/flutter_tools/test/general.shard/ios/code_signing_test.dart
@@ -440,6 +440,62 @@ Config: () => mockConfig, AnsiTerminal: () => testTerminal, }); + + testUsingContext('find-identity failure', () async { + when(mockProcessManager.runSync(<String>['which', 'security'])) + .thenReturn(exitsHappy); + when(mockProcessManager.runSync(<String>['which', 'openssl'])) + .thenReturn(exitsHappy); + when(mockProcessManager.runSync( + argThat(contains('find-identity')), + environment: anyNamed('environment'), + workingDirectory: anyNamed('workingDirectory'), + )).thenReturn(ProcessResult(0, 1, '', '')); + + final Map<String, String> signingConfigs = await getCodeSigningIdentityDevelopmentTeam(iosApp: app); + expect(signingConfigs, isNull); + }, + overrides: <Type, Generator>{ + ProcessManager: () => mockProcessManager, + Config: () => mockConfig, + AnsiTerminal: () => testTerminal, + }); + + testUsingContext('find-certificate failure', () async { + when(mockProcessManager.runSync(<String>['which', 'security'])) + .thenReturn(exitsHappy); + when(mockProcessManager.runSync(<String>['which', 'openssl'])) + .thenReturn(exitsHappy); + when(mockProcessManager.runSync( + argThat(contains('find-identity')), + environment: anyNamed('environment'), + workingDirectory: anyNamed('workingDirectory'), + )).thenReturn(ProcessResult( + 1, // pid + 0, // exitCode + ''' +1) 86f7e437faa5a7fce15d1ddcb9eaeaea377667b8 "iPhone Developer: Profile 1 (1111AAAA11)" +2) da4b9237bacccdf19c0760cab7aec4a8359010b0 "iPhone Developer: Profile 2 (2222BBBB22)" +3) 5bf1fd927dfb8679496a2e6cf00cbe50c1c87145 "iPhone Developer: Profile 3 (3333CCCC33)" + 3 valid identities found''', + '', + )); + mockTerminalStdInStream = + Stream<String>.fromFuture(Future<String>.value('3')); + when(mockProcessManager.runSync( + <String>['security', 'find-certificate', '-c', '3333CCCC33', '-p'], + environment: anyNamed('environment'), + workingDirectory: anyNamed('workingDirectory'), + )).thenReturn(ProcessResult(1, 1, '', '' )); + + final Map<String, String> signingConfigs = await getCodeSigningIdentityDevelopmentTeam(iosApp: app); + expect(signingConfigs, isNull); + }, + overrides: <Type, Generator>{ + ProcessManager: () => mockProcessManager, + Config: () => mockConfig, + AnsiTerminal: () => testTerminal, + }); }); }
diff --git a/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart b/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart index 9026ec0..b560538 100644 --- a/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart +++ b/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart
@@ -143,6 +143,12 @@ .thenReturn(ProcessResult(1, 0, 'Xcode 8.3.3\nBuild version 8E3004b', '')); expect(xcodeProjectInterpreter.isInstalled, isTrue); }); + + testUsingOsxContext('build settings is empty when xcodebuild failed to get the build settings', () { + when(mockProcessManager.runSync(argThat(contains(xcodebuild)))) + .thenReturn(ProcessResult(0, 1, '', '')); + expect(xcodeProjectInterpreter.getBuildSettings('', ''), const <String, String>{}); + }); }); group('Xcode project properties', () { test('properties from default project can be parsed', () {