diff --git a/packages/flutter_tools/lib/runner.dart b/packages/flutter_tools/lib/runner.dart index b72368238eb..cb7f957edd7 100644 --- a/packages/flutter_tools/lib/runner.dart +++ b/packages/flutter_tools/lib/runner.dart @@ -14,7 +14,6 @@ import 'src/base/context.dart'; import 'src/base/file_system.dart'; import 'src/base/io.dart'; import 'src/base/logger.dart'; -import 'src/base/net.dart'; import 'src/base/process.dart'; import 'src/context_runner.dart'; import 'src/doctor.dart'; @@ -135,20 +134,25 @@ Future _handleToolError( command: args.join(' '), ); - final String errorString = error.toString(); - globals.printError('Oops; flutter has exited unexpectedly: "$errorString".'); + globals.printError('Oops; flutter has exited unexpectedly: "$error".'); try { - await _informUserOfCrash(args, error, stackTrace, errorString); + final CrashDetails details = CrashDetails( + command: _crashCommand(args), + error: error, + stackTrace: stackTrace, + doctorText: await _doctorText(), + ); + final File file = await _createLocalCrashReport(details); + await globals.crashReporter.informUser(details, file); return _exit(1); // This catch catches all exceptions to ensure the message below is printed. } catch (error) { // ignore: avoid_catches_without_on_clauses globals.stdio.stderrWrite( 'Unable to generate crash report due to secondary error: $error\n' - 'please let us know at https://github.com/flutter/flutter/issues.\n', - ); - // Any exception throw here (including one thrown by `_exit()`) will + '${globals.userMessages.flutterToolBugInstructions}\n'); + // Any exception thrown here (including one thrown by `_exit()`) will // get caught by our zone's `onError` handler. In order to avoid an // infinite error loop, we throw an error that is recognized above // and will trigger an immediate exit. @@ -157,42 +161,12 @@ Future _handleToolError( } } -Future _informUserOfCrash(List args, dynamic error, StackTrace stackTrace, String errorString) async { - final String doctorText = await _doctorText(); - final File file = await _createLocalCrashReport(args, error, stackTrace, doctorText); - - globals.printError('A crash report has been written to ${file.path}.'); - globals.printStatus('This crash may already be reported. Check GitHub for similar crashes.', emphasis: true); - - final HttpClientFactory clientFactory = context.get(); - final GitHubTemplateCreator gitHubTemplateCreator = context.get() ?? GitHubTemplateCreator( - fileSystem: globals.fs, - logger: globals.logger, - flutterProjectFactory: globals.projectFactory, - client: clientFactory != null ? clientFactory() : HttpClient(), - ); - final String similarIssuesURL = GitHubTemplateCreator.toolCrashSimilarIssuesURL(errorString); - globals.printStatus('$similarIssuesURL\n', wrap: false); - globals.printStatus('To report your crash to the Flutter team, first read the guide to filing a bug.', emphasis: true); - globals.printStatus('https://flutter.dev/docs/resources/bug-reports\n', wrap: false); - globals.printStatus('Create a new GitHub issue by pasting this link into your browser and completing the issue template. Thank you!', emphasis: true); - - final String command = _crashCommand(args); - final String gitHubTemplateURL = await gitHubTemplateCreator.toolCrashIssueTemplateGitHubURL( - command, - error, - stackTrace, - doctorText - ); - globals.printStatus('$gitHubTemplateURL\n', wrap: false); -} - String _crashCommand(List args) => 'flutter ${args.join(' ')}'; String _crashException(dynamic error) => '${error.runtimeType}: $error'; /// Saves the crash report to a local file. -Future _createLocalCrashReport(List args, dynamic error, StackTrace stackTrace, String doctorText) async { +Future _createLocalCrashReport(CrashDetails details) async { File crashFile = globals.fsUtils.getUniqueFile( globals.fs.currentDirectory, 'flutter', @@ -201,17 +175,18 @@ Future _createLocalCrashReport(List args, dynamic error, StackTrac final StringBuffer buffer = StringBuffer(); - buffer.writeln('Flutter crash report; please file at https://github.com/flutter/flutter/issues.\n'); + buffer.writeln('Flutter crash report.'); + buffer.writeln('${globals.userMessages.flutterToolBugInstructions}\n'); buffer.writeln('## command\n'); - buffer.writeln('${_crashCommand(args)}\n'); + buffer.writeln('${details.command}\n'); buffer.writeln('## exception\n'); - buffer.writeln('${_crashException(error)}\n'); - buffer.writeln('```\n$stackTrace```\n'); + buffer.writeln('${_crashException(details.error)}\n'); + buffer.writeln('```\n${details.stackTrace}```\n'); buffer.writeln('## flutter doctor\n'); - buffer.writeln('```\n$doctorText```'); + buffer.writeln('```\n${details.doctorText}```'); try { crashFile.writeAsStringSync(buffer.toString()); diff --git a/packages/flutter_tools/lib/src/android/android_device_discovery.dart b/packages/flutter_tools/lib/src/android/android_device_discovery.dart index a97f02c0454..7e634f3af0c 100644 --- a/packages/flutter_tools/lib/src/android/android_device_discovery.dart +++ b/packages/flutter_tools/lib/src/android/android_device_discovery.dart @@ -160,7 +160,7 @@ class AndroidDevices extends PollingDeviceDiscovery { diagnostics?.add( 'Unexpected failure parsing device information from adb output:\n' '$line\n' - 'Please report a bug at https://github.com/flutter/flutter/issues/new/choose'); + '${globals.userMessages.flutterToolBugInstructions}'); } } } diff --git a/packages/flutter_tools/lib/src/base/user_messages.dart b/packages/flutter_tools/lib/src/base/user_messages.dart index 0f13e8c6721..8d9652a58aa 100644 --- a/packages/flutter_tools/lib/src/base/user_messages.dart +++ b/packages/flutter_tools/lib/src/base/user_messages.dart @@ -10,6 +10,10 @@ UserMessages get userMessages => context.get(); /// Class containing message strings that can be produced by Flutter tools. class UserMessages { + // Messages used in multiple components. + String get flutterToolBugInstructions => + 'Please report a bug at https://github.com/flutter/flutter/issues.'; + // Messages used in FlutterValidator String flutterStatusInfo(String channel, String version, String os, String locale) => 'Channel ${channel ?? 'unknown'}, ${version ?? 'Unknown'}, on $os, locale $locale'; diff --git a/packages/flutter_tools/lib/src/context_runner.dart b/packages/flutter_tools/lib/src/context_runner.dart index bc6d94e5a41..72a3ba74642 100644 --- a/packages/flutter_tools/lib/src/context_runner.dart +++ b/packages/flutter_tools/lib/src/context_runner.dart @@ -116,6 +116,12 @@ Future runInContext( logger: globals.logger, platform: globals.platform, ), + CrashReporter: () => CrashReporter( + fileSystem: globals.fs, + logger: globals.logger, + flutterProjectFactory: globals.projectFactory, + client: globals.httpClientFactory?.call() ?? HttpClient(), + ), DevFSConfig: () => DevFSConfig(), DeviceManager: () => DeviceManager(), Doctor: () => Doctor(logger: globals.logger), diff --git a/packages/flutter_tools/lib/src/globals.dart b/packages/flutter_tools/lib/src/globals.dart index 22bee70e360..1ad5f12f05a 100644 --- a/packages/flutter_tools/lib/src/globals.dart +++ b/packages/flutter_tools/lib/src/globals.dart @@ -39,7 +39,9 @@ Artifacts get artifacts => context.get(); BuildSystem get buildSystem => context.get(); Cache get cache => context.get(); Config get config => context.get(); +CrashReporter get crashReporter => context.get(); Doctor get doctor => context.get(); +HttpClientFactory get httpClientFactory => context.get(); Logger get logger => context.get(); OperatingSystemUtils get os => context.get(); PersistentToolState get persistentToolState => PersistentToolState.instance; diff --git a/packages/flutter_tools/lib/src/reporting/crash_reporting.dart b/packages/flutter_tools/lib/src/reporting/crash_reporting.dart index 04425ec2741..e3220066648 100644 --- a/packages/flutter_tools/lib/src/reporting/crash_reporting.dart +++ b/packages/flutter_tools/lib/src/reporting/crash_reporting.dart @@ -26,15 +26,73 @@ const String _kStackTraceFileField = 'DartError'; /// it must be supplied in the request. const String _kStackTraceFilename = 'stacktrace_file'; +class CrashDetails { + CrashDetails({ + @required this.command, + @required this.error, + @required this.stackTrace, + @required this.doctorText, + }); + + final String command; + final dynamic error; + final StackTrace stackTrace; + final String doctorText; +} + +/// Reports information about the crash to the user. +class CrashReporter { + CrashReporter({ + @required FileSystem fileSystem, + @required Logger logger, + @required FlutterProjectFactory flutterProjectFactory, + @required HttpClient client, + }) : _fileSystem = fileSystem, + _logger = logger, + _flutterProjectFactory = flutterProjectFactory, + _client = client; + + final FileSystem _fileSystem; + final Logger _logger; + final FlutterProjectFactory _flutterProjectFactory; + final HttpClient _client; + + /// Prints instructions for filing a bug about the crash. + Future informUser(CrashDetails details, File crashFile) async { + _logger.printError('A crash report has been written to ${crashFile.path}.'); + _logger.printStatus('This crash may already be reported. Check GitHub for similar crashes.', emphasis: true); + + final String similarIssuesURL = GitHubTemplateCreator.toolCrashSimilarIssuesURL(details.error.toString()); + _logger.printStatus('$similarIssuesURL\n', wrap: false); + _logger.printStatus('To report your crash to the Flutter team, first read the guide to filing a bug.', emphasis: true); + _logger.printStatus('https://flutter.dev/docs/resources/bug-reports\n', wrap: false); + + _logger.printStatus('Create a new GitHub issue by pasting this link into your browser and completing the issue template. Thank you!', emphasis: true); + + final GitHubTemplateCreator gitHubTemplateCreator = GitHubTemplateCreator( + fileSystem: _fileSystem, + logger: _logger, + flutterProjectFactory: _flutterProjectFactory, + client: _client, + ); + + final String gitHubTemplateURL = await gitHubTemplateCreator.toolCrashIssueTemplateGitHubURL( + details.command, + details.error, + details.stackTrace, + details.doctorText, + ); + _logger.printStatus('$gitHubTemplateURL\n', wrap: false); + } +} + /// Sends crash reports to Google. /// -/// There are two ways to override the behavior of this class: -/// -/// * Define a `FLUTTER_CRASH_SERVER_BASE_URL` environment variable that points -/// to a custom crash reporting server. This is useful if your development -/// environment is behind a firewall and unable to send crash reports to -/// Google, or when you wish to use your own server for collecting crash -/// reports from Flutter Tools. +/// To override the behavior of this class, define a +/// `FLUTTER_CRASH_SERVER_BASE_URL` environment variable that points to a custom +/// crash reporting server. This is useful if your development environment is +/// behind a firewall and unable to send crash reports to Google, or when you +/// wish to use your own server for collecting crash reports from Flutter Tools. class CrashReportSender { CrashReportSender({ @required http.Client client, diff --git a/packages/flutter_tools/test/general.shard/crash_reporting_test.dart b/packages/flutter_tools/test/general.shard/crash_reporting_test.dart index 5162581992e..6a916201a72 100644 --- a/packages/flutter_tools/test/general.shard/crash_reporting_test.dart +++ b/packages/flutter_tools/test/general.shard/crash_reporting_test.dart @@ -5,11 +5,13 @@ import 'dart:async'; import 'dart:convert'; +import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/os.dart'; import 'package:flutter_tools/src/doctor.dart'; +import 'package:flutter_tools/src/project.dart'; import 'package:flutter_tools/src/reporting/reporting.dart'; import 'package:http/http.dart'; import 'package:http/testing.dart'; @@ -18,22 +20,25 @@ import 'package:platform/platform.dart'; import '../src/common.dart'; import '../src/context.dart'; +import '../src/testbed.dart'; void main() { BufferLogger logger; + FileSystem fs; MockUsage mockUsage; Platform platform; OperatingSystemUtils operatingSystemUtils; setUp(() async { logger = BufferLogger.test(); + fs = MemoryFileSystem.test(); mockUsage = MockUsage(); when(mockUsage.clientId).thenReturn('00000000-0000-4000-0000-000000000000'); platform = FakePlatform(environment: {}, operatingSystem: 'linux'); operatingSystemUtils = OperatingSystemUtils( - fileSystem: MemoryFileSystem.test(), + fileSystem: fs, logger: logger, platform: platform, processManager: FakeProcessManager.any(), @@ -71,6 +76,29 @@ void main() { expect(logger.traceText, contains('Crash report sent (report ID: test-report-id)')); } + testWithoutContext('CrashReporter.informUser provides basic instructions', () async { + final CrashReporter crashReporter = CrashReporter( + fileSystem: fs, + logger: logger, + flutterProjectFactory: FlutterProjectFactory(fileSystem: fs, logger: logger), + client: FakeHttpClient(), + ); + + final File file = fs.file('flutter_00.log'); + + await crashReporter.informUser( + CrashDetails( + command: 'arg1 arg2 arg3', + error: Exception('Dummy exception'), + stackTrace: StackTrace.current, + doctorText: 'Fake doctor text'), + file, + ); + + expect(logger.errorText, contains('A crash report has been written to ${file.path}.')); + expect(logger.statusText, contains('https://github.com/flutter/flutter/issues/new')); + }); + testWithoutContext('suppress analytics', () async { when(mockUsage.suppressAnalytics).thenReturn(true); diff --git a/packages/flutter_tools/test/general.shard/runner/runner_test.dart b/packages/flutter_tools/test/general.shard/runner/runner_test.dart index 44d3d793ca1..1b33cf13d08 100644 --- a/packages/flutter_tools/test/general.shard/runner/runner_test.dart +++ b/packages/flutter_tools/test/general.shard/runner/runner_test.dart @@ -9,6 +9,7 @@ import 'package:flutter_tools/runner.dart' as runner; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/io.dart' as io; import 'package:flutter_tools/src/base/common.dart'; +import 'package:flutter_tools/src/base/user_messages.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/reporting/reporting.dart'; @@ -19,11 +20,11 @@ import 'package:platform/platform.dart'; import '../../src/common.dart'; import '../../src/context.dart'; +const String kCustomBugInstructions = 'These are instructions to report with a custom bug tracker.'; + void main() { group('runner', () { - MockGitHubTemplateCreator mockGitHubTemplateCreator; setUp(() { - mockGitHubTemplateCreator = MockGitHubTemplateCreator(); // Instead of exiting with dart:io exit(), this causes an exception to // be thrown, which we catch with the onError callback in the zone below. io.setExitFunctionForTests((int _) { throw 'test exit';}); @@ -77,10 +78,7 @@ void main() { Usage: () => CrashingUsage(), }); - testUsingContext('GitHub issue template', () async { - const String templateURL = 'https://example.com/2'; - when(mockGitHubTemplateCreator.toolCrashIssueTemplateGitHubURL(any, any, any, any)) - .thenAnswer((_) async => templateURL); + testUsingContext('create local report', () async { final Completer completer = Completer(); // runner.run() asynchronously calls the exit function set above, so we // catch it in a zone. @@ -105,14 +103,22 @@ void main() { await completer.future; final String errorText = testLogger.errorText; - expect(errorText, contains('A crash report has been written to /flutter_01.log.')); expect(errorText, contains('Oops; flutter has exited unexpectedly: "an exception % --".\n')); - final String statusText = testLogger.statusText; - expect(statusText, contains('https://github.com/flutter/flutter/issues?q=is%3Aissue+an+exception+%25+--')); - expect(statusText, contains('https://flutter.dev/docs/resources/bug-reports')); - expect(statusText, contains(templateURL)); + final File log = globals.fs.file('/flutter_01.log'); + final String logContents = log.readAsStringSync(); + expect(logContents, contains(kCustomBugInstructions)); + expect(logContents, contains('flutter test')); + expect(logContents, contains('String: an exception % --')); + expect(logContents, contains('CrashingFlutterCommand.runCommand')); + expect(logContents, contains('[✓] Flutter')); + final VerificationResult argVerification = verify(globals.crashReporter.informUser(captureAny, any)); + final CrashDetails sentDetails = argVerification.captured.first as CrashDetails; + expect(sentDetails.command, 'flutter test'); + expect(sentDetails.error, 'an exception % --'); + expect(sentDetails.stackTrace.toString(), contains('CrashingFlutterCommand.runCommand')); + expect(sentDetails.doctorText, contains('[✓] Flutter')); }, overrides: { Platform: () => FakePlatform( environment: { @@ -123,7 +129,7 @@ void main() { ), FileSystem: () => MemoryFileSystem(), ProcessManager: () => FakeProcessManager.any(), - GitHubTemplateCreator: () => mockGitHubTemplateCreator, + UserMessages: () => CustomBugInstructions(), }); }); } @@ -222,3 +228,8 @@ class CrashingUsage implements Usage { @override void printWelcome() => _impl.printWelcome(); } + +class CustomBugInstructions extends UserMessages { + @override + String get flutterToolBugInstructions => kCustomBugInstructions; +} diff --git a/packages/flutter_tools/test/src/context.dart b/packages/flutter_tools/test/src/context.dart index 42cbec52cf5..49b20acbf95 100644 --- a/packages/flutter_tools/test/src/context.dart +++ b/packages/flutter_tools/test/src/context.dart @@ -131,7 +131,7 @@ void testUsingContext( PlistParser: () => FakePlistParser(), Signals: () => FakeSignals(), Pub: () => ThrowingPub(), // prevent accidentally using pub. - GitHubTemplateCreator: () => MockGitHubTemplateCreator(), + CrashReporter: () => MockCrashReporter(), TemplateRenderer: () => const MustacheTemplateRenderer(), }, body: () { @@ -432,7 +432,7 @@ class MockClock extends Mock implements SystemClock {} class MockHttpClient extends Mock implements HttpClient {} -class MockGitHubTemplateCreator extends Mock implements GitHubTemplateCreator {} +class MockCrashReporter extends Mock implements CrashReporter {} class FakePlistParser implements PlistParser { @override