[flutter_tool] Handle crashes from doctor validators (#38920)

packages/flutter_tools/lib/src/doctor.dart

@@ -7,6 +7,7 @@ import 'dart:async';
 import 'android/android_studio_validator.dart';
 import 'android/android_workflow.dart';
 import 'artifacts.dart';
+import 'base/async_guard.dart';
 import 'base/common.dart';
 import 'base/context.dart';
 import 'base/file_system.dart';
@@ -19,6 +20,7 @@ import 'base/user_messages.dart';
 import 'base/utils.dart';
 import 'base/version.dart';
 import 'cache.dart';
+import 'commands/doctor.dart';
 import 'device.dart';
 import 'fuchsia/fuchsia_workflow.dart';
 import 'globals.dart';
@@ -32,6 +34,7 @@ import 'macos/macos_workflow.dart';
 import 'macos/xcode_validator.dart';
 import 'proxy_validator.dart';
 import 'reporting/reporting.dart';
+import 'runner/flutter_command.dart';
 import 'tester/flutter_tester.dart';
 import 'version.dart';
 import 'vscode/vscode_validator.dart';
@@ -58,19 +61,29 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider {
 
   @override
   List<DoctorValidator> get validators {
-    if (_validators == null) {
+    if (_validators != null) {
+      return _validators;
+    }
+
     final List<DoctorValidator> ideValidators = <DoctorValidator>[
       ...AndroidStudioValidator.allValidators,
       ...IntelliJValidator.installedValidators,
       ...VsCodeValidator.installedValidators,
     ];
 
+    bool verbose = false;
+    if (FlutterCommand.current is DoctorCommand) {
+      verbose = (FlutterCommand.current as DoctorCommand).verbose;
+    }
+
     _validators = <DoctorValidator>[
       FlutterValidator(),
       if (androidWorkflow.appliesToHostPlatform)
-          GroupedValidator(<DoctorValidator>[androidValidator, androidLicenseValidator]),
+        GroupedValidator(<DoctorValidator>[androidValidator, androidLicenseValidator],
+          verbose: verbose),
       if (iosWorkflow.appliesToHostPlatform || macOSWorkflow.appliesToHostPlatform)
-          GroupedValidator(<DoctorValidator>[xcodeValidator, cocoapodsValidator]),
+        GroupedValidator(<DoctorValidator>[xcodeValidator, cocoapodsValidator],
+          verbose: verbose),
       if (webWorkflow.appliesToHostPlatform)
         const WebValidator(),
       if (linuxWorkflow.appliesToHostPlatform)
@@ -86,7 +99,6 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider {
       if (deviceManager.canListAnything)
         DeviceValidator(),
     ];
-    }
     return _validators;
   }
 
@@ -137,7 +149,9 @@ class Doctor {
   List<ValidatorTask> startValidatorTasks() {
     final List<ValidatorTask> tasks = <ValidatorTask>[];
     for (DoctorValidator validator in validators) {
-      tasks.add(ValidatorTask(validator, validator.validate()));
+      final Future<ValidationResult> result =
+          asyncGuard<ValidationResult>(() => validator.validate());
+      tasks.add(ValidatorTask(validator, result));
     }
     return tasks;
   }
@@ -148,44 +162,62 @@ class Doctor {
 
   /// Print a summary of the state of the tooling, as well as how to get more info.
   Future<void> summary() async {
-    printStatus(await summaryText);
+    printStatus(await _summaryText());
   }
 
-  Future<String> get summaryText async {
+  Future<String> _summaryText() async {
     final StringBuffer buffer = StringBuffer();
 
-    bool allGood = true;
+    bool missingComponent = false;
+    bool sawACrash = false;
 
     for (DoctorValidator validator in validators) {
       final StringBuffer lineBuffer = StringBuffer();
-      final ValidationResult result = await validator.validate();
-      lineBuffer.write('${result.coloredLeadingBox} ${validator.title} is ');
+      ValidationResult result;
+      try {
+        result = await asyncGuard<ValidationResult>(() => validator.validate());
+      } catch (exception) {
+        // We're generating a summary, so drop the stack trace.
+        result = ValidationResult.crash(exception);
+      }
+      lineBuffer.write('${result.coloredLeadingBox} ${validator.title}: ');
       switch (result.type) {
+        case ValidationType.crash:
+          lineBuffer.write('the doctor check crashed without a result.');
+          sawACrash = true;
+          break;
         case ValidationType.missing:
-          lineBuffer.write('not installed.');
+          lineBuffer.write('is not installed.');
           break;
         case ValidationType.partial:
-          lineBuffer.write('partially installed; more components are available.');
+          lineBuffer.write('is partially installed; more components are available.');
           break;
         case ValidationType.notAvailable:
-          lineBuffer.write('not available.');
+          lineBuffer.write('is not available.');
           break;
         case ValidationType.installed:
-          lineBuffer.write('fully installed.');
+          lineBuffer.write('is fully installed.');
           break;
       }
 
-      if (result.statusInfo != null)
+      if (result.statusInfo != null) {
         lineBuffer.write(' (${result.statusInfo})');
+      }
 
       buffer.write(wrapText(lineBuffer.toString(), hangingIndent: result.leadingBox.length + 1));
       buffer.writeln();
 
-      if (result.type != ValidationType.installed)
-        allGood = false;
+      if (result.type != ValidationType.installed) {
+        missingComponent = true;
+      }
     }
 
-    if (!allGood) {
+    if (sawACrash) {
+      buffer.writeln();
+      buffer.writeln('Run "flutter doctor" for information about why a doctor check crashed.');
+    }
+
+    if (missingComponent) {
       buffer.writeln();
       buffer.writeln('Run "flutter doctor" for information about installing additional components.');
     }
@@ -217,13 +249,19 @@ class Doctor {
       ValidationResult result;
       try {
         result = await validatorTask.result;
-      } catch (exception) {
-        status.cancel();
-        rethrow;
-      }
+      } catch (exception, stackTrace) {
+        // Only include the stacktrace in verbose mode.
+        result = ValidationResult.crash(
+            exception, stackTrace: verbose ? stackTrace : null);
+      } finally {
         status.stop();
+      }
 
       switch (result.type) {
+        case ValidationType.crash:
+          doctorResult = false;
+          issues += 1;
+          break;
         case ValidationType.missing:
           doctorResult = false;
           issues += 1;
@@ -258,13 +296,15 @@ class Doctor {
           }
         }
       }
-      if (verbose)
+      if (verbose) {
         printStatus('');
       }
+    }
 
     // Make sure there's always one line before the summary even when not verbose.
-    if (!verbose)
+    if (!verbose) {
       printStatus('');
+    }
 
     if (issues > 0) {
       printStatus('${terminal.color('!', TerminalColor.yellow)} Doctor found issues in $issues categor${issues > 1 ? "ies" : "y"}.', hangingIndent: 2);
@@ -302,6 +342,7 @@ abstract class Workflow {
 }
 
 enum ValidationType {
+  crash,
   missing,
   partial,
   notAvailable,
@@ -330,9 +371,12 @@ abstract class DoctorValidator {
 /// passed to the constructor and reports the statusInfo of the first validator
 /// that provides one. Other titles and statusInfo strings are discarded.
 class GroupedValidator extends DoctorValidator {
-  GroupedValidator(this.subValidators) : super(subValidators[0].title);
+  GroupedValidator(this.subValidators, {
+    this.verbose = false,
+  }) : super(subValidators[0].title);
 
   final List<DoctorValidator> subValidators;
+  final bool verbose;
 
   List<ValidationResult> _subResults;
 
@@ -351,13 +395,20 @@ class GroupedValidator extends DoctorValidator {
   Future<ValidationResult> validate() async {
     final List<ValidatorTask> tasks = <ValidatorTask>[];
     for (DoctorValidator validator in subValidators) {
-      tasks.add(ValidatorTask(validator, validator.validate()));
+      final Future<ValidationResult> result =
+          asyncGuard<ValidationResult>(() => validator.validate());
+      tasks.add(ValidatorTask(validator, result));
     }
 
     final List<ValidationResult> results = <ValidationResult>[];
     for (ValidatorTask subValidator in tasks) {
       _currentSlowWarning = subValidator.validator.slowWarning;
+      try {
         results.add(await subValidator.result);
+      } catch (exception, stackTrace) {
+        results.add(ValidationResult.crash(
+            exception, stackTrace: verbose ? stackTrace : null));
+      }
     }
     _currentSlowWarning = 'Merging results...';
     return _mergeValidationResults(results);
@@ -382,6 +433,7 @@ class GroupedValidator extends DoctorValidator {
         case ValidationType.partial:
           mergedType = ValidationType.partial;
           break;
+        case ValidationType.crash:
         case ValidationType.missing:
           if (mergedType == ValidationType.installed) {
             mergedType = ValidationType.partial;
@@ -403,6 +455,19 @@ class ValidationResult {
   /// if no [messages] are hints or errors.
   ValidationResult(this.type, this.messages, { this.statusInfo });
 
+  factory ValidationResult.crash(Object error, { StackTrace stackTrace }) {
+    return ValidationResult(ValidationType.crash, <ValidationMessage>[
+      ValidationMessage.error(
+          'Due to an error, the doctor check did not complete. '
+          'If the error message below is not helpful, '
+          'please let us know about this issue at https://github.com/flutter/flutter/issues.'),
+      if (stackTrace == null)
+          ValidationMessage.error('$error'),
+      if (stackTrace != null)
+          ValidationMessage.error('$error:\n$stackTrace'),
+    ], statusInfo: 'the doctor check crashed');
+  }
+
   final ValidationType type;
   // A short message about the status.
   final String statusInfo;
@@ -411,6 +476,8 @@ class ValidationResult {
   String get leadingBox {
     assert(type != null);
     switch (type) {
+      case ValidationType.crash:
+        return '[☠]';
       case ValidationType.missing:
         return '[✗]';
       case ValidationType.installed:
@@ -425,6 +492,8 @@ class ValidationResult {
   String get coloredLeadingBox {
     assert(type != null);
     switch (type) {
+      case ValidationType.crash:
+        return terminal.color(leadingBox, TerminalColor.red);
       case ValidationType.missing:
         return terminal.color(leadingBox, TerminalColor.red);
       case ValidationType.installed:
@@ -440,6 +509,8 @@ class ValidationResult {
   String get typeStr {
     assert(type != null);
     switch (type) {
+      case ValidationType.crash:
+        return 'crash';
       case ValidationType.missing:
         return 'missing';
       case ValidationType.installed:

packages/flutter_tools/test/general.shard/commands/doctor_test.dart

@@ -308,6 +308,23 @@ void main() {
       ));
     }, overrides: noColorTerminalOverride);
 
+    testUsingContext('validate non-verbose output format for run with crash', () async {
+      expect(await FakeCrashingDoctor().diagnose(verbose: false), isFalse);
+      expect(testLogger.statusText, equals(
+              'Doctor summary (to see all details, run flutter doctor -v):\n'
+              '[✓] Passing Validator (with statusInfo)\n'
+              '[✓] Another Passing Validator (with statusInfo)\n'
+              '[☠] Crashing validator (the doctor check crashed)\n'
+              '    ✗ Due to an error, the doctor check did not complete. If the error message below is not helpful, '
+              'please let us know about this issue at https://github.com/flutter/flutter/issues.\n'
+              '    ✗ fatal error\n'
+              '[✓] Validators are fun (with statusInfo)\n'
+              '[✓] Four score and seven validators ago (with statusInfo)\n'
+              '\n'
+              '! Doctor found issues in 1 category.\n'
+      ));
+    }, overrides: noColorTerminalOverride);
+
     testUsingContext('validate non-verbose output format when only one category fails', () async {
       expect(await FakeSinglePassingDoctor().diagnose(verbose: false), isTrue);
       expect(testLogger.statusText, equals(
@@ -647,6 +664,15 @@ class PartialValidatorWithHintsOnly extends DoctorValidator {
   }
 }
 
+class CrashingValidator extends DoctorValidator {
+  CrashingValidator() : super('Crashing validator');
+
+  @override
+  Future<ValidationResult> validate() async {
+    throw 'fatal error';
+  }
+}
+
 /// A doctor that fails with a missing [ValidationResult].
 class FakeDoctor extends Doctor {
   List<DoctorValidator> _validators;
@@ -711,6 +737,23 @@ class FakeQuietDoctor extends Doctor {
   }
 }
 
+/// A doctor with a validator that throws an exception.
+class FakeCrashingDoctor extends Doctor {
+  List<DoctorValidator> _validators;
+  @override
+  List<DoctorValidator> get validators {
+    if (_validators == null) {
+      _validators = <DoctorValidator>[];
+      _validators.add(PassingValidator('Passing Validator'));
+      _validators.add(PassingValidator('Another Passing Validator'));
+      _validators.add(CrashingValidator());
+      _validators.add(PassingValidator('Validators are fun'));
+      _validators.add(PassingValidator('Four score and seven validators ago'));
+    }
+    return _validators;
+  }
+}
+
 /// A DoctorValidatorsProvider that overrides the default validators without
 /// overriding the doctor.
 class FakeDoctorValidatorsProvider implements DoctorValidatorsProvider {