[flutter_tools] rethrow process exceptions as tool exit from gradle build (#64745)

The flutter tool is currently unable to detect missing permissions in gradle/gradle.bat that would cause a gradle build to fail via process exception. Rather than crashing and exiting, we can display the exception as an error message and tool exit.

While linux/macOS are able to add the +x bit, this is not possible on windows with our current file system/OS API. These crashes represent a substantial amount of crash reporting, but are otherwise not actionable on our end.

packages/flutter_tools/lib/runner.dart

@@ -28,7 +28,7 @@ import 'src/runner/flutter_command_runner.dart';
 // TODO(jonahwilliams): update command type once g3 has rolled.
 Future<int> run(
   List<String> args,
-  dynamic commands, {
+  List<FlutterCommand> Function() commands, {
     bool muteCommandLogging = false,
     bool verbose = false,
     bool verboseHelp = false,
@@ -42,17 +42,11 @@ Future<int> run(
     args = List<String>.of(args);
     args.removeWhere((String option) => option == '-v' || option == '--verbose');
   }
-  List<FlutterCommand> Function() commandGenerator;
-  if (commands is List<FlutterCommand>) {
-    commandGenerator = () => commands;
-  } else {
-    commandGenerator = commands as List<FlutterCommand> Function();
-  }
 
   return runInContext<int>(() async {
     reportCrashes ??= !await globals.isRunningOnBot;
     final FlutterCommandRunner runner = FlutterCommandRunner(verboseHelp: verboseHelp);
-    commandGenerator().forEach(runner.addCommand);
+    commands().forEach(runner.addCommand);
 
     // Initialize the system locale.
     final String systemLocale = await intl_standalone.findSystemLocale();

packages/flutter_tools/lib/src/android/gradle.dart

@@ -397,7 +397,7 @@ Future<void> buildGradleApp({
       environment: gradleEnvironment,
       mapFunction: consumeLog,
     );
-  } on ProcessException catch(exception) {
+  } on ProcessException catch (exception) {
     consumeLog(exception.toString());
     // Rethrow the exception if the error isn't handled by any of the
     // `localGradleErrors`.

packages/flutter_tools/lib/src/base/error_handling_io.dart

@@ -3,16 +3,17 @@
 // found in the LICENSE file.
 
 import 'dart:convert';
-import 'dart:io' as io show Directory, File, Link;
+import 'dart:io' as io show Directory, File, Link, ProcessException, ProcessResult, ProcessSignal, systemEncoding, Process, ProcessStartMode;
 
 import 'package:file/file.dart';
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p; // ignore: package_path_import
+import 'package:process/process.dart';
 
 import 'common.dart' show throwToolExit;
 import 'platform.dart';
 
-// The Flutter tool hits file system errors that only the end-user can address.
+// The Flutter tool hits file system and process errors that only the end-user can address.
 // We would like these errors to not hit crash logging. In these cases, we
 // should exit gracefully and provide potentially useful advice. For example, if
 // a write fails because the target device is full, we can explain that with a
@@ -355,9 +356,16 @@ Future<T> _run<T>(Future<T> Function() op, {
     return await op();
   } on FileSystemException catch (e) {
     if (platform.isWindows) {
-      _handleWindowsException(e, failureMessage);
+      _handleWindowsException(e, failureMessage, e.osError?.errorCode ?? 0);
     } else if (platform.isLinux) {
-      _handleLinuxException(e, failureMessage);
+      _handleLinuxException(e, failureMessage, e.osError?.errorCode ?? 0);
+    }
+    rethrow;
+  } on io.ProcessException catch (e) {
+    if (platform.isWindows) {
+      _handleWindowsException(e, failureMessage, e.errorCode ?? 0);
+    } else if (platform.isLinux) {
+      _handleLinuxException(e, failureMessage, e.errorCode ?? 0);
     }
     rethrow;
   }
@@ -372,20 +380,121 @@ T _runSync<T>(T Function() op, {
     return op();
   } on FileSystemException catch (e) {
     if (platform.isWindows) {
-      _handleWindowsException(e, failureMessage);
+      _handleWindowsException(e, failureMessage, e.osError?.errorCode ?? 0);
     } else if (platform.isLinux) {
-      _handleLinuxException(e, failureMessage);
+      _handleLinuxException(e, failureMessage, e.osError?.errorCode ?? 0);
+    }
+    rethrow;
+  } on io.ProcessException catch (e) {
+    if (platform.isWindows) {
+      _handleWindowsException(e, failureMessage, e.errorCode ?? 0);
+    } else if (platform.isLinux) {
+      _handleLinuxException(e, failureMessage, e.errorCode ?? 0);
     }
     rethrow;
   }
 }
 
-void _handleLinuxException(FileSystemException e, String message) {
+/// A [ProcessManager] that throws a [ToolExit] on certain errors.
+///
+/// If a [ProcessException] is not caused by the Flutter tool, and can only be
+/// addressed by the user, it should be caught by this [ProcessManager] and thrown
+/// as a [ToolExit] using [throwToolExit].
+///
+/// See also:
+///   * [ErrorHandlngFileSystem], for a similar file system strategy.
+class ErrorHandlingProcessManager extends ProcessManager {
+  ErrorHandlingProcessManager({
+    @required ProcessManager delegate,
+    @required Platform platform,
+  }) : _delegate = delegate,
+       _platform = platform;
+
+  final ProcessManager _delegate;
+  final Platform _platform;
+
+  @override
+  bool canRun(dynamic executable, {String workingDirectory}) {
+    return _runSync(
+      () => _delegate.canRun(executable, workingDirectory: workingDirectory),
+      platform: _platform,
+    );
+  }
+
+  @override
+  bool killPid(int pid, [io.ProcessSignal signal = io.ProcessSignal.sigterm]) {
+    return _runSync(
+      () => _delegate.killPid(pid, signal),
+      platform: _platform,
+    );
+  }
+
+  @override
+  Future<io.ProcessResult> run(
+    List<dynamic> command, {
+    String workingDirectory,
+    Map<String, String> environment,
+    bool includeParentEnvironment = true,
+    bool runInShell = false,
+    Encoding stdoutEncoding = io.systemEncoding,
+    Encoding stderrEncoding = io.systemEncoding,
+  }) {
+    return _run(() => _delegate.run(
+      command,
+      workingDirectory: workingDirectory,
+      environment: environment,
+      includeParentEnvironment: includeParentEnvironment,
+      runInShell: runInShell,
+      stdoutEncoding: stdoutEncoding,
+      stderrEncoding: stderrEncoding,
+    ), platform: _platform);
+  }
+
+  @override
+  Future<io.Process> start(
+    List<dynamic> command, {
+    String workingDirectory,
+    Map<String, String> environment,
+    bool includeParentEnvironment = true,
+    bool runInShell = false,
+    io.ProcessStartMode mode = io.ProcessStartMode.normal,
+  }) {
+    return _run(() => _delegate.start(
+      command,
+      workingDirectory: workingDirectory,
+      environment: environment,
+      includeParentEnvironment: includeParentEnvironment,
+      runInShell: runInShell,
+    ), platform: _platform);
+  }
+
+  @override
+  io.ProcessResult runSync(
+    List<dynamic> command, {
+    String workingDirectory,
+    Map<String, String> environment,
+    bool includeParentEnvironment = true,
+    bool runInShell = false,
+    Encoding stdoutEncoding = io.systemEncoding,
+    Encoding stderrEncoding = io.systemEncoding,
+  }) {
+    return _runSync(() => _delegate.runSync(
+      command,
+      workingDirectory: workingDirectory,
+      environment: environment,
+      includeParentEnvironment: includeParentEnvironment,
+      runInShell: runInShell,
+      stdoutEncoding: stdoutEncoding,
+      stderrEncoding: stderrEncoding,
+    ), platform: _platform);
+  }
+}
+
+void _handleLinuxException(Exception e, String message, int errorCode) {
   // From:
   // https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/errno.h
   // https://github.com/torvalds/linux/blob/master/include/uapi/asm-generic/errno-base.h
   const int enospc = 28;
-  final int errorCode = e.osError?.errorCode ?? 0;
   // Catch errors and bail when:
   switch (errorCode) {
     case enospc:
@@ -401,13 +510,12 @@ void _handleLinuxException(FileSystemException e, String message) {
   }
 }
 
-void _handleWindowsException(FileSystemException e, String message) {
+void _handleWindowsException(Exception e, String message, int errorCode) {
   // From:
   // https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes
   const int kDeviceFull = 112;
   const int kUserMappedSectionOpened = 1224;
   const int kAccessDenied = 5;
-  final int errorCode = e.osError?.errorCode ?? 0;
   // Catch errors and bail when:
   switch (errorCode) {
     case kAccessDenied:

packages/flutter_tools/lib/src/context_runner.dart

@@ -4,6 +4,8 @@
 
 import 'dart:async';
 
+import 'package:process/process.dart';
+
 import 'android/android_sdk.dart';
 import 'android/android_studio.dart';
 import 'android/android_workflow.dart';
@@ -13,6 +15,7 @@ import 'artifacts.dart';
 import 'asset.dart';
 import 'base/config.dart';
 import 'base/context.dart';
+import 'base/error_handling_io.dart';
 import 'base/io.dart';
 import 'base/logger.dart';
 import 'base/os.dart';
@@ -214,6 +217,10 @@ Future<T> runInContext<T>(
         platform: globals.platform,
       ),
       ProcessInfo: () => ProcessInfo(),
+      ProcessManager: () => ErrorHandlingProcessManager(
+        delegate: const LocalProcessManager(),
+        platform: globals.platform,
+      ),
       ProcessUtils: () => ProcessUtils(
         processManager: globals.processManager,
         logger: globals.logger,

packages/flutter_tools/lib/src/convert.dart

@@ -28,7 +28,7 @@ class Utf8Codec extends Encoding {
   String get name => cnv.utf8.name;
 }
 
-Encoding get utf8 => const Utf8Codec();
+const Encoding utf8 = Utf8Codec();
 
 class Utf8Decoder extends cnv.Utf8Decoder {
   const Utf8Decoder({this.reportErrors = true}) : super(allowMalformed: true);

packages/flutter_tools/lib/src/globals.dart

@@ -10,7 +10,7 @@ import 'artifacts.dart';
 import 'base/bot_detector.dart';
 import 'base/config.dart';
 import 'base/context.dart';
-import 'base/error_handling_file_system.dart';
+import 'base/error_handling_io.dart';
 import 'base/file_system.dart';
 import 'base/io.dart';
 import 'base/logger.dart';

packages/flutter_tools/test/general.shard/base/error_handling_io_test.dart

@@ -3,13 +3,15 @@
 // found in the LICENSE file.
 
 import 'package:file/file.dart';
-import 'package:flutter_tools/src/base/error_handling_file_system.dart';
+import 'package:flutter_tools/src/base/error_handling_io.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/io.dart';
 import 'package:flutter_tools/src/base/platform.dart';
 import 'package:mockito/mockito.dart';
 import 'package:path/path.dart' as path; // ignore: package_path_import
 
 import '../../src/common.dart';
+import '../../src/context.dart';
 
 class MockFile extends Mock implements File {}
 class MockFileSystem extends Mock implements FileSystem {}
@@ -191,7 +193,7 @@ void main() {
   });
 
   group('throws ToolExit on Linux', () {
-    const int enospc= 28;
+    const int enospc = 28;
     MockFileSystem mockFileSystem;
     ErrorHandlingFileSystem fs;
 
@@ -291,4 +293,135 @@ void main() {
       expect(fs.file('file').toString(), equals(mockFile.toString()));
     });
   });
+
+  group('ProcessManager on windows throws tool exit', () {
+    const int kDeviceFull = 112;
+    const int kUserMappedSectionOpened = 1224;
+    const int kUserPermissionDenied = 5;
+
+    test('when the device is full', () {
+      final MockProcessManager mockProcessManager = MockProcessManager();
+      final ProcessManager processManager = ErrorHandlingProcessManager(
+        delegate: mockProcessManager,
+        platform: windowsPlatform,
+      );
+      setupProcessManagerMocks(mockProcessManager, kDeviceFull);
+
+      const String expectedMessage = 'The target device is full';
+      expect(() => processManager.canRun('foo'),
+             throwsToolExit(message: expectedMessage));
+      expect(() => processManager.killPid(1),
+             throwsToolExit(message: expectedMessage));
+      expect(() async => await processManager.start(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+      expect(() async => await processManager.run(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+      expect(() => processManager.runSync(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+    });
+
+    test('when the file is being used by another program', () {
+      final MockProcessManager mockProcessManager = MockProcessManager();
+      final ProcessManager processManager = ErrorHandlingProcessManager(
+        delegate: mockProcessManager,
+        platform: windowsPlatform,
+      );
+      setupProcessManagerMocks(mockProcessManager, kUserMappedSectionOpened);
+
+      const String expectedMessage = 'The file is being used by another program';
+      expect(() => processManager.canRun('foo'),
+             throwsToolExit(message: expectedMessage));
+      expect(() => processManager.killPid(1),
+             throwsToolExit(message: expectedMessage));
+      expect(() async => await processManager.start(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+      expect(() async => await processManager.run(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+      expect(() => processManager.runSync(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+    });
+
+    test('when permissions are denied', () {
+      final MockProcessManager mockProcessManager = MockProcessManager();
+      final ProcessManager processManager = ErrorHandlingProcessManager(
+        delegate: mockProcessManager,
+        platform: windowsPlatform,
+      );
+      setupProcessManagerMocks(mockProcessManager, kUserPermissionDenied);
+
+      const String expectedMessage = 'The flutter tool cannot access the file';
+      expect(() => processManager.canRun('foo'),
+             throwsToolExit(message: expectedMessage));
+      expect(() => processManager.killPid(1),
+             throwsToolExit(message: expectedMessage));
+      expect(() async => await processManager.start(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+      expect(() async => await processManager.run(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+      expect(() => processManager.runSync(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+    });
+  });
+
+  group('ProcessManager on linux throws tool exit', () {
+    const int enospc = 28;
+
+    test('when writing to a full device', () {
+      final MockProcessManager mockProcessManager = MockProcessManager();
+      final ProcessManager processManager = ErrorHandlingProcessManager(
+        delegate: mockProcessManager,
+        platform: linuxPlatform,
+      );
+      setupProcessManagerMocks(mockProcessManager, enospc);
+
+      const String expectedMessage = 'The target device is full';
+      expect(() => processManager.canRun('foo'),
+             throwsToolExit(message: expectedMessage));
+      expect(() => processManager.killPid(1),
+             throwsToolExit(message: expectedMessage));
+      expect(() async => await processManager.start(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+      expect(() async => await processManager.run(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+      expect(() => processManager.runSync(<String>['foo']),
+             throwsToolExit(message: expectedMessage));
+    });
+  });
 }
+
+void setupProcessManagerMocks(
+  MockProcessManager processManager,
+  int errorCode,
+) {
+  when(processManager.canRun(any, workingDirectory: anyNamed('workingDirectory')))
+    .thenThrow(ProcessException('', <String>[], '', errorCode));
+  when(processManager.killPid(any, any))
+    .thenThrow(ProcessException('', <String>[], '', errorCode));
+  when(processManager.runSync(
+    any,
+    environment: anyNamed('environment'),
+    includeParentEnvironment: anyNamed('includeParentEnvironment'),
+    runInShell: anyNamed('runInShell'),
+    workingDirectory: anyNamed('workingDirectory'),
+    stdoutEncoding: anyNamed('stdoutEncoding'),
+    stderrEncoding: anyNamed('stderrEncoding'),
+  )).thenThrow(ProcessException('', <String>[], '', errorCode));
+  when(processManager.run(
+    any,
+    environment: anyNamed('environment'),
+    includeParentEnvironment: anyNamed('includeParentEnvironment'),
+    runInShell: anyNamed('runInShell'),
+    workingDirectory: anyNamed('workingDirectory'),
+    stdoutEncoding: anyNamed('stdoutEncoding'),
+    stderrEncoding: anyNamed('stderrEncoding'),
+  )).thenThrow(ProcessException('', <String>[], '', errorCode));
+  when(processManager.start(
+    any,
+    environment: anyNamed('environment'),
+    includeParentEnvironment: anyNamed('includeParentEnvironment'),
+    runInShell: anyNamed('runInShell'),
+    workingDirectory: anyNamed('workingDirectory'),
+  )).thenThrow(ProcessException('', <String>[], '', errorCode));
+}
+
+class MockProcessManager extends Mock implements ProcessManager {}

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

@@ -62,7 +62,7 @@ void main() {
     final List<String> allowedPaths = <String>[
       globals.fs.path.join(flutterTools, 'lib', 'src', 'base', 'io.dart'),
       globals.fs.path.join(flutterTools, 'lib', 'src', 'base', 'platform.dart'),
-      globals.fs.path.join(flutterTools, 'lib', 'src', 'base', 'error_handling_file_system.dart'),
+      globals.fs.path.join(flutterTools, 'lib', 'src', 'base', 'error_handling_io.dart'),
     ];
     bool _isNotAllowed(FileSystemEntity entity) => allowedPaths.every((String path) => path != entity.path);
 
@@ -158,7 +158,7 @@ void main() {
   test('no unauthorized imports of dart:convert', () {
     final List<String> allowedPaths = <String>[
       globals.fs.path.join(flutterTools, 'lib', 'src', 'convert.dart'),
-      globals.fs.path.join(flutterTools, 'lib', 'src', 'base', 'error_handling_file_system.dart'),
+      globals.fs.path.join(flutterTools, 'lib', 'src', 'base', 'error_handling_io.dart'),
     ];
     bool _isNotAllowed(FileSystemEntity entity) => allowedPaths.every((String path) => path != entity.path);
 

packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart

@@ -7,7 +7,7 @@ import 'dart:io' as io;
 
 import 'package:args/command_runner.dart';
 import 'package:flutter_tools/src/base/common.dart';
-import 'package:flutter_tools/src/base/error_handling_file_system.dart';
+import 'package:flutter_tools/src/base/error_handling_io.dart';
 import 'package:flutter_tools/src/base/io.dart';
 import 'package:flutter_tools/src/base/signals.dart';
 import 'package:flutter_tools/src/base/time.dart';

packages/flutter_tools/test/general.shard/runner/runner_test.dart

@@ -108,7 +108,7 @@ void main() {
         () {
           unawaited(runner.run(
             <String>['crash'],
-            <FlutterCommand>[
+            () => <FlutterCommand>[
               CrashingFlutterCommand(asyncCrash: true, completer: commandCompleter),
             ],
             // This flutterVersion disables crash reporting.

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

@@ -9,7 +9,7 @@ import 'package:file/file.dart';
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/context.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
-import 'package:flutter_tools/src/base/error_handling_file_system.dart';
+import 'package:flutter_tools/src/base/error_handling_io.dart';
 import 'package:flutter_tools/src/base/process.dart';
 import 'package:flutter_tools/src/globals.dart' as globals;