Unverified Commit 36e8b93d authored by Zachary Anderson's avatar Zachary Anderson Committed by GitHub

[flutter_tool] Only send one crash report per run (#38925)

parent ea64b84d
...@@ -115,39 +115,40 @@ Future<int> _handleToolError( ...@@ -115,39 +115,40 @@ Future<int> _handleToolError(
stderr.writeln('$error'); stderr.writeln('$error');
stderr.writeln(stackTrace.toString()); stderr.writeln(stackTrace.toString());
return _exit(1); return _exit(1);
} else { }
// Report to both [Usage] and [CrashReportSender].
flutterUsage.sendException(error);
await CrashReportSender.instance.sendReport(
error: error,
stackTrace: stackTrace,
getFlutterVersion: getFlutterVersion,
command: args.join(' '),
);
if (error is String) // Report to both [Usage] and [CrashReportSender].
stderr.writeln('Oops; flutter has exited unexpectedly: "$error".'); flutterUsage.sendException(error);
else await CrashReportSender.instance.sendReport(
stderr.writeln('Oops; flutter has exited unexpectedly.'); error: error,
stackTrace: stackTrace,
getFlutterVersion: getFlutterVersion,
command: args.join(' '),
);
try { if (error is String) {
final File file = await _createLocalCrashReport(args, error, stackTrace); stderr.writeln('Oops; flutter has exited unexpectedly: "$error".');
stderr.writeln( } else {
'Crash report written to ${file.path};\n' stderr.writeln('Oops; flutter has exited unexpectedly.');
'please let us know at https://github.com/flutter/flutter/issues.', }
);
return _exit(1); try {
} catch (error) { final File file = await _createLocalCrashReport(args, error, stackTrace);
stderr.writeln( stderr.writeln(
'Unable to generate crash report due to secondary error: $error\n' 'Crash report written to ${file.path};\n'
'please let us know at https://github.com/flutter/flutter/issues.', 'please let us know at https://github.com/flutter/flutter/issues.',
); );
// Any exception throw here (including one thrown by `_exit()`) will return _exit(1);
// get caught by our zone's `onError` handler. In order to avoid an } catch (error) {
// infinite error loop, we throw an error that is recognized above stderr.writeln(
// and will trigger an immediate exit. 'Unable to generate crash report due to secondary error: $error\n'
throw ProcessExit(1, immediate: true); 'please let us know at https://github.com/flutter/flutter/issues.',
} );
// Any exception throw 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.
throw ProcessExit(1, immediate: true);
} }
} }
} }
......
...@@ -44,6 +44,8 @@ class CrashReportSender { ...@@ -44,6 +44,8 @@ class CrashReportSender {
static CrashReportSender get instance => _instance ?? CrashReportSender._(http.Client()); static CrashReportSender get instance => _instance ?? CrashReportSender._(http.Client());
bool _crashReportSent = false;
/// Overrides the default [http.Client] with [client] for testing purposes. /// Overrides the default [http.Client] with [client] for testing purposes.
@visibleForTesting @visibleForTesting
static void initializeWith(http.Client client) { static void initializeWith(http.Client client) {
...@@ -76,6 +78,10 @@ class CrashReportSender { ...@@ -76,6 +78,10 @@ class CrashReportSender {
@required String getFlutterVersion(), @required String getFlutterVersion(),
@required String command, @required String command,
}) async { }) async {
// Only send one crash report per run.
if (_crashReportSent) {
return;
}
try { try {
final String flutterVersion = getFlutterVersion(); final String flutterVersion = getFlutterVersion();
...@@ -116,6 +122,7 @@ class CrashReportSender { ...@@ -116,6 +122,7 @@ class CrashReportSender {
final String reportId = await http.ByteStream(resp.stream) final String reportId = await http.ByteStream(resp.stream)
.bytesToString(); .bytesToString();
printStatus('Crash report sent (report ID: $reportId)'); printStatus('Crash report sent (report ID: $reportId)');
_crashReportSent = true;
} else { } else {
printError('Failed to send crash report. Server responded with HTTP status code ${resp.statusCode}'); printError('Failed to send crash report. Server responded with HTTP status code ${resp.statusCode}');
} }
......
...@@ -14,11 +14,13 @@ import 'package:flutter_tools/src/base/io.dart'; ...@@ -14,11 +14,13 @@ import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/doctor.dart';
import 'package:flutter_tools/src/reporting/reporting.dart'; import 'package:flutter_tools/src/reporting/reporting.dart';
import 'package:flutter_tools/src/runner/flutter_command.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart';
import 'package:http/http.dart'; import 'package:http/http.dart';
import 'package:http/testing.dart'; import 'package:http/testing.dart';
import 'package:pedantic/pedantic.dart'; import 'package:pedantic/pedantic.dart';
import 'package:quiver/testing/async.dart';
import '../src/common.dart'; import '../src/common.dart';
import '../src/context.dart'; import '../src/context.dart';
...@@ -32,6 +34,7 @@ void main() { ...@@ -32,6 +34,7 @@ void main() {
setUp(() async { setUp(() async {
tools.crashFileSystem = MemoryFileSystem(); tools.crashFileSystem = MemoryFileSystem();
setExitFunctionForTests((_) { }); setExitFunctionForTests((_) { });
MockCrashReportSender.sendCalls = 0;
}); });
tearDown(() { tearDown(() {
...@@ -72,12 +75,43 @@ void main() { ...@@ -72,12 +75,43 @@ void main() {
reportCrashes: true, reportCrashes: true,
flutterVersion: 'test-version', flutterVersion: 'test-version',
)); ));
expect(await exitCodeCompleter.future, equals(1)); expect(await exitCodeCompleter.future, 1);
await verifyCrashReportSent(requestInfo); await verifyCrashReportSent(requestInfo);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
Stdio: () => const _NoStderr(), Stdio: () => const _NoStderr(),
}); });
testUsingContext('should send only one crash report when async throws many', () async {
final Completer<int> exitCodeCompleter = Completer<int>();
setExitFunctionForTests((int exitCode) {
if (!exitCodeCompleter.isCompleted) {
exitCodeCompleter.complete(exitCode);
}
});
final RequestInfo requestInfo = RequestInfo();
final MockCrashReportSender sender = MockCrashReportSender(requestInfo);
CrashReportSender.initializeWith(sender);
FakeAsync().run((FakeAsync time) {
time.elapse(const Duration(seconds: 1));
unawaited(tools.run(
<String>['crash'],
<FlutterCommand>[_MultiCrashAsyncCommand(crashes: 4)],
reportCrashes: true,
flutterVersion: 'test-version',
));
time.elapse(const Duration(seconds: 1));
time.flushMicrotasks();
});
expect(await exitCodeCompleter.future, 1);
expect(MockCrashReportSender.sendCalls, 1);
await verifyCrashReportSent(requestInfo, crashes: 4);
}, overrides: <Type, Generator>{
DoctorValidatorsProvider: () => FakeDoctorValidatorsProvider(),
Stdio: () => const _NoStderr(),
});
testUsingContext('should not send a crash report if on a user-branch', () async { testUsingContext('should not send a crash report if on a user-branch', () async {
String method; String method;
Uri uri; Uri uri;
...@@ -159,7 +193,9 @@ class RequestInfo { ...@@ -159,7 +193,9 @@ class RequestInfo {
Map<String, String> fields; Map<String, String> fields;
} }
Future<void> verifyCrashReportSent(RequestInfo crashInfo) async { Future<void> verifyCrashReportSent(RequestInfo crashInfo, {
int crashes = 1,
}) async {
// Verify that we sent the crash report. // Verify that we sent the crash report.
expect(crashInfo.method, 'POST'); expect(crashInfo.method, 'POST');
expect(crashInfo.uri, Uri( expect(crashInfo.uri, Uri(
...@@ -190,12 +226,13 @@ Future<void> verifyCrashReportSent(RequestInfo crashInfo) async { ...@@ -190,12 +226,13 @@ Future<void> verifyCrashReportSent(RequestInfo crashInfo) async {
final List<String> writtenFiles = final List<String> writtenFiles =
(await tools.crashFileSystem.directory('/').list(recursive: true).toList()) (await tools.crashFileSystem.directory('/').list(recursive: true).toList())
.map((FileSystemEntity e) => e.path).toList(); .map((FileSystemEntity e) => e.path).toList();
expect(writtenFiles, hasLength(1)); expect(writtenFiles, hasLength(crashes));
expect(writtenFiles, contains('flutter_01.log')); expect(writtenFiles, contains('flutter_01.log'));
} }
class MockCrashReportSender extends MockClient { class MockCrashReportSender extends MockClient {
MockCrashReportSender(RequestInfo crashInfo) : super((Request request) async { MockCrashReportSender(RequestInfo crashInfo) : super((Request request) async {
MockCrashReportSender.sendCalls++;
crashInfo.method = request.method; crashInfo.method = request.method;
crashInfo.uri = request.url; crashInfo.uri = request.url;
...@@ -229,6 +266,8 @@ class MockCrashReportSender extends MockClient { ...@@ -229,6 +266,8 @@ class MockCrashReportSender extends MockClient {
200, 200,
); );
}); });
static int sendCalls = 0;
} }
/// Throws a random error to simulate a CLI crash. /// Throws a random error to simulate a CLI crash.
...@@ -278,6 +317,41 @@ class _CrashAsyncCommand extends FlutterCommand { ...@@ -278,6 +317,41 @@ class _CrashAsyncCommand extends FlutterCommand {
} }
} }
/// Generates multiple asynchronous unhandled exceptions.
class _MultiCrashAsyncCommand extends FlutterCommand {
_MultiCrashAsyncCommand({
int crashes = 1,
}) : _crashes = crashes;
final int _crashes;
@override
String get description => 'Simulates a crash';
@override
String get name => 'crash';
@override
Future<FlutterCommandResult> runCommand() async {
for (int i = 0; i < _crashes; i++) {
Timer.run(() {
throw StateError('Test bad state error');
});
}
return Completer<FlutterCommandResult>().future; // expect StateError
}
}
/// A DoctorValidatorsProvider that overrides the default validators without
/// overriding the doctor.
class FakeDoctorValidatorsProvider implements DoctorValidatorsProvider {
@override
List<DoctorValidator> get validators => <DoctorValidator>[];
@override
List<Workflow> get workflows => <Workflow>[];
}
class _NoStderr extends Stdio { class _NoStderr extends Stdio {
const _NoStderr(); const _NoStderr();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment