From f0174b176a87ca12a8128ca1cd5edadcfede46b5 Mon Sep 17 00:00:00 2001 From: Helin Shiah Date: Mon, 8 Jun 2020 11:24:23 -0700 Subject: [PATCH] Send text error in JSON and print in tools (#58994) * Revert "Revert "Send text error in JSON and print in tools (#58284)" (#58872)" This reverts commit c2d5e18cb2df9400ab3b34f12fb1eba957084e2f. * Put streamListen in try/catch if extension events already listened for --- .../lib/src/widgets/widget_inspector.dart | 11 ++- .../test/widgets/widget_inspector_test.dart | 5 ++ .../src/build_runner/resident_web_runner.dart | 10 +++ .../lib/src/resident_runner.dart | 19 ++++- packages/flutter_tools/lib/src/vmservice.dart | 17 ++++ .../commands.shard/hermetic/attach_test.dart | 1 + .../test/general.shard/cold_test.dart | 1 + .../test/general.shard/hot_test.dart | 1 + .../general.shard/resident_runner_test.dart | 1 + .../resident_web_runner_test.dart | 83 ++++++++++++++++--- .../test/general.shard/vmservice_test.dart | 25 ++++++ 11 files changed, 158 insertions(+), 16 deletions(-) diff --git a/packages/flutter/lib/src/widgets/widget_inspector.dart b/packages/flutter/lib/src/widgets/widget_inspector.dart index 03177fa988a..fe655c6a380 100644 --- a/packages/flutter/lib/src/widgets/widget_inspector.dart +++ b/packages/flutter/lib/src/widgets/widget_inspector.dart @@ -929,8 +929,17 @@ mixin WidgetInspectorService { ); errorJson['errorsSinceReload'] = _errorsSinceReload; - _errorsSinceReload += 1; + if (_errorsSinceReload == 0) { + errorJson['renderedErrorText'] = TextTreeRenderer( + wrapWidth: FlutterError.wrapWidth, + wrapWidthProperties: FlutterError.wrapWidth, + maxDescendentsTruncatableNode: 5, + ).render(details.toDiagnosticsNode(style: DiagnosticsTreeStyle.error)).trimRight(); + } else { + errorJson['renderedErrorText'] = 'Another exception was thrown: ${details.summary}'; + } + _errorsSinceReload += 1; postEvent('Flutter.Error', errorJson); } diff --git a/packages/flutter/test/widgets/widget_inspector_test.dart b/packages/flutter/test/widgets/widget_inspector_test.dart index 8cdd02aa643..7c467e28d5c 100644 --- a/packages/flutter/test/widgets/widget_inspector_test.dart +++ b/packages/flutter/test/widgets/widget_inspector_test.dart @@ -2254,6 +2254,10 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { // Validate that we received an error count. expect(error['errorsSinceReload'], 0); + expect( + error['renderedErrorText'], + startsWith( + '══╡ EXCEPTION CAUGHT BY RENDERING LIBRARY ╞════════════')); // Send a second error. FlutterError.reportError(FlutterErrorDetailsForRendering( @@ -2267,6 +2271,7 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { expect(flutterErrorEvents, hasLength(2)); error = flutterErrorEvents.last; expect(error['errorsSinceReload'], 1); + expect(error['renderedErrorText'], startsWith('Another exception was thrown:')); // Reloads the app. final FlutterExceptionHandler oldHandler = FlutterError.onError; diff --git a/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart b/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart index e2ad8f7b2bd..a28fb741780 100644 --- a/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart +++ b/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart @@ -112,6 +112,7 @@ abstract class ResidentWebRunner extends ResidentRunner { ConnectionResult _connectionResult; StreamSubscription _stdOutSub; StreamSubscription _stdErrSub; + StreamSubscription _extensionEventSub; bool _exited = false; WipConnection _wipConnection; ChromiumLauncher _chromiumLauncher; @@ -150,6 +151,7 @@ abstract class ResidentWebRunner extends ResidentRunner { } await _stdOutSub?.cancel(); await _stdErrSub?.cancel(); + await _extensionEventSub?.cancel(); await device.device.stopApp(null); try { _generatedEntrypointDirectory?.deleteSync(recursive: true); @@ -675,6 +677,8 @@ class _ResidentWebRunner extends ResidentWebRunner { final String message = utf8.decode(base64.decode(log.bytes)); globals.printStatus(message, newline: false); }); + _extensionEventSub = + _vmService.onExtensionEvent.listen(printStructuredErrorLog); try { await _vmService.streamListen(vmservice.EventStreams.kStdout); } on vmservice.RPCError { @@ -693,6 +697,12 @@ class _ResidentWebRunner extends ResidentWebRunner { // It is safe to ignore this error because we expect an error to be // thrown if we're not already subscribed. } + try { + await _vmService.streamListen(vmservice.EventStreams.kExtension); + } on vmservice.RPCError { + // It is safe to ignore this error because we expect an error to be + // thrown if we're not already subscribed. + } unawaited(_vmService.registerService('reloadSources', 'FlutterTools')); _vmService.registerServiceCallback('reloadSources', (Map params) async { final bool pause = params['pause'] as bool ?? false; diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index 5a999b1a903..bb0282eba8a 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -184,6 +184,7 @@ class FlutterDevice { CompileExpression compileExpression, ReloadMethod reloadMethod, GetSkSLMethod getSkSLMethod, + PrintStructuredErrorLogMethod printStructuredErrorLogMethod, }) { final Completer completer = Completer(); StreamSubscription subscription; @@ -203,6 +204,7 @@ class FlutterDevice { compileExpression: compileExpression, reloadMethod: reloadMethod, getSkSLMethod: getSkSLMethod, + printStructuredErrorLogMethod: printStructuredErrorLogMethod, device: device, ); } on Exception catch (exception) { @@ -1055,11 +1057,23 @@ abstract class ResidentRunner { final String copyPath = getDefaultCachedKernelPath( trackWidgetCreation: trackWidgetCreation, ); - globals.fs.file(copyPath).parent.createSync(recursive: true); + globals.fs + .file(copyPath) + .parent + .createSync(recursive: true); outputDill.copySync(copyPath); } } + void printStructuredErrorLog(vm_service.Event event) { + if (event.extensionKind == 'Flutter.Error') { + final Map json = event.extensionData?.data; + if (json != null && json.containsKey('renderedErrorText')) { + globals.printStatus('\n${json['renderedErrorText']}'); + } + } + } + /// If the [reloadSources] parameter is not null the 'reloadSources' service /// will be registered. // @@ -1084,7 +1098,8 @@ abstract class ResidentRunner { restart: restart, compileExpression: compileExpression, reloadMethod: reloadMethod, - getSkSLMethod: getSkSLMethod + getSkSLMethod: getSkSLMethod, + printStructuredErrorLogMethod: printStructuredErrorLog, ); // This will wait for at least one flutter view before returning. final Status status = globals.logger.startProgress( diff --git a/packages/flutter_tools/lib/src/vmservice.dart b/packages/flutter_tools/lib/src/vmservice.dart index 9fcf284bbf7..ad7218bff5b 100644 --- a/packages/flutter_tools/lib/src/vmservice.dart +++ b/packages/flutter_tools/lib/src/vmservice.dart @@ -31,6 +31,8 @@ const int kIsolateReloadBarred = 1005; /// for [WebSocket]s (used by tests). typedef WebSocketConnector = Future Function(String url, {io.CompressionOptions compression}); +typedef PrintStructuredErrorLogMethod = void Function(vm_service.Event); + WebSocketConnector _openChannel = _defaultOpenChannel; /// The error codes for the JSON-RPC standard. @@ -149,6 +151,7 @@ typedef VMServiceConnector = Future Function(Uri httpUri, CompileExpression compileExpression, ReloadMethod reloadMethod, GetSkSLMethod getSkSLMethod, + PrintStructuredErrorLogMethod printStructuredErrorLogMethod, io.CompressionOptions compression, Device device, }); @@ -175,6 +178,7 @@ vm_service.VmService setUpVmService( Device device, ReloadMethod reloadMethod, GetSkSLMethod skSLMethod, + PrintStructuredErrorLogMethod printStructuredErrorLogMethod, vm_service.VmService vmService ) { if (reloadSources != null) { @@ -293,6 +297,15 @@ vm_service.VmService setUpVmService( }); vmService.registerService('flutterGetSkSL', 'Flutter Tools'); } + if (printStructuredErrorLogMethod != null) { + try { + vmService.streamListen(vm_service.EventStreams.kExtension); + } on vm_service.RPCError { + // It is safe to ignore this error because we expect an error to be + // thrown if we're already subscribed. + } + vmService.onExtensionEvent.listen(printStructuredErrorLogMethod); + } return vmService; } @@ -311,6 +324,7 @@ Future connectToVmService( CompileExpression compileExpression, ReloadMethod reloadMethod, GetSkSLMethod getSkSLMethod, + PrintStructuredErrorLogMethod printStructuredErrorLogMethod, io.CompressionOptions compression = io.CompressionOptions.compressionDefault, Device device, }) async { @@ -323,6 +337,7 @@ Future connectToVmService( device: device, reloadMethod: reloadMethod, getSkSLMethod: getSkSLMethod, + printStructuredErrorLogMethod: printStructuredErrorLogMethod, ); } @@ -333,6 +348,7 @@ Future _connect( CompileExpression compileExpression, ReloadMethod reloadMethod, GetSkSLMethod getSkSLMethod, + PrintStructuredErrorLogMethod printStructuredErrorLogMethod, io.CompressionOptions compression = io.CompressionOptions.compressionDefault, Device device, }) async { @@ -354,6 +370,7 @@ Future _connect( device, reloadMethod, getSkSLMethod, + printStructuredErrorLogMethod, delegateService, ); _httpAddressExpando[service] = httpUri; diff --git a/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart index 4b48cdb3117..8fae84fe244 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart @@ -679,6 +679,7 @@ VMServiceConnector getFakeVmServiceFactory({ CompileExpression compileExpression, ReloadMethod reloadMethod, GetSkSLMethod getSkSLMethod, + PrintStructuredErrorLogMethod printStructuredErrorLogMethod, CompressionOptions compression, Device device, }) async { diff --git a/packages/flutter_tools/test/general.shard/cold_test.dart b/packages/flutter_tools/test/general.shard/cold_test.dart index 83b3d74e6c9..d9f88a9d55e 100644 --- a/packages/flutter_tools/test/general.shard/cold_test.dart +++ b/packages/flutter_tools/test/general.shard/cold_test.dart @@ -135,6 +135,7 @@ class TestFlutterDevice extends FlutterDevice { CompileExpression compileExpression, ReloadMethod reloadMethod, GetSkSLMethod getSkSLMethod, + PrintStructuredErrorLogMethod printStructuredErrorLogMethod, }) async { throw exception; } diff --git a/packages/flutter_tools/test/general.shard/hot_test.dart b/packages/flutter_tools/test/general.shard/hot_test.dart index 48de79b31ea..1f786394b1a 100644 --- a/packages/flutter_tools/test/general.shard/hot_test.dart +++ b/packages/flutter_tools/test/general.shard/hot_test.dart @@ -510,6 +510,7 @@ class TestFlutterDevice extends FlutterDevice { CompileExpression compileExpression, ReloadMethod reloadMethod, GetSkSLMethod getSkSLMethod, + PrintStructuredErrorLogMethod printStructuredErrorLogMethod, }) async { throw exception; } diff --git a/packages/flutter_tools/test/general.shard/resident_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_runner_test.dart index 37b299107a1..4cf97ff41f6 100644 --- a/packages/flutter_tools/test/general.shard/resident_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_runner_test.dart @@ -1357,6 +1357,7 @@ void main() { CompileExpression compileExpression, ReloadMethod reloadMethod, GetSkSLMethod getSkSLMethod, + PrintStructuredErrorLogMethod printStructuredErrorLogMethod, io.CompressionOptions compression, Device device, }) async => mockVMService, 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 aca6cecb0db..160a8cc572d 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 @@ -4,6 +4,7 @@ import 'dart:async'; import 'dart:convert'; +import 'dart:io'; import 'package:dwds/dwds.dart'; import 'package:flutter_tools/src/base/common.dart'; @@ -56,6 +57,12 @@ const List kAttachIsolateExpectations = { + 'streamId': 'Extension', + }, + ), FakeVmServiceRequest( method: 'registerService', args: { @@ -340,6 +347,68 @@ void main() { expect(testLogger.statusText, contains('SO IS THIS')); })); + test('Listens to extension events with structured errors', () => testbed.run(() async { + final Map extensionData = { + 'test': 'data', + 'renderedErrorText': 'error text', + }; + final Map emptyExtensionData = { + 'test': 'data', + 'renderedErrorText': '', + }; + final Map nonStructuredErrorData = { + 'other': 'other stuff', + }; + fakeVmServiceHost = FakeVmServiceHost(requests: [ + ...kAttachExpectations, + FakeVmServiceStreamResponse( + streamId: 'Extension', + event: vm_service.Event( + timestamp: 0, + extensionKind: 'Flutter.Error', + extensionData: vm_service.ExtensionData.parse(extensionData), + kind: vm_service.EventStreams.kExtension, + ), + ), + // Empty error text should not break anything. + FakeVmServiceStreamResponse( + streamId: 'Extension', + event: vm_service.Event( + timestamp: 0, + extensionKind: 'Flutter.Error', + extensionData: vm_service.ExtensionData.parse(emptyExtensionData), + kind: vm_service.EventStreams.kExtension, + ), + ), + // This is not Flutter.Error kind data, so it should not be logged. + FakeVmServiceStreamResponse( + streamId: 'Extension', + event: vm_service.Event( + timestamp: 0, + extensionKind: 'Other', + extensionData: vm_service.ExtensionData.parse(nonStructuredErrorData), + kind: vm_service.EventStreams.kExtension, + ), + ), + ]); + + _setupMocks(); + final Completer connectionInfoCompleter = Completer(); + unawaited(residentWebRunner.run( + connectionInfoCompleter: connectionInfoCompleter, + )); + await connectionInfoCompleter.future; + + // Need these to run events, otherwise expect statements below run before + // structured errors are processed. + await null; + await null; + await null; + + expect(testLogger.statusText, contains('\nerror text')); + expect(testLogger.statusText, isNot(contains('other stuff'))); + })); + test('Does not run main with --start-paused', () => testbed.run(() async { fakeVmServiceHost = FakeVmServiceHost(requests: kAttachExpectations.toList()); residentWebRunner = DwdsWebRunnerFactory().createWebRunner( @@ -1073,19 +1142,7 @@ void main() { test('Sends launched app.webLaunchUrl event for Chrome device', () => testbed.run(() async { fakeVmServiceHost = FakeVmServiceHost(requests: [ ...kAttachLogExpectations, - const FakeVmServiceRequest( - method: 'streamListen', - args: { - 'streamId': 'Isolate' - } - ), - const FakeVmServiceRequest( - method: 'registerService', - args: { - 'service': 'reloadSources', - 'alias': 'FlutterTools', - } - ) + ...kAttachIsolateExpectations, ]); _setupMocks(); final ChromiumLauncher chromiumLauncher = MockChromeLauncher(); diff --git a/packages/flutter_tools/test/general.shard/vmservice_test.dart b/packages/flutter_tools/test/general.shard/vmservice_test.dart index 859e8b07a41..78bfed103e1 100644 --- a/packages/flutter_tools/test/general.shard/vmservice_test.dart +++ b/packages/flutter_tools/test/general.shard/vmservice_test.dart @@ -104,6 +104,7 @@ void main() { null, null, null, + null, mockVMService, ); @@ -123,6 +124,7 @@ void main() { null, reloadMethod, null, + null, mockVMService, ); @@ -142,6 +144,7 @@ void main() { mockDevice, null, null, + null, mockVMService, ); @@ -159,6 +162,7 @@ void main() { null, null, () async => 'hello', + null, mockVMService, ); @@ -167,6 +171,26 @@ void main() { Logger: () => BufferLogger.test() }); + testUsingContext('VmService registers flutterPrintStructuredErrorLogMethod', () async { + final MockVMService mockVMService = MockVMService(); + when(mockVMService.onExtensionEvent).thenAnswer((Invocation invocation) { + return const Stream.empty(); + }); + setUpVmService( + null, + null, + null, + null, + null, + null, + (vm_service.Event event) async => 'hello', + mockVMService, + ); + verify(mockVMService.streamListen(vm_service.EventStreams.kExtension)).called(1); + }, overrides: { + Logger: () => BufferLogger.test() + }); + testUsingContext('VMService returns correct FlutterVersion', () async { final MockVMService mockVMService = MockVMService(); setUpVmService( @@ -176,6 +200,7 @@ void main() { null, null, null, + null, mockVMService, );