From d91797807ef324629d4e28b748d48914a5b36d42 Mon Sep 17 00:00:00 2001 From: Alexander Aprelev Date: Mon, 10 Jun 2019 09:52:22 -0700 Subject: [PATCH] Report async callback errors that currently go unreported. (#34081) * Report async callback errors that currently go unreported. * Add a test --- packages/flutter_tools/lib/runner.dart | 21 +- .../test/crash_reporting_test.dart | 189 +++++++++++------- 2 files changed, 133 insertions(+), 77 deletions(-) diff --git a/packages/flutter_tools/lib/runner.dart b/packages/flutter_tools/lib/runner.dart index 6f11a83d28a..d8174ad0c04 100644 --- a/packages/flutter_tools/lib/runner.dart +++ b/packages/flutter_tools/lib/runner.dart @@ -56,14 +56,19 @@ Future run( onFailure: (String _) => 'en_US', ); - try { - await runner.run(args); - await _exit(0); - } catch (error, stackTrace) { - String getVersion() => flutterVersion ?? FlutterVersion.instance.getVersionString(redactUnknownBranches: true); - return await _handleToolError(error, stackTrace, verbose, args, reportCrashes, getVersion); - } - return 0; + String getVersion() => flutterVersion ?? FlutterVersion.instance.getVersionString(redactUnknownBranches: true); + return await runZoned>(() async { + try { + await runner.run(args); + return await _exit(0); + } catch (error, stackTrace) { + return await _handleToolError( + error, stackTrace, verbose, args, reportCrashes, getVersion); + } + }, onError: (Object error, StackTrace stackTrace) async { + await _handleToolError( + error, stackTrace, verbose, args, reportCrashes, getVersion); + }); }, overrides: overrides); } diff --git a/packages/flutter_tools/test/crash_reporting_test.dart b/packages/flutter_tools/test/crash_reporting_test.dart index 12c8ed9c3c4..a427fd3f738 100644 --- a/packages/flutter_tools/test/crash_reporting_test.dart +++ b/packages/flutter_tools/test/crash_reporting_test.dart @@ -19,6 +19,7 @@ import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/crash_reporting.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart'; +import 'package:pedantic/pedantic.dart'; import 'src/common.dart'; import 'src/context.dart'; @@ -40,85 +41,40 @@ void main() { }); testUsingContext('should send crash reports', () async { - String method; - Uri uri; - Map fields; - - CrashReportSender.initializeWith(MockClient((Request request) async { - method = request.method; - uri = request.url; - - // A very ad-hoc multipart request parser. Good enough for this test. - String boundary = request.headers['Content-Type']; - boundary = boundary.substring(boundary.indexOf('boundary=') + 9); - fields = Map.fromIterable( - utf8.decode(request.bodyBytes) - .split('--$boundary') - .map>((String part) { - final Match nameMatch = RegExp(r'name="(.*)"').firstMatch(part); - if (nameMatch == null) - return null; - final String name = nameMatch[1]; - final String value = part.split('\n').skip(2).join('\n').trim(); - return [name, value]; - }) - .where((List pair) => pair != null), - key: (dynamic key) { - final List pair = key; - return pair[0]; - }, - value: (dynamic value) { - final List pair = value; - return pair[1]; - }, - ); - - return Response( - 'test-report-id', - 200, - ); - })); + final RequestInfo requestInfo = RequestInfo(); + CrashReportSender.initializeWith(MockCrashReportSender(requestInfo)); final int exitCode = await tools.run( ['crash'], [_CrashCommand()], reportCrashes: true, flutterVersion: 'test-version', ); - expect(exitCode, 1); - // Verify that we sent the crash report. - expect(method, 'POST'); - expect(uri, Uri( - scheme: 'https', - host: 'clients2.google.com', - port: 443, - path: '/cr/report', - queryParameters: { - 'product': 'Flutter_Tools', - 'version': 'test-version', - }, + await verifyCrashReportSent(requestInfo); + }, overrides: { + Stdio: () => const _NoStderr(), + }); + + testUsingContext('should send crash reports when async throws', () async { + final Completer exitCodeCompleter = Completer(); + setExitFunctionForTests((int exitCode) { + exitCodeCompleter.complete(exitCode); + }); + + final RequestInfo requestInfo = RequestInfo(); + + CrashReportSender.initializeWith(MockCrashReportSender(requestInfo)); + + unawaited(tools.run( + ['crash'], + [_CrashAsyncCommand()], + reportCrashes: true, + flutterVersion: 'test-version', )); - expect(fields['uuid'], '00000000-0000-4000-0000-000000000000'); - expect(fields['product'], 'Flutter_Tools'); - expect(fields['version'], 'test-version'); - expect(fields['osName'], platform.operatingSystem); - expect(fields['osVersion'], 'fake OS name and version'); - expect(fields['type'], 'DartError'); - expect(fields['error_runtime_type'], 'StateError'); - expect(fields['error_message'], 'Bad state: Test bad state error'); - - final BufferLogger logger = context.get(); - expect(logger.statusText, 'Sending crash report to Google.\n' - 'Crash report sent (report ID: test-report-id)\n'); - - // Verify that we've written the crash report to disk. - final List writtenFiles = - (await tools.crashFileSystem.directory('/').list(recursive: true).toList()) - .map((FileSystemEntity e) => e.path).toList(); - expect(writtenFiles, hasLength(1)); - expect(writtenFiles, contains('flutter_01.log')); + expect(await exitCodeCompleter.future, equals(1)); + await verifyCrashReportSent(requestInfo); }, overrides: { Stdio: () => const _NoStderr(), }); @@ -198,6 +154,83 @@ void main() { }); } +class RequestInfo { + String method; + Uri uri; + Map fields; +} + +Future verifyCrashReportSent(RequestInfo crashInfo) async { + // Verify that we sent the crash report. + expect(crashInfo.method, 'POST'); + expect(crashInfo.uri, Uri( + scheme: 'https', + host: 'clients2.google.com', + port: 443, + path: '/cr/report', + queryParameters: { + 'product': 'Flutter_Tools', + 'version': 'test-version', + }, + )); + expect(crashInfo.fields['uuid'], '00000000-0000-4000-0000-000000000000'); + expect(crashInfo.fields['product'], 'Flutter_Tools'); + expect(crashInfo.fields['version'], 'test-version'); + expect(crashInfo.fields['osName'], platform.operatingSystem); + expect(crashInfo.fields['osVersion'], 'fake OS name and version'); + expect(crashInfo.fields['type'], 'DartError'); + expect(crashInfo.fields['error_runtime_type'], 'StateError'); + expect(crashInfo.fields['error_message'], 'Bad state: Test bad state error'); + + final BufferLogger logger = context.get(); + expect(logger.statusText, 'Sending crash report to Google.\n' + 'Crash report sent (report ID: test-report-id)\n'); + + // Verify that we've written the crash report to disk. + final List writtenFiles = + (await tools.crashFileSystem.directory('/').list(recursive: true).toList()) + .map((FileSystemEntity e) => e.path).toList(); + expect(writtenFiles, hasLength(1)); + expect(writtenFiles, contains('flutter_01.log')); +} + +class MockCrashReportSender extends MockClient { + MockCrashReportSender(RequestInfo crashInfo) : super((Request request) async { + crashInfo.method = request.method; + crashInfo.uri = request.url; + + // A very ad-hoc multipart request parser. Good enough for this test. + String boundary = request.headers['Content-Type']; + boundary = boundary.substring(boundary.indexOf('boundary=') + 9); + crashInfo.fields = Map.fromIterable( + utf8.decode(request.bodyBytes) + .split('--$boundary') + .map>((String part) { + final Match nameMatch = RegExp(r'name="(.*)"').firstMatch(part); + if (nameMatch == null) + return null; + final String name = nameMatch[1]; + final String value = part.split('\n').skip(2).join('\n').trim(); + return [name, value]; + }) + .where((List pair) => pair != null), + key: (dynamic key) { + final List pair = key; + return pair[0]; + }, + value: (dynamic value) { + final List pair = value; + return pair[1]; + }, + ); + + return Response( + 'test-report-id', + 200, + ); + }); +} + /// Throws a random error to simulate a CLI crash. class _CrashCommand extends FlutterCommand { @@ -227,6 +260,24 @@ class _CrashCommand extends FlutterCommand { } } +/// Throws StateError from async callback. +class _CrashAsyncCommand extends FlutterCommand { + + @override + String get description => 'Simulates a crash'; + + @override + String get name => 'crash'; + + @override + Future runCommand() async { + Timer.run(() { + throw StateError('Test bad state error'); + }); + return Completer().future; // expect StateError + } +} + class _NoStderr extends Stdio { const _NoStderr();