add crash reporting without enabling it (#8518)

* add crash reporting without enabling it

* do not drop futures on the floor

* return exitCode from executable run

* debug travis

* remove unnecessary todo

* rename local fs getter

packages/flutter_tools/lib/executable.dart

@@ -5,7 +5,9 @@
 import 'dart:async';
 
 import 'package:args/command_runner.dart';
+import 'package:flutter_tools/src/version.dart';
 import 'package:intl/intl_standalone.dart' as intl;
+import 'package:meta/meta.dart';
 import 'package:process/process.dart';
 import 'package:stack_trace/stack_trace.dart';
 
@@ -42,6 +44,7 @@ import 'src/commands/test.dart';
 import 'src/commands/trace.dart';
 import 'src/commands/update_packages.dart';
 import 'src/commands/upgrade.dart';
+import 'src/crash_reporting.dart';
 import 'src/devfs.dart';
 import 'src/device.dart';
 import 'src/doctor.dart';
@@ -49,10 +52,10 @@ import 'src/globals.dart';
 import 'src/ios/mac.dart';
 import 'src/ios/simulators.dart';
 import 'src/run_hot.dart';
+import 'src/runner/flutter_command.dart';
 import 'src/runner/flutter_command_runner.dart';
 import 'src/usage.dart';
 
-
 /// Main entry point for commands.
 ///
 /// This function is intended to be used from the `flutter` command line tool.
@@ -62,40 +65,53 @@ Future<Null> main(List<String> args) async {
       (args.isNotEmpty && args.first == 'help') || (args.length == 1 && verbose);
   bool verboseHelp = help && verbose;
 
+  await run(args, <FlutterCommand>[
+    new AnalyzeCommand(verboseHelp: verboseHelp),
+    new BuildCommand(verboseHelp: verboseHelp),
+    new ChannelCommand(),
+    new ConfigCommand(),
+    new CreateCommand(),
+    new DaemonCommand(hidden: !verboseHelp),
+    new DevicesCommand(),
+    new DoctorCommand(),
+    new DriveCommand(),
+    new FormatCommand(),
+    new InstallCommand(),
+    new LogsCommand(),
+    new PackagesCommand(),
+    new PrecacheCommand(),
+    new RunCommand(verboseHelp: verboseHelp),
+    new ScreenshotCommand(),
+    new StopCommand(),
+    new TestCommand(),
+    new TraceCommand(),
+    new UpdatePackagesCommand(hidden: !verboseHelp),
+    new UpgradeCommand(),
+  ], verbose: verbose, verboseHelp: verboseHelp);
+}
+
+Future<int> run(List<String> args, List<FlutterCommand> subCommands, {
+  bool verbose: false,
+  bool verboseHelp: false,
+  bool reportCrashes,
+  String flutterVersion,
+}) async {
+  reportCrashes ??= !isRunningOnBot;
+
   if (verboseHelp) {
     // Remove the verbose option; for help, users don't need to see verbose logs.
     args = new List<String>.from(args);
     args.removeWhere((String option) => option == '-v' || option == '--verbose');
   }
 
-  FlutterCommandRunner runner = new FlutterCommandRunner(verboseHelp: verboseHelp)
-    ..addCommand(new AnalyzeCommand(verboseHelp: verboseHelp))
-    ..addCommand(new BuildCommand(verboseHelp: verboseHelp))
-    ..addCommand(new ChannelCommand())
-    ..addCommand(new ConfigCommand())
-    ..addCommand(new CreateCommand())
-    ..addCommand(new DaemonCommand(hidden: !verboseHelp))
-    ..addCommand(new DevicesCommand())
-    ..addCommand(new DoctorCommand())
-    ..addCommand(new DriveCommand())
-    ..addCommand(new FormatCommand())
-    ..addCommand(new InstallCommand())
-    ..addCommand(new LogsCommand())
-    ..addCommand(new PackagesCommand())
-    ..addCommand(new PrecacheCommand())
-    ..addCommand(new RunCommand(verboseHelp: verboseHelp))
-    ..addCommand(new ScreenshotCommand())
-    ..addCommand(new StopCommand())
-    ..addCommand(new TestCommand())
-    ..addCommand(new TraceCommand())
-    ..addCommand(new UpdatePackagesCommand(hidden: !verboseHelp))
-    ..addCommand(new UpgradeCommand());
+  FlutterCommandRunner runner = new FlutterCommandRunner(verboseHelp: verboseHelp);
+  subCommands.forEach(runner.addCommand);
 
   // Construct a context.
   AppContext _executableContext = new AppContext();
 
   // Make the context current.
-  await _executableContext.runInZone(() async {
+  return await _executableContext.runInZone(() async {
     // Initialize the context with some defaults.
     // NOTE: Similar lists also exist in `bin/fuchsia_builder.dart` and
     // `test/src/context.dart`. If you update this list of defaults, look
@@ -124,10 +140,28 @@ Future<Null> main(List<String> args) async {
     // Initialize the system locale.
     await intl.findSystemLocale();
 
-    return Chain.capture<Future<Null>>(() async {
+    Completer<int> runCompleter = new Completer<int>();
+    Chain.capture<Future<Null>>(() async {
       await runner.run(args);
       await _exit(0);
+      runCompleter.complete(0);
     }, onError: (dynamic error, Chain chain) {
+      flutterVersion ??= FlutterVersion.getVersionString();
+      _handleToolError(error, chain, verbose, args, reportCrashes, flutterVersion)
+          .then(runCompleter.complete, onError: runCompleter.completeError);
+    });
+    return runCompleter.future;
+  });
+}
+
+Future<int> _handleToolError(
+    dynamic error,
+    Chain chain,
+    bool verbose,
+    List<String> args,
+    bool reportCrashes,
+    String flutterVersion,
+) async {
   if (error is UsageException) {
     stderr.writeln(error.message);
     stderr.writeln();
@@ -136,7 +170,7 @@ Future<Null> main(List<String> args) async {
             "flutter commands and options."
     );
     // Argument error exit code.
-        _exit(64);
+    return _exit(64);
   } else if (error is ToolExit) {
     if (error.message != null)
       stderr.writeln(error.message);
@@ -145,49 +179,61 @@ Future<Null> main(List<String> args) async {
       stderr.writeln(chain.terse.toString());
       stderr.writeln();
     }
-        _exit(error.exitCode ?? 1);
+    return _exit(error.exitCode ?? 1);
   } else if (error is ProcessExit) {
     // We've caught an exit code.
-        _exit(error.exitCode);
+    return _exit(error.exitCode);
   } else {
     // We've crashed; emit a log report.
     stderr.writeln();
 
     flutterUsage.sendException(error, chain);
 
-        if (isRunningOnBot) {
+    if (!reportCrashes) {
       // Print the stack trace on the bots - don't write a crash report.
       stderr.writeln('$error');
       stderr.writeln(chain.terse.toString());
-          _exit(1);
+      return _exit(1);
     } else {
       if (error is String)
         stderr.writeln('Oops; flutter has exited unexpectedly: "$error".');
       else
         stderr.writeln('Oops; flutter has exited unexpectedly.');
 
-          _createCrashReport(args, error, chain).then<Null>((File file) {
+      await CrashReportSender.instance.sendReport(
+        error: error,
+        stackTrace: chain,
+        flutterVersion: flutterVersion,
+      );
+      try {
+        File file = await _createLocalCrashReport(args, error, chain);
         stderr.writeln(
             'Crash report written to ${file.path};\n'
                 'please let us know at https://github.com/flutter/flutter/issues.',
         );
-            _exit(1);
-          }).catchError((dynamic error) {
+        return _exit(1);
+      } catch (error) {
         stderr.writeln(
             'Unable to generate crash report due to secondary error: $error\n'
                 'please let us know at https://github.com/flutter/flutter/issues.',
         );
-            _exit(1);
-          });
+        return _exit(1);
+      }
     }
   }
-    });
-  });
 }
 
-Future<File> _createCrashReport(List<String> args, dynamic error, Chain chain) async {
-  FileSystem fs = const LocalFileSystem();
-  File crashFile = getUniqueFile(fs.currentDirectory, 'flutter', 'log');
+/// File system used by the crash reporting logic.
+///
+/// We do not want to use the file system stored in the context because it may
+/// be recording. Additionally, in the case of a crash we do not trust the
+/// integrity of the [AppContext].
+@visibleForTesting
+FileSystem crashFileSystem = new LocalFileSystem();
+
+/// Saves the crash report to a local file.
+Future<File> _createLocalCrashReport(List<String> args, dynamic error, Chain chain) async {
+  File crashFile = getUniqueFile(crashFileSystem.currentDirectory, 'flutter', 'log');
 
   StringBuffer buffer = new StringBuffer();
 
@@ -207,7 +253,7 @@ Future<File> _createCrashReport(List<String> args, dynamic error, Chain chain) a
     await crashFile.writeAsString(buffer.toString());
   } on FileSystemException catch (_) {
     // Fallback to the system temporary directory.
-    crashFile = getUniqueFile(fs.systemTempDirectory, 'flutter', 'log');
+    crashFile = getUniqueFile(crashFileSystem.systemTempDirectory, 'flutter', 'log');
     try {
       await crashFile.writeAsString(buffer.toString());
     } on FileSystemException catch (e) {
@@ -234,7 +280,7 @@ Future<String> _doctorText() async {
   }
 }
 
-Future<Null> _exit(int code) async {
+Future<int> _exit(int code) async {
   if (flutterUsage.isFirstRun)
     flutterUsage.printUsage();
 
@@ -259,4 +305,5 @@ Future<Null> _exit(int code) async {
   });
 
   await completer.future;
+  return code;
 }

packages/flutter_tools/test/all.dart

@@ -23,6 +23,7 @@ import 'bug_report_test.dart' as bug_report_test;
 import 'channel_test.dart' as channel_test;
 import 'config_test.dart' as config_test;
 import 'context_test.dart' as context_test;
+import 'crash_reporting_test.dart' as crash_reporting_test;
 import 'create_test.dart' as create_test;
 import 'daemon_test.dart' as daemon_test;
 import 'dart_dependencies_test.dart' as dart_dependencies_test;
@@ -64,6 +65,7 @@ void main() {
   channel_test.main();
   config_test.main();
   context_test.main();
+  crash_reporting_test.main();
   create_test.main();
   daemon_test.main();
   dart_dependencies_test.main();

packages/flutter_tools/test/crash_reporting_test.dart

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+
+import 'package:file/file.dart';
+import 'package:file/local.dart';
+import 'package:file/memory.dart';
+import 'package:http/http.dart';
+import 'package:http/testing.dart';
+import 'package:test/test.dart';
+
+import 'package:flutter_tools/executable.dart' as tools;
+import 'package:flutter_tools/src/base/context.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' as os;
+import 'package:flutter_tools/src/crash_reporting.dart';
+import 'package:flutter_tools/src/runner/flutter_command.dart';
+import 'src/context.dart';
+
+void main() {
+  group('crash reporting', () {
+    int testPort;
+
+    setUp(() async {
+      tools.crashFileSystem = new MemoryFileSystem();
+      setExitFunctionForTests((_) { });
+      testPort = await os.findAvailablePort();
+      overrideBaseCrashUrlForTesting(Uri.parse('http://localhost:$testPort/test-path'));
+    });
+
+    tearDown(() {
+      tools.crashFileSystem = new LocalFileSystem();
+      restoreExitFunction();
+      resetBaseCrashUrlForTesting();
+    });
+
+    testUsingContext('should send crash reports', () async {
+      String method;
+      Uri uri;
+
+      CrashReportSender.initializeWith(new MockClient((Request request) async {
+        method = request.method;
+        uri = request.url;
+        return new Response(
+            'test-report-id',
+            200
+        );
+      }));
+
+      int exitCode = await tools.run(
+        <String>['crash'],
+        <FlutterCommand>[new _CrashCommand()],
+        reportCrashes: true,
+        flutterVersion: 'test-version',
+      );
+
+      expect(exitCode, 1);
+
+      // Verify that we sent the crash report.
+      expect(method, 'POST');
+      expect(uri, new Uri(
+        scheme: 'http',
+        host: 'localhost',
+        port: testPort,
+        path: '/test-path',
+        queryParameters: <String, String>{
+          'product': 'Flutter_Tools',
+          'version' : 'test-version',
+        },
+      ));
+      BufferLogger logger = context[Logger];
+      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.
+      List<String> 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'));
+    });
+  });
+}
+
+/// Throws a random error to simulate a CLI crash.
+class _CrashCommand extends FlutterCommand {
+
+  @override
+  String get description => 'Simulates a crash';
+
+  @override
+  String get name => 'crash';
+
+  @override
+  Future<Null> runCommand() async {
+    void fn1() {
+      throw new StateError('Test bad state error');
+    }
+
+    void fn2() {
+      fn1();
+    }
+
+    void fn3() {
+      fn2();
+    }
+
+    fn3();
+  }
+}