diff --git a/dev/bots/analyze.dart b/dev/bots/analyze.dart index 9c707d45b9..304b4068dd 100644 --- a/dev/bots/analyze.dart +++ b/dev/bots/analyze.dart @@ -169,8 +169,9 @@ Future run(List arguments) async { // Try with the --watch analyzer, to make sure it returns success also. // The --benchmark argument exits after one run. + // We specify a failureMessage so that the actual output is muted in the case where _runFlutterAnalyze above already failed. printProgress('Dart analysis (with --watch)...'); - await _runFlutterAnalyze(flutterRoot, options: [ + await _runFlutterAnalyze(flutterRoot, failureMessage: 'Dart analyzer failed when --watch was used.', options: [ '--flutter-repo', '--watch', '--benchmark', @@ -196,7 +197,7 @@ Future run(List arguments) async { ], workingDirectory: flutterRoot, ); - await _runFlutterAnalyze(outDir.path, options: [ + await _runFlutterAnalyze(outDir.path, failureMessage: 'Dart analyzer failed on mega_gallery benchmark.', options: [ '--watch', '--benchmark', ...arguments, @@ -1942,11 +1943,13 @@ Diff at character #$indexOfDifference Future _runFlutterAnalyze(String workingDirectory, { List options = const [], + String? failureMessage, }) async { return runCommand( flutter, ['analyze', ...options], workingDirectory: workingDirectory, + failureMessage: failureMessage, ); } diff --git a/dev/bots/run_command.dart b/dev/bots/run_command.dart index 0a013bed3f..a58d5be9b6 100644 --- a/dev/bots/run_command.dart +++ b/dev/bots/run_command.dart @@ -190,13 +190,24 @@ Future runCommand(String executable, List arguments, { print(result.flattenedStderr); break; } + String allOutput; + if (failureMessage == null) { + allOutput = '${result.flattenedStdout}\n${result.flattenedStderr}'; + if (allOutput.split('\n').length > 10) { + allOutput = '(stdout/stderr output was more than 10 lines)'; + } + } else { + allOutput = ''; + } foundError([ if (failureMessage != null) - failureMessage - else - '$bold${red}Command exited with exit code ${result.exitCode} but expected: ${expectNonZeroExit ? (expectedExitCode ?? 'non-zero') : 'zero'} exit code.$reset', + failureMessage, '${bold}Command: $green$commandDescription$reset', - '${bold}Relative working directory: $cyan$relativeWorkingDir$reset', + if (failureMessage == null) + '$bold${red}Command exited with exit code ${result.exitCode} but expected ${expectNonZeroExit ? (expectedExitCode ?? 'non-zero') : 'zero'} exit code.$reset', + '${bold}Working directory: $cyan${path.absolute(relativeWorkingDir)}$reset', + if (allOutput.isNotEmpty) + '${bold}stdout and stderr output:\n$allOutput', ]); } else { print('ELAPSED TIME: ${prettyPrintDuration(result.elapsedTime)} for $green$commandDescription$reset in $cyan$relativeWorkingDir$reset'); diff --git a/dev/bots/utils.dart b/dev/bots/utils.dart index f94c4757be..91d744d5af 100644 --- a/dev/bots/utils.dart +++ b/dev/bots/utils.dart @@ -82,8 +82,7 @@ VoidCallback? onError; bool get hasError => _hasError; bool _hasError = false; -Iterable get errorMessages => _errorMessages; -List _errorMessages = []; +List> _errorMessages = >[]; final List _pendingLogs = []; Timer? _hideTimer; // When this is null, the output is verbose. @@ -104,7 +103,7 @@ void foundError(List messages) { // another error. _pendingLogs.forEach(_printLoudly); _pendingLogs.clear(); - _errorMessages.addAll(messages); + _errorMessages.add(messages); _hasError = true; if (onError != null) { onError!(); @@ -124,8 +123,18 @@ Never reportErrorsAndExit() { _hideTimer?.cancel(); _hideTimer = null; print(redLine); - print('For your convenience, the error messages reported above are repeated here:'); - _errorMessages.forEach(print); + print('${red}For your convenience, the error messages reported above are repeated here:$reset'); + final bool printSeparators = _errorMessages.any((List messages) => messages.length > 1); + if (printSeparators) { + print(' 🙙 🙛 '); + } + for (int index = 0; index < _errorMessages.length * 2 - 1; index += 1) { + if (index.isEven) { + _errorMessages[index ~/ 2].forEach(print); + } else if (printSeparators) { + print(' 🙙 🙛 '); + } + } print(redLine); system.exit(1); } diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index dccdd2a6a4..78707b7b02 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -1329,9 +1329,25 @@ abstract class ResidentRunner extends ResidentHandlers { void printStructuredErrorLog(vm_service.Event event) { if (event.extensionKind == 'Flutter.Error' && !machine) { - final Map? json = event.extensionData?.data; + final Map? json = event.extensionData?.data; if (json != null && json.containsKey('renderedErrorText')) { - globals.printStatus('\n${json['renderedErrorText']}'); + final int errorsSinceReload; + if (json.containsKey('errorsSinceReload') && json['errorsSinceReload'] is int) { + errorsSinceReload = json['errorsSinceReload']! as int; + } else { + errorsSinceReload = 0; + } + if (errorsSinceReload == 0) { + // We print a blank line around the first error, to more clearly emphasize it + // in the output. (Other errors don't get this.) + globals.printStatus(''); + } + globals.printStatus('${json['renderedErrorText']}'); + if (errorsSinceReload == 0) { + globals.printStatus(''); + } + } else { + globals.printError('Received an invalid ${globals.logger.terminal.bolden("Flutter.Error")} message from app: $json'); } } } diff --git a/packages/flutter_tools/lib/src/vmservice.dart b/packages/flutter_tools/lib/src/vmservice.dart index a6e177a6e2..50d62ff948 100644 --- a/packages/flutter_tools/lib/src/vmservice.dart +++ b/packages/flutter_tools/lib/src/vmservice.dart @@ -285,11 +285,11 @@ Future setUpVmService( } if (printStructuredErrorLogMethod != null) { vmService.onExtensionEvent.listen(printStructuredErrorLogMethod); - // It is safe to ignore this error because we expect an error to be - // thrown if we're already subscribed. registrationRequests.add(vmService .streamListen(vm_service.EventStreams.kExtension) .then((vm_service.Success success) => success) + // It is safe to ignore this error because we expect an error to be + // thrown if we're already subscribed. .catchError((Object? error) => null, test: (Object? error) => error is vm_service.RPCError) ); } diff --git a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart index c5baa2b640..47b91fb20f 100644 --- a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart @@ -429,25 +429,69 @@ void main() { () async { final ResidentRunner residentWebRunner = setUpResidentRunner(flutterDevice, logger: testLogger); - final Map extensionData = { - 'test': 'data', - 'renderedErrorText': 'error text', - }; - final Map emptyExtensionData = { - 'test': 'data', - 'renderedErrorText': '', - }; - final Map nonStructuredErrorData = { - 'other': 'other stuff', - }; - fakeVmServiceHost = FakeVmServiceHost(requests: [ + final List requests = [ ...kAttachExpectations, + // This is the first error message of a session. FakeVmServiceStreamResponse( streamId: 'Extension', event: vm_service.Event( timestamp: 0, extensionKind: 'Flutter.Error', - extensionData: vm_service.ExtensionData.parse(extensionData), + extensionData: vm_service.ExtensionData.parse({ + 'errorsSinceReload': 0, + 'renderedErrorText': 'first', + }), + kind: vm_service.EventStreams.kExtension, + ), + ), + // This is the second error message of a session. + FakeVmServiceStreamResponse( + streamId: 'Extension', + event: vm_service.Event( + timestamp: 0, + extensionKind: 'Flutter.Error', + extensionData: vm_service.ExtensionData.parse({ + 'errorsSinceReload': 1, + 'renderedErrorText': 'second', + }), + kind: vm_service.EventStreams.kExtension, + ), + ), + // This is not Flutter.Error kind data, so it should not be logged, even though it has a renderedErrorText field. + FakeVmServiceStreamResponse( + streamId: 'Extension', + event: vm_service.Event( + timestamp: 0, + extensionKind: 'Other', + extensionData: vm_service.ExtensionData.parse({ + 'errorsSinceReload': 2, + 'renderedErrorText': 'not an error', + }), + kind: vm_service.EventStreams.kExtension, + ), + ), + // This is the third error message of a session. + FakeVmServiceStreamResponse( + streamId: 'Extension', + event: vm_service.Event( + timestamp: 0, + extensionKind: 'Flutter.Error', + extensionData: vm_service.ExtensionData.parse({ + 'errorsSinceReload': 2, + 'renderedErrorText': 'third', + }), + kind: vm_service.EventStreams.kExtension, + ), + ), + // This is bogus error data. + FakeVmServiceStreamResponse( + streamId: 'Extension', + event: vm_service.Event( + timestamp: 0, + extensionKind: 'Flutter.Error', + extensionData: vm_service.ExtensionData.parse({ + 'other': 'bad stuff', + }), kind: vm_service.EventStreams.kExtension, ), ), @@ -457,21 +501,33 @@ void main() { event: vm_service.Event( timestamp: 0, extensionKind: 'Flutter.Error', - extensionData: vm_service.ExtensionData.parse(emptyExtensionData), + extensionData: vm_service.ExtensionData.parse({ + 'test': 'data', + 'renderedErrorText': '', + }), kind: vm_service.EventStreams.kExtension, ), ), - // This is not Flutter.Error kind data, so it should not be logged. + // Messages without errorsSinceReload should act as if errorsSinceReload: 0 FakeVmServiceStreamResponse( streamId: 'Extension', event: vm_service.Event( timestamp: 0, - extensionKind: 'Other', - extensionData: vm_service.ExtensionData.parse(nonStructuredErrorData), + extensionKind: 'Flutter.Error', + extensionData: vm_service.ExtensionData.parse({ + 'test': 'data', + 'renderedErrorText': 'error text', + }), kind: vm_service.EventStreams.kExtension, ), ), - ]); + // When adding things here, make sure the last one is supposed to output something + // to the statusLog, otherwise you won't be able to distinguish the absence of output + // due to it passing the test from absence due to it not running the test. + ]; + // We use requests below, so make a copy of it here (FakeVmServiceHost will + // clear its copy internally, which would affect our pumping below). + fakeVmServiceHost = FakeVmServiceHost(requests: requests.toList()); setupMocks(); final Completer connectionInfoCompleter = @@ -480,10 +536,34 @@ void main() { connectionInfoCompleter: connectionInfoCompleter, )); await connectionInfoCompleter.future; - await null; + assert(requests.length > 5, 'requests was modified'); + for (final Object _ in requests) { + // pump the task queue once for each message + await null; + } - expect(testLogger.statusText, contains('\nerror text')); - expect(testLogger.statusText, isNot(contains('other stuff'))); + expect(testLogger.statusText, + 'Launching lib/main.dart on FakeDevice in debug mode...\n' + 'Waiting for connection from debug service on FakeDevice...\n' + 'Debug service listening on ws://127.0.0.1/abcd/\n' + '\n' + '💪 Running with sound null safety 💪\n' + '\n' + 'first\n' + '\n' + 'second\n' + 'third\n' + '\n' + '\n' // the empty message + '\n' + '\n' + 'error text\n' + '\n' + ); + + expect(testLogger.errorText, + 'Received an invalid Flutter.Error message from app: {other: bad stuff}\n' + ); }, overrides: { FileSystem: () => fileSystem, ProcessManager: () => processManager, diff --git a/packages/flutter_tools/test/integration.shard/overall_experience_test.dart b/packages/flutter_tools/test/integration.shard/overall_experience_test.dart index 6df9683c20..6a4d8ed62a 100644 --- a/packages/flutter_tools/test/integration.shard/overall_experience_test.dart +++ b/packages/flutter_tools/test/integration.shard/overall_experience_test.dart @@ -548,6 +548,7 @@ void main() { ' verticalDirection: down', '◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤', '════════════════════════════════════════════════════════════════════════════════════════════════════', + '', startsWith('Reloaded 0 libraries in '), '', 'Application finished.',