[flutter tools] Move _informUserOfCrash into crash_reporting.dart (#55614)

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<int> _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<int> _handleToolError(
   }
 }
 
-Future<void> _informUserOfCrash(List<String> 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<HttpClientFactory>();
-  final GitHubTemplateCreator gitHubTemplateCreator = context.get<GitHubTemplateCreator>() ?? 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<String> args) => 'flutter ${args.join(' ')}';
 
 String _crashException(dynamic error) => '${error.runtimeType}: $error';
 
 /// Saves the crash report to a local file.
-Future<File> _createLocalCrashReport(List<String> args, dynamic error, StackTrace stackTrace, String doctorText) async {
+Future<File> _createLocalCrashReport(CrashDetails details) async {
   File crashFile = globals.fsUtils.getUniqueFile(
     globals.fs.currentDirectory,
     'flutter',
@@ -201,17 +175,18 @@ Future<File> _createLocalCrashReport(List<String> 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());

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}');
       }
     }
   }

packages/flutter_tools/lib/src/base/user_messages.dart

@@ -10,6 +10,10 @@ UserMessages get userMessages => context.get<UserMessages>();
 
 /// 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';

packages/flutter_tools/lib/src/context_runner.dart

@@ -116,6 +116,12 @@ Future<T> runInContext<T>(
         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),

packages/flutter_tools/lib/src/globals.dart

@@ -39,7 +39,9 @@ Artifacts get artifacts => context.get<Artifacts>();
 BuildSystem get buildSystem => context.get<BuildSystem>();
 Cache get cache => context.get<Cache>();
 Config get config => context.get<Config>();
+CrashReporter get crashReporter => context.get<CrashReporter>();
 Doctor get doctor => context.get<Doctor>();
+HttpClientFactory get httpClientFactory => context.get<HttpClientFactory>();
 Logger get logger => context.get<Logger>();
 OperatingSystemUtils get os => context.get<OperatingSystemUtils>();
 PersistentToolState get persistentToolState => PersistentToolState.instance;

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<void> 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,

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: <String, String>{}, 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);
 

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<void> completer = Completer<void>();
       // 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: <Type, Generator>{
       Platform: () => FakePlatform(
         environment: <String, String>{
@@ -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;
+}

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