Skip to content

Commit

Permalink
[flutter_tool] Only send one crash report per run (#38925)
Browse files Browse the repository at this point in the history
  • Loading branch information
zanderso committed Aug 21, 2019
1 parent ea64b84 commit 36e8b93
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 34 deletions.
63 changes: 32 additions & 31 deletions packages/flutter_tools/lib/runner.dart
Expand Up @@ -115,39 +115,40 @@ Future<int> _handleToolError(
stderr.writeln('$error');
stderr.writeln(stackTrace.toString());
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)
stderr.writeln('Oops; flutter has exited unexpectedly: "$error".');
else
stderr.writeln('Oops; flutter has exited unexpectedly.');
// Report to both [Usage] and [CrashReportSender].
flutterUsage.sendException(error);
await CrashReportSender.instance.sendReport(
error: error,
stackTrace: stackTrace,
getFlutterVersion: getFlutterVersion,
command: args.join(' '),
);

try {
final File file = await _createLocalCrashReport(args, error, stackTrace);
stderr.writeln(
'Crash report written to ${file.path};\n'
'please let us know at https://github.com/flutter/flutter/issues.',
);
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.',
);
// 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);
}
if (error is String) {
stderr.writeln('Oops; flutter has exited unexpectedly: "$error".');
} else {
stderr.writeln('Oops; flutter has exited unexpectedly.');
}

try {
final File file = await _createLocalCrashReport(args, error, stackTrace);
stderr.writeln(
'Crash report written to ${file.path};\n'
'please let us know at https://github.com/flutter/flutter/issues.',
);
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.',
);
// 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);
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions packages/flutter_tools/lib/src/reporting/crash_reporting.dart
Expand Up @@ -44,6 +44,8 @@ class CrashReportSender {

static CrashReportSender get instance => _instance ?? CrashReportSender._(http.Client());

bool _crashReportSent = false;

/// Overrides the default [http.Client] with [client] for testing purposes.
@visibleForTesting
static void initializeWith(http.Client client) {
Expand Down Expand Up @@ -76,6 +78,10 @@ class CrashReportSender {
@required String getFlutterVersion(),
@required String command,
}) async {
// Only send one crash report per run.
if (_crashReportSent) {
return;
}
try {
final String flutterVersion = getFlutterVersion();

Expand Down Expand Up @@ -116,6 +122,7 @@ class CrashReportSender {
final String reportId = await http.ByteStream(resp.stream)
.bytesToString();
printStatus('Crash report sent (report ID: $reportId)');
_crashReportSent = true;
} else {
printError('Failed to send crash report. Server responded with HTTP status code ${resp.statusCode}');
}
Expand Down
Expand Up @@ -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/platform.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/runner/flutter_command.dart';
import 'package:http/http.dart';
import 'package:http/testing.dart';
import 'package:pedantic/pedantic.dart';
import 'package:quiver/testing/async.dart';

import '../src/common.dart';
import '../src/context.dart';
Expand All @@ -32,6 +34,7 @@ void main() {
setUp(() async {
tools.crashFileSystem = MemoryFileSystem();
setExitFunctionForTests((_) { });
MockCrashReportSender.sendCalls = 0;
});

tearDown(() {
Expand Down Expand Up @@ -72,12 +75,43 @@ void main() {
reportCrashes: true,
flutterVersion: 'test-version',
));
expect(await exitCodeCompleter.future, equals(1));
expect(await exitCodeCompleter.future, 1);
await verifyCrashReportSent(requestInfo);
}, overrides: <Type, Generator>{
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 {
String method;
Uri uri;
Expand Down Expand Up @@ -159,7 +193,9 @@ class RequestInfo {
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.
expect(crashInfo.method, 'POST');
expect(crashInfo.uri, Uri(
Expand Down Expand Up @@ -190,12 +226,13 @@ Future<void> verifyCrashReportSent(RequestInfo crashInfo) async {
final List<String> writtenFiles =
(await tools.crashFileSystem.directory('/').list(recursive: true).toList())
.map((FileSystemEntity e) => e.path).toList();
expect(writtenFiles, hasLength(1));
expect(writtenFiles, hasLength(crashes));
expect(writtenFiles, contains('flutter_01.log'));
}

class MockCrashReportSender extends MockClient {
MockCrashReportSender(RequestInfo crashInfo) : super((Request request) async {
MockCrashReportSender.sendCalls++;
crashInfo.method = request.method;
crashInfo.uri = request.url;

Expand Down Expand Up @@ -229,6 +266,8 @@ class MockCrashReportSender extends MockClient {
200,
);
});

static int sendCalls = 0;
}

/// Throws a random error to simulate a CLI crash.
Expand Down Expand Up @@ -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 {
const _NoStderr();

Expand Down

0 comments on commit 36e8b93

Please sign in to comment.