Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Commit 4c99da6

Browse files
HixieHixie
authored
Avoid printing blank lines between "Another exception was thrown:" messages. (#119587)
1 parent 8f5949e commit 4c99da6

File tree

7 files changed

+156
-36
lines changed

7 files changed

+156
-36
lines changed

‎dev/bots/analyze.dart‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,9 @@ Future<void> run(List<String> arguments) async {
169169

170170
// Try with the --watch analyzer, to make sure it returns success also.
171171
// The --benchmark argument exits after one run.
172+
// We specify a failureMessage so that the actual output is muted in the case where _runFlutterAnalyze above already failed.
172173
printProgress('Dart analysis (with --watch)...');
173-
await _runFlutterAnalyze(flutterRoot, options: <String>[
174+
await _runFlutterAnalyze(flutterRoot, failureMessage: 'Dart analyzer failed when --watch was used.', options: <String>[
174175
'--flutter-repo',
175176
'--watch',
176177
'--benchmark',
@@ -196,7 +197,7 @@ Future<void> run(List<String> arguments) async {
196197
],
197198
workingDirectory: flutterRoot,
198199
);
199-
await _runFlutterAnalyze(outDir.path, options: <String>[
200+
await _runFlutterAnalyze(outDir.path, failureMessage: 'Dart analyzer failed on mega_gallery benchmark.', options: <String>[
200201
'--watch',
201202
'--benchmark',
202203
...arguments,
@@ -1942,11 +1943,13 @@ Diff at character #$indexOfDifference
19421943

19431944
Future<CommandResult> _runFlutterAnalyze(String workingDirectory, {
19441945
List<String> options = const <String>[],
1946+
String? failureMessage,
19451947
}) async {
19461948
return runCommand(
19471949
flutter,
19481950
<String>['analyze', ...options],
19491951
workingDirectory: workingDirectory,
1952+
failureMessage: failureMessage,
19501953
);
19511954
}
19521955

‎dev/bots/run_command.dart‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,13 +190,24 @@ Future<CommandResult> runCommand(String executable, List<String> arguments, {
190190
print(result.flattenedStderr);
191191
break;
192192
}
193+
String allOutput;
194+
if (failureMessage == null) {
195+
allOutput = '${result.flattenedStdout}\n${result.flattenedStderr}';
196+
if (allOutput.split('\n').length > 10) {
197+
allOutput = '(stdout/stderr output was more than 10 lines)';
198+
}
199+
} else {
200+
allOutput = '';
201+
}
193202
foundError(<String>[
194203
if (failureMessage != null)
195-
failureMessage
196-
else
197-
'$bold${red}Command exited with exit code ${result.exitCode} but expected: ${expectNonZeroExit ? (expectedExitCode ?? 'non-zero') : 'zero'} exit code.$reset',
204+
failureMessage,
198205
'${bold}Command: $green$commandDescription$reset',
199-
'${bold}Relative working directory: $cyan$relativeWorkingDir$reset',
206+
if (failureMessage == null)
207+
'$bold${red}Command exited with exit code ${result.exitCode} but expected ${expectNonZeroExit ? (expectedExitCode ?? 'non-zero') : 'zero'} exit code.$reset',
208+
'${bold}Working directory: $cyan${path.absolute(relativeWorkingDir)}$reset',
209+
if (allOutput.isNotEmpty)
210+
'${bold}stdout and stderr output:\n$allOutput',
200211
]);
201212
} else {
202213
print('ELAPSED TIME: ${prettyPrintDuration(result.elapsedTime)} for $green$commandDescription$reset in $cyan$relativeWorkingDir$reset');

‎dev/bots/utils.dart‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,7 @@ VoidCallback? onError;
8282
bool get hasError => _hasError;
8383
bool _hasError = false;
8484

85-
Iterable<String> get errorMessages => _errorMessages;
86-
List<String> _errorMessages = <String>[];
85+
List<List<String>> _errorMessages = <List<String>>[];
8786

8887
final List<String> _pendingLogs = <String>[];
8988
Timer? _hideTimer; // When this is null, the output is verbose.
@@ -104,7 +103,7 @@ void foundError(List<String> messages) {
104103
// another error.
105104
_pendingLogs.forEach(_printLoudly);
106105
_pendingLogs.clear();
107-
_errorMessages.addAll(messages);
106+
_errorMessages.add(messages);
108107
_hasError = true;
109108
if (onError != null) {
110109
onError!();
@@ -124,8 +123,18 @@ Never reportErrorsAndExit() {
124123
_hideTimer?.cancel();
125124
_hideTimer = null;
126125
print(redLine);
127-
print('For your convenience, the error messages reported above are repeated here:');
128-
_errorMessages.forEach(print);
126+
print('${red}For your convenience, the error messages reported above are repeated here:$reset');
127+
final bool printSeparators = _errorMessages.any((List<String> messages) => messages.length > 1);
128+
if (printSeparators) {
129+
print(' 🙙 🙛 ');
130+
}
131+
for (int index = 0; index < _errorMessages.length * 2 - 1; index += 1) {
132+
if (index.isEven) {
133+
_errorMessages[index ~/ 2].forEach(print);
134+
} else if (printSeparators) {
135+
print(' 🙙 🙛 ');
136+
}
137+
}
129138
print(redLine);
130139
system.exit(1);
131140
}

‎packages/flutter_tools/lib/src/resident_runner.dart‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,9 +1329,25 @@ abstract class ResidentRunner extends ResidentHandlers {
13291329

13301330
void printStructuredErrorLog(vm_service.Event event) {
13311331
if (event.extensionKind == 'Flutter.Error' && !machine) {
1332-
final Map<dynamic, dynamic>? json = event.extensionData?.data;
1332+
final Map<String, Object?>? json = event.extensionData?.data;
13331333
if (json != null && json.containsKey('renderedErrorText')) {
1334-
globals.printStatus('\n${json['renderedErrorText']}');
1334+
final int errorsSinceReload;
1335+
if (json.containsKey('errorsSinceReload') && json['errorsSinceReload'] is int) {
1336+
errorsSinceReload = json['errorsSinceReload']! as int;
1337+
} else {
1338+
errorsSinceReload = 0;
1339+
}
1340+
if (errorsSinceReload == 0) {
1341+
// We print a blank line around the first error, to more clearly emphasize it
1342+
// in the output. (Other errors don't get this.)
1343+
globals.printStatus('');
1344+
}
1345+
globals.printStatus('${json['renderedErrorText']}');
1346+
if (errorsSinceReload == 0) {
1347+
globals.printStatus('');
1348+
}
1349+
} else {
1350+
globals.printError('Received an invalid ${globals.logger.terminal.bolden("Flutter.Error")} message from app: $json');
13351351
}
13361352
}
13371353
}

‎packages/flutter_tools/lib/src/vmservice.dart‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,11 +285,11 @@ Future<vm_service.VmService> setUpVmService(
285285
}
286286
if (printStructuredErrorLogMethod != null) {
287287
vmService.onExtensionEvent.listen(printStructuredErrorLogMethod);
288-
// It is safe to ignore this error because we expect an error to be
289-
// thrown if we're already subscribed.
290288
registrationRequests.add(vmService
291289
.streamListen(vm_service.EventStreams.kExtension)
292290
.then<vm_service.Success?>((vm_service.Success success) => success)
291+
// It is safe to ignore this error because we expect an error to be
292+
// thrown if we're already subscribed.
293293
.catchError((Object? error) => null, test: (Object? error) => error is vm_service.RPCError)
294294
);
295295
}

‎packages/flutter_tools/test/general.shard/resident_web_runner_test.dart‎

Lines changed: 101 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -429,49 +429,105 @@ void main() {
429429
() async {
430430
final ResidentRunner residentWebRunner =
431431
setUpResidentRunner(flutterDevice, logger: testLogger);
432-
final Map<String, String> extensionData = <String, String>{
433-
'test': 'data',
434-
'renderedErrorText': 'error text',
435-
};
436-
final Map<String, String> emptyExtensionData = <String, String>{
437-
'test': 'data',
438-
'renderedErrorText': '',
439-
};
440-
final Map<String, String> nonStructuredErrorData = <String, String>{
441-
'other': 'other stuff',
442-
};
443-
fakeVmServiceHost = FakeVmServiceHost(requests: <VmServiceExpectation>[
432+
final List<VmServiceExpectation> requests = <VmServiceExpectation>[
444433
...kAttachExpectations,
434+
// This is the first error message of a session.
445435
FakeVmServiceStreamResponse(
446436
streamId: 'Extension',
447437
event: vm_service.Event(
448438
timestamp: 0,
449439
extensionKind: 'Flutter.Error',
450-
extensionData: vm_service.ExtensionData.parse(extensionData),
440+
extensionData: vm_service.ExtensionData.parse(<String, Object?>{
441+
'errorsSinceReload': 0,
442+
'renderedErrorText': 'first',
443+
}),
451444
kind: vm_service.EventStreams.kExtension,
452445
),
453446
),
454-
// Empty error text should not break anything.
447+
// This is the second error message of a session.
455448
FakeVmServiceStreamResponse(
456449
streamId: 'Extension',
457450
event: vm_service.Event(
458451
timestamp: 0,
459452
extensionKind: 'Flutter.Error',
460-
extensionData: vm_service.ExtensionData.parse(emptyExtensionData),
453+
extensionData: vm_service.ExtensionData.parse(<String, Object?>{
454+
'errorsSinceReload': 1,
455+
'renderedErrorText': 'second',
456+
}),
461457
kind: vm_service.EventStreams.kExtension,
462458
),
463459
),
464-
// This is not Flutter.Error kind data, so it should not be logged.
460+
// This is not Flutter.Error kind data, so it should not be logged, even though it has a renderedErrorText field.
465461
FakeVmServiceStreamResponse(
466462
streamId: 'Extension',
467463
event: vm_service.Event(
468464
timestamp: 0,
469465
extensionKind: 'Other',
470-
extensionData: vm_service.ExtensionData.parse(nonStructuredErrorData),
466+
extensionData: vm_service.ExtensionData.parse(<String, Object?>{
467+
'errorsSinceReload': 2,
468+
'renderedErrorText': 'not an error',
469+
}),
471470
kind: vm_service.EventStreams.kExtension,
472471
),
473472
),
474-
]);
473+
// This is the third error message of a session.
474+
FakeVmServiceStreamResponse(
475+
streamId: 'Extension',
476+
event: vm_service.Event(
477+
timestamp: 0,
478+
extensionKind: 'Flutter.Error',
479+
extensionData: vm_service.ExtensionData.parse(<String, Object?>{
480+
'errorsSinceReload': 2,
481+
'renderedErrorText': 'third',
482+
}),
483+
kind: vm_service.EventStreams.kExtension,
484+
),
485+
),
486+
// This is bogus error data.
487+
FakeVmServiceStreamResponse(
488+
streamId: 'Extension',
489+
event: vm_service.Event(
490+
timestamp: 0,
491+
extensionKind: 'Flutter.Error',
492+
extensionData: vm_service.ExtensionData.parse(<String, Object?>{
493+
'other': 'bad stuff',
494+
}),
495+
kind: vm_service.EventStreams.kExtension,
496+
),
497+
),
498+
// Empty error text should not break anything.
499+
FakeVmServiceStreamResponse(
500+
streamId: 'Extension',
501+
event: vm_service.Event(
502+
timestamp: 0,
503+
extensionKind: 'Flutter.Error',
504+
extensionData: vm_service.ExtensionData.parse(<String, Object?>{
505+
'test': 'data',
506+
'renderedErrorText': '',
507+
}),
508+
kind: vm_service.EventStreams.kExtension,
509+
),
510+
),
511+
// Messages without errorsSinceReload should act as if errorsSinceReload: 0
512+
FakeVmServiceStreamResponse(
513+
streamId: 'Extension',
514+
event: vm_service.Event(
515+
timestamp: 0,
516+
extensionKind: 'Flutter.Error',
517+
extensionData: vm_service.ExtensionData.parse(<String, Object?>{
518+
'test': 'data',
519+
'renderedErrorText': 'error text',
520+
}),
521+
kind: vm_service.EventStreams.kExtension,
522+
),
523+
),
524+
// When adding things here, make sure the last one is supposed to output something
525+
// to the statusLog, otherwise you won't be able to distinguish the absence of output
526+
// due to it passing the test from absence due to it not running the test.
527+
];
528+
// We use requests below, so make a copy of it here (FakeVmServiceHost will
529+
// clear its copy internally, which would affect our pumping below).
530+
fakeVmServiceHost = FakeVmServiceHost(requests: requests.toList());
475531

476532
setupMocks();
477533
final Completer<DebugConnectionInfo> connectionInfoCompleter =
@@ -480,10 +536,34 @@ void main() {
480536
connectionInfoCompleter: connectionInfoCompleter,
481537
));
482538
await connectionInfoCompleter.future;
483-
await null;
539+
assert(requests.length > 5, 'requests was modified');
540+
for (final Object _ in requests) {
541+
// pump the task queue once for each message
542+
await null;
543+
}
484544

485-
expect(testLogger.statusText, contains('\nerror text'));
486-
expect(testLogger.statusText, isNot(contains('other stuff')));
545+
expect(testLogger.statusText,
546+
'Launching lib/main.dart on FakeDevice in debug mode...\n'
547+
'Waiting for connection from debug service on FakeDevice...\n'
548+
'Debug service listening on ws://127.0.0.1/abcd/\n'
549+
'\n'
550+
'💪 Running with sound null safety 💪\n'
551+
'\n'
552+
'first\n'
553+
'\n'
554+
'second\n'
555+
'third\n'
556+
'\n'
557+
'\n' // the empty message
558+
'\n'
559+
'\n'
560+
'error text\n'
561+
'\n'
562+
);
563+
564+
expect(testLogger.errorText,
565+
'Received an invalid Flutter.Error message from app: {other: bad stuff}\n'
566+
);
487567
}, overrides: <Type, Generator>{
488568
FileSystem: () => fileSystem,
489569
ProcessManager: () => processManager,

‎packages/flutter_tools/test/integration.shard/overall_experience_test.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,7 @@ void main() {
548548
' verticalDirection: down',
549549
'◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤',
550550
'════════════════════════════════════════════════════════════════════════════════════════════════════',
551+
'',
551552
startsWith('Reloaded 0 libraries in '),
552553
'',
553554
'Application finished.',

0 commit comments

Comments
 (0)