Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Aug 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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