[flutter_tools] Allow the tool to suppress compilation errors. (#58539)

Suppress compilation errors on startup so they are not duplicated from the native build step.

packages/flutter_tools/lib/src/build_system/targets/dart.dart

@@ -258,7 +258,7 @@ class KernelSnapshot extends Target {
       packageConfig: packageConfig,
     );
     if (output == null || output.errorCount != 0) {
-      throw Exception('Errors during snapshot creation: $output');
+      throw Exception();
     }
   }
 }

packages/flutter_tools/lib/src/codegen.dart

@@ -186,6 +186,7 @@ class CodeGeneratingResidentCompiler implements ResidentCompiler {
     List<Uri> invalidatedFiles, {
       String outputPath,
       PackageConfig packageConfig,
+      bool suppressErrors = false,
     }) async {
     if (_codegenDaemon.lastStatus != CodegenStatus.Succeeded && _codegenDaemon.lastStatus != CodegenStatus.Failed) {
       await _codegenDaemon.buildResults.firstWhere((CodegenStatus status) {
@@ -200,6 +201,7 @@ class CodeGeneratingResidentCompiler implements ResidentCompiler {
       invalidatedFiles,
       outputPath: outputPath,
       packageConfig: packageConfig,
+      suppressErrors: suppressErrors,
     );
   }
 

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

@@ -226,13 +226,13 @@ class AssembleCommand extends FlutterCommand {
       );
     if (!result.success) {
       for (final ExceptionMeasurement measurement in result.exceptions.values) {
-        globals.printError('Target ${measurement.target} failed: ${measurement.exception}',
-          stackTrace: measurement.fatal
-            ? measurement.stackTrace
-            : null,
-        );
+        if (measurement.fatal || globals.logger.isVerbose) {
+          globals.printError('Target ${measurement.target} failed: ${measurement.exception}',
+            stackTrace: measurement.stackTrace
+          );
+        }
       }
-      throwToolExit('build failed.');
+      throwToolExit('');
     }
     globals.printTrace('build succeeded.');
     if (argResults.wasParsed('build-inputs')) {

packages/flutter_tools/lib/src/compile.dart

@@ -92,7 +92,6 @@ class StdoutHandler {
     reset();
   }
 
-  bool compilerMessageReceived = false;
   final CompilerMessageConsumer consumer;
   String boundaryKey;
   StdoutState state = StdoutState.CollectDiagnostic;
@@ -101,44 +100,13 @@ class StdoutHandler {
 
   bool _suppressCompilerMessages;
   bool _expectSources;
-  bool _badState = false;
 
   void handler(String message) {
-    if (_badState) {
-      return;
-    }
     const String kResultPrefix = 'result ';
     if (boundaryKey == null && message.startsWith(kResultPrefix)) {
       boundaryKey = message.substring(kResultPrefix.length);
       return;
     }
-    // Invalid state, see commented issue below for more information.
-    // NB: both the completeError and _badState flags are required to avoid
-    // filling the console with exceptions.
-    if (boundaryKey == null) {
-      // Throwing a synchronous exception via throwToolExit will fail to cancel
-      // the stream. Instead use completeError so that the error is returned
-      // from the awaited future that the compiler consumers are expecting.
-      compilerOutput.completeError(ToolExit(
-        'The Dart compiler encountered an internal problem. '
-        'The Flutter team would greatly appreciate if you could leave a '
-        'comment on the issue https://github.com/flutter/flutter/issues/35924 '
-        'describing what you were doing when the crash happened.\n\n'
-        'Additional debugging information:\n'
-        '  StdoutState: $state\n'
-        '  compilerMessageReceived: $compilerMessageReceived\n'
-        '  _expectSources: $_expectSources\n'
-        '  sources: $sources\n'
-      ));
-      // There are several event turns before the tool actually exits from a
-      // tool exception. Normally, the stream should be cancelled to prevent
-      // more events from entering the bad state, but because the error
-      // is coming from handler itself, there is no clean way to pipe this
-      // through. Instead, we set a flag to prevent more messages from
-      // registering.
-      _badState = true;
-      return;
-    }
     if (message.startsWith(boundaryKey)) {
       if (_expectSources) {
         if (state == StdoutState.CollectDiagnostic) {
@@ -160,10 +128,6 @@ class StdoutHandler {
     }
     if (state == StdoutState.CollectDiagnostic) {
       if (!_suppressCompilerMessages) {
-        if (compilerMessageReceived == false) {
-          consumer('\nCompiler message:');
-          compilerMessageReceived = true;
-        }
         consumer(message);
       }
     } else {
@@ -185,7 +149,6 @@ class StdoutHandler {
   // with its own boundary key and new completer.
   void reset({ bool suppressCompilerMessages = false, bool expectSources = true }) {
     boundaryKey = null;
-    compilerMessageReceived = false;
     compilerOutput = Completer<CompilerOutput>();
     _suppressCompilerMessages = suppressCompilerMessages;
     _expectSources = expectSources;
@@ -349,12 +312,14 @@ class _RecompileRequest extends _CompilationRequest {
     this.invalidatedFiles,
     this.outputPath,
     this.packageConfig,
+    this.suppressErrors,
   ) : super(completer);
 
   Uri mainUri;
   List<Uri> invalidatedFiles;
   String outputPath;
   PackageConfig packageConfig;
+  bool suppressErrors;
 
   @override
   Future<CompilerOutput> _run(DefaultResidentCompiler compiler) async =>
@@ -456,6 +421,7 @@ abstract class ResidentCompiler {
     List<Uri> invalidatedFiles, {
     @required String outputPath,
     @required PackageConfig packageConfig,
+    bool suppressErrors = false,
   });
 
   Future<CompilerOutput> compileExpression(
@@ -577,6 +543,7 @@ class DefaultResidentCompiler implements ResidentCompiler {
     List<Uri> invalidatedFiles, {
     @required String outputPath,
     @required PackageConfig packageConfig,
+    bool suppressErrors = false,
   }) async {
     assert(outputPath != null);
     if (!_controller.hasListener) {
@@ -585,7 +552,7 @@ class DefaultResidentCompiler implements ResidentCompiler {
 
     final Completer<CompilerOutput> completer = Completer<CompilerOutput>();
     _controller.add(
-      _RecompileRequest(completer, mainUri, invalidatedFiles, outputPath, packageConfig)
+      _RecompileRequest(completer, mainUri, invalidatedFiles, outputPath, packageConfig, suppressErrors)
     );
     return completer.future;
   }
@@ -593,6 +560,7 @@ class DefaultResidentCompiler implements ResidentCompiler {
   Future<CompilerOutput> _recompile(_RecompileRequest request) async {
     _stdoutHandler.reset();
     _compileRequestNeedsConfirmation = true;
+    _stdoutHandler._suppressCompilerMessages = request.suppressErrors;
 
     if (_server == null) {
       return _compile(
@@ -714,7 +682,7 @@ class DefaultResidentCompiler implements ResidentCompiler {
     _server.stderr
       .transform<String>(utf8.decoder)
       .transform<String>(const LineSplitter())
-      .listen((String message) { globals.printError(message); });
+      .listen(globals.printError);
 
     unawaited(_server.exitCode.then((int code) {
       if (code != 0) {

packages/flutter_tools/lib/src/run_hot.dart

@@ -372,6 +372,11 @@ class HotRunner extends ResidentRunner {
           device.generator.recompile(
             globals.fs.file(mainPath).uri,
             <Uri>[],
+            // When running without a provided applicationBinary, the tool will
+            // simultaneously run the initial frontend_server compilation and
+            // the native build step. If there is a Dart compilation error, it
+            // should only be displayed once.
+            suppressErrors: applicationBinary == null,
             outputPath: dillOutputPath ??
               getDefaultApplicationKernelPath(trackWidgetCreation: debuggingOptions.buildInfo.trackWidgetCreation),
             packageConfig: packageConfig,

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

@@ -96,7 +96,7 @@ void main() {
 
     await expectLater(commandRunner.run(<String>['assemble', '-o Output', 'debug_macos_bundle_flutter_assets']),
       throwsToolExit());
-    expect(testLogger.errorText, contains('bar'));
+    expect(testLogger.errorText, isNot(contains('bar')));
     expect(testLogger.errorText, isNot(contains(testStackTrace.toString())));
   });
 

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

@@ -64,7 +64,7 @@ void main() {
     );
 
     expect(mockFrontendServerStdIn.getAndClear(), isEmpty);
-    expect(testLogger.errorText, equals('\nCompiler message:\nline1\nline2\n'));
+    expect(testLogger.errorText, equals('line1\nline2\n'));
     expect(output.outputFilename, equals('/path/to/main.dart.dill'));
     final VerificationResult argVerification = verify(mockProcessManager.start(captureAny));
     expect(argVerification.captured.single, containsAll(<String>[
@@ -163,7 +163,7 @@ void main() {
     );
 
     expect(mockFrontendServerStdIn.getAndClear(), isEmpty);
-    expect(testLogger.errorText, equals('\nCompiler message:\nline1\nline2\n'));
+    expect(testLogger.errorText, equals('line1\nline2\n'));
     expect(output, equals(null));
   }, overrides: <Type, Generator>{
     ProcessManager: () => mockProcessManager,
@@ -191,7 +191,7 @@ void main() {
       packagesPath: '.packages',
     );
     expect(mockFrontendServerStdIn.getAndClear(), isEmpty);
-    expect(testLogger.errorText, equals('\nCompiler message:\nline1\nline2\n'));
+    expect(testLogger.errorText, equals('line1\nline2\n'));
     expect(output, equals(null));
   }, overrides: <Type, Generator>{
     ProcessManager: () => mockProcessManager,

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

@@ -85,7 +85,7 @@ void main() {
           'compile file:///path/to/main.dart\n');
       verifyNoMoreInteractions(mockFrontendServerStdIn);
       expect(testLogger.errorText,
-          equals('\nCompiler message:\nline1\nline2\n'));
+          equals('line1\nline2\n'));
       expect(output.outputFilename, equals('/path/to/main.dart.dill'));
 
       compileExpressionResponseCompleter.complete(
@@ -131,7 +131,7 @@ void main() {
         packageConfig: PackageConfig.empty,
       ).then((CompilerOutput outputCompile) {
         expect(testLogger.errorText,
-            equals('\nCompiler message:\nline1\nline2\n'));
+            equals('line1\nline2\n'));
         expect(outputCompile.outputFilename, equals('/path/to/main.dart.dill'));
 
         compileExpressionResponseCompleter1.complete(Future<List<int>>.value(utf8.encode(

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

@@ -66,7 +66,7 @@ void main() {
     );
     expect(mockFrontendServerStdIn.getAndClear(), 'compile /path/to/main.dart\n');
     verifyNoMoreInteractions(mockFrontendServerStdIn);
-    expect(testLogger.errorText, equals('\nCompiler message:\nline1\nline2\n'));
+    expect(testLogger.errorText, equals('line1\nline2\n'));
     expect(output.outputFilename, equals('/path/to/main.dart.dill'));
   }, overrides: <Type, Generator>{
     ProcessManager: () => mockProcessManager,
@@ -141,9 +141,9 @@ void main() {
     verifyNoMoreInteractions(mockFrontendServerStdIn);
     expect(mockFrontendServerStdIn.getAndClear(), isEmpty);
     expect(testLogger.errorText, equals(
-      '\nCompiler message:\nline0\nline1\n'
-      '\nCompiler message:\nline1\nline2\n'
-      '\nCompiler message:\nline1\nline2\n'
+      'line0\nline1\n'
+      'line1\nline2\n'
+      'line1\nline2\n'
     ));
   }, overrides: <Type, Generator>{
     ProcessManager: () => mockProcessManager,
@@ -151,6 +151,44 @@ void main() {
     Platform: kNoColorTerminalPlatform,
   });
 
+  testUsingContext('incremental compile can suppress errors', () async {
+    final StreamController<List<int>> stdoutController = StreamController<List<int>>();
+    when(mockFrontendServer.stdout)
+      .thenAnswer((Invocation invocation) => stdoutController.stream);
+
+    stdoutController.add(utf8.encode('result abc\nline0\nline1\nabc\nabc /path/to/main.dart.dill 0\n'));
+
+    await generator.recompile(
+      Uri.parse('/path/to/main.dart'),
+      <Uri>[],
+      outputPath: '/build/',
+      packageConfig: PackageConfig.empty,
+    );
+    expect(mockFrontendServerStdIn.getAndClear(), 'compile /path/to/main.dart\n');
+
+    await _recompile(stdoutController, generator, mockFrontendServerStdIn,
+      'result abc\nline1\nline2\nabc\nabc /path/to/main.dart.dill 0\n');
+
+    await _accept(stdoutController, generator, mockFrontendServerStdIn, r'^accept\n$');
+
+    await _recompile(stdoutController, generator, mockFrontendServerStdIn,
+      'result abc\nline1\nline2\nabc\nabc /path/to/main.dart.dill 0\n', suppressErrors: true);
+
+    verifyNoMoreInteractions(mockFrontendServerStdIn);
+    expect(mockFrontendServerStdIn.getAndClear(), isEmpty);
+
+    // Compiler message is not printed with suppressErrors: true above.
+    expect(testLogger.errorText, isNot(equals(
+      'line0\nline1\n'
+      'line1\nline2\n'
+      'line1\nline2\n'
+    )));
+  }, overrides: <Type, Generator>{
+    ProcessManager: () => mockProcessManager,
+    OutputPreferences: () => OutputPreferences(showColor: false),
+    Platform: kNoColorTerminalPlatform,
+  });
+
   testUsingContext('incremental compile and recompile twice', () async {
     final StreamController<List<int>> streamController = StreamController<List<int>>();
     when(mockFrontendServer.stdout)
@@ -174,9 +212,9 @@ void main() {
     verifyNoMoreInteractions(mockFrontendServerStdIn);
     expect(mockFrontendServerStdIn.getAndClear(), isEmpty);
     expect(testLogger.errorText, equals(
-      '\nCompiler message:\nline0\nline1\n'
-      '\nCompiler message:\nline1\nline2\n'
-      '\nCompiler message:\nline2\nline3\n'
+      'line0\nline1\n'
+      'line1\nline2\n'
+      'line2\nline3\n'
     ));
   }, overrides: <Type, Generator>{
     ProcessManager: () => mockProcessManager,
@@ -190,6 +228,7 @@ Future<void> _recompile(
   ResidentCompiler generator,
   MockStdIn mockFrontendServerStdIn,
   String mockCompilerOutput,
+  { bool suppressErrors = false }
 ) async {
   // Put content into the output stream after generator.recompile gets
   // going few lines below, resets completer.
@@ -201,6 +240,7 @@ Future<void> _recompile(
     <Uri>[Uri.parse('/path/to/main.dart')],
     outputPath: '/build/',
     packageConfig: PackageConfig.empty,
+    suppressErrors: suppressErrors,
   );
   expect(output.outputFilename, equals('/path/to/main.dart.dill'));
   final String commands = mockFrontendServerStdIn.getAndClear();

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

@@ -19,14 +19,6 @@ void main() {
     expect(output.outputFilename, 'message');
   });
 
-  testUsingContext('StdOutHandler crash test', () async {
-    final StdoutHandler stdoutHandler = StdoutHandler();
-    final Future<CompilerOutput> output = stdoutHandler.compilerOutput.future;
-    stdoutHandler.handler('message with no result');
-
-    expect(output, throwsToolExit());
-  });
-
   test('TargetModel values', () {
     expect(TargetModel('vm'), TargetModel.vm);
     expect(TargetModel.vm.toString(), 'vm');

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

@@ -207,6 +207,93 @@ void main() {
     expect(fakeVmServiceHost.hasRemainingExpectations, false);
   }));
 
+  test('ResidentRunner suppresses errors for the initial compilation', () => testbed.run(() async {
+    globals.fs.file(globals.fs.path.join('lib', 'main.dart'))
+      .createSync(recursive: true);
+    fakeVmServiceHost = FakeVmServiceHost(requests: <VmServiceExpectation>[
+      listViews,
+      listViews,
+    ]);
+    final MockResidentCompiler residentCompiler = MockResidentCompiler();
+    residentRunner = HotRunner(
+      <FlutterDevice>[
+        mockFlutterDevice,
+      ],
+      stayResident: false,
+      debuggingOptions: DebuggingOptions.enabled(BuildInfo.debug),
+    );
+    when(mockFlutterDevice.generator).thenReturn(residentCompiler);
+    when(residentCompiler.recompile(
+      any,
+      any,
+      outputPath: anyNamed('outputPath'),
+      packageConfig: anyNamed('packageConfig'),
+      suppressErrors: true,
+    )).thenAnswer((Invocation invocation) async {
+      return const CompilerOutput('foo', 0 ,<Uri>[]);
+    });
+    when(mockFlutterDevice.runHot(
+      hotRunner: anyNamed('hotRunner'),
+      route: anyNamed('route'),
+    )).thenAnswer((Invocation invocation) async {
+      return 0;
+    });
+
+    expect(await residentRunner.run(), 0);
+    verify(residentCompiler.recompile(
+      any,
+      any,
+      outputPath: anyNamed('outputPath'),
+      packageConfig: anyNamed('packageConfig'),
+      suppressErrors: true,
+    )).called(1);
+    expect(fakeVmServiceHost.hasRemainingExpectations, false);
+  }));
+
+  test('ResidentRunner does not suppressErrors if running with an applicationBinary', () => testbed.run(() async {
+    globals.fs.file(globals.fs.path.join('lib', 'main.dart'))
+      .createSync(recursive: true);
+    fakeVmServiceHost = FakeVmServiceHost(requests: <VmServiceExpectation>[
+      listViews,
+      listViews,
+    ]);
+    final MockResidentCompiler residentCompiler = MockResidentCompiler();
+    residentRunner = HotRunner(
+      <FlutterDevice>[
+        mockFlutterDevice,
+      ],
+      applicationBinary: globals.fs.file('app.apk'),
+      stayResident: false,
+      debuggingOptions: DebuggingOptions.enabled(BuildInfo.debug),
+    );
+    when(mockFlutterDevice.generator).thenReturn(residentCompiler);
+    when(residentCompiler.recompile(
+      any,
+      any,
+      outputPath: anyNamed('outputPath'),
+      packageConfig: anyNamed('packageConfig'),
+      suppressErrors: false,
+    )).thenAnswer((Invocation invocation) async {
+      return const CompilerOutput('foo', 0, <Uri>[]);
+    });
+    when(mockFlutterDevice.runHot(
+      hotRunner: anyNamed('hotRunner'),
+      route: anyNamed('route'),
+    )).thenAnswer((Invocation invocation) async {
+      return 0;
+    });
+
+    expect(await residentRunner.run(), 0);
+    verify(residentCompiler.recompile(
+      any,
+      any,
+      outputPath: anyNamed('outputPath'),
+      packageConfig: anyNamed('packageConfig'),
+      suppressErrors: false,
+    )).called(1);
+    expect(fakeVmServiceHost.hasRemainingExpectations, false);
+  }));
+
   test('ResidentRunner can attach to device successfully with --fast-start', () => testbed.run(() async {
     fakeVmServiceHost = FakeVmServiceHost(requests: <VmServiceExpectation>[
       listViews,
@@ -1215,6 +1302,7 @@ class MockDeviceLogReader extends Mock implements DeviceLogReader {}
 class MockDevicePortForwarder extends Mock implements DevicePortForwarder {}
 class MockUsage extends Mock implements Usage {}
 class MockProcessManager extends Mock implements ProcessManager {}
+class MockResidentCompiler extends Mock implements ResidentCompiler {}
 
 class TestFlutterDevice extends FlutterDevice {
   TestFlutterDevice(Device device, { Stream<Uri> observatoryUris })

packages/flutter_tools/test/src/mocks.dart

@@ -638,7 +638,11 @@ class MockResidentCompiler extends BasicMock implements ResidentCompiler {
   }
 
   @override
-  Future<CompilerOutput> recompile(Uri mainPath, List<Uri> invalidatedFiles, { String outputPath, PackageConfig packageConfig }) async {
+  Future<CompilerOutput> recompile(Uri mainPath, List<Uri> invalidatedFiles, {
+    String outputPath,
+    PackageConfig packageConfig,
+    bool suppressErrors = false,
+  }) async {
     globals.fs.file(outputPath).createSync(recursive: true);
     globals.fs.file(outputPath).writeAsStringSync('compiled_kernel_output');
     return CompilerOutput(outputPath, 0, <Uri>[]);