mirror of
https://github.com/flutter/flutter.git
synced 2026-02-20 02:29:02 +08:00
[ Tool ] Only process a single unhandled tool exception (#178335)
Without this change, if multiple asynchronous exceptions are thrown while processing an exception, multiple exception analytics events can be sent for a single process crash, skewing crashlytics data. Fixes https://github.com/flutter/flutter/issues/178318
This commit is contained in:
parent
702ca6735c
commit
162bf57347
@ -46,6 +46,9 @@ Future<int> run(
|
||||
args.removeWhere((String option) => option == '-vv' || option == '-v' || option == '--verbose');
|
||||
}
|
||||
|
||||
// Reset this on each run to ensure we don't leak state across tests.
|
||||
_alreadyHandlingToolError = null;
|
||||
|
||||
final bool usingLocalEngine = args.any((a) => a.startsWith('--local-engine'));
|
||||
|
||||
return runInContext<int>(() async {
|
||||
@ -149,6 +152,13 @@ Future<int> run(
|
||||
}, overrides: overrides);
|
||||
}
|
||||
|
||||
/// Track if we're actively processing an error so we don't try and process
|
||||
/// additional asynchronous exceptions while we're trying to shut down.
|
||||
///
|
||||
/// NOTE: This state is cleared at the beginning of [run] to ensure state
|
||||
/// doesn't leak when running tests.
|
||||
Future<int>? _alreadyHandlingToolError;
|
||||
|
||||
Future<int> _handleToolError(
|
||||
Object error,
|
||||
StackTrace? stackTrace,
|
||||
@ -159,6 +169,30 @@ Future<int> _handleToolError(
|
||||
ShutdownHooks shutdownHooks, {
|
||||
required bool usingLocalEngine,
|
||||
required FeatureFlags featureFlags,
|
||||
}) async {
|
||||
return _alreadyHandlingToolError ??= _handleToolErrorImpl(
|
||||
error,
|
||||
stackTrace,
|
||||
verbose,
|
||||
args,
|
||||
reportCrashes,
|
||||
getFlutterVersion,
|
||||
shutdownHooks,
|
||||
usingLocalEngine: usingLocalEngine,
|
||||
featureFlags: featureFlags,
|
||||
);
|
||||
}
|
||||
|
||||
Future<int> _handleToolErrorImpl(
|
||||
Object error,
|
||||
StackTrace? stackTrace,
|
||||
bool verbose,
|
||||
List<String> args,
|
||||
bool reportCrashes,
|
||||
String Function() getFlutterVersion,
|
||||
ShutdownHooks shutdownHooks, {
|
||||
required bool usingLocalEngine,
|
||||
required FeatureFlags featureFlags,
|
||||
}) async {
|
||||
if (error is UsageException) {
|
||||
globals.printError('${error.message}\n');
|
||||
|
||||
@ -23,6 +23,7 @@ import 'package:flutter_tools/src/globals.dart' as globals;
|
||||
import 'package:flutter_tools/src/reporting/crash_reporting.dart';
|
||||
import 'package:flutter_tools/src/runner/flutter_command.dart';
|
||||
import 'package:test/fake.dart';
|
||||
import 'package:unified_analytics/testing.dart';
|
||||
import 'package:unified_analytics/unified_analytics.dart';
|
||||
|
||||
import '../../src/common.dart';
|
||||
@ -289,6 +290,67 @@ void main() {
|
||||
},
|
||||
);
|
||||
|
||||
testUsingContext(
|
||||
"doesn't send multiple events for additional asynchronous exceptions "
|
||||
'thrown during shutdown',
|
||||
() async {
|
||||
// Regression test for https://github.com/flutter/flutter/issues/178318.
|
||||
final command = MultipleExceptionCrashingFlutterCommand();
|
||||
var exceptionCount = 0;
|
||||
unawaited(
|
||||
runZonedGuarded<Future<void>?>(
|
||||
() {
|
||||
unawaited(
|
||||
runner.run(
|
||||
<String>['crash'],
|
||||
() => <FlutterCommand>[command],
|
||||
// This flutterVersion disables crash reporting.
|
||||
flutterVersion: '[user-branch]/',
|
||||
reportCrashes: true,
|
||||
shutdownHooks: ShutdownHooks(),
|
||||
),
|
||||
);
|
||||
return null;
|
||||
},
|
||||
(Object error, StackTrace stack) {
|
||||
// Keep track of the number of exceptions thrown to ensure that
|
||||
// the count matches the number of exceptions we expect.
|
||||
exceptionCount++;
|
||||
},
|
||||
),
|
||||
);
|
||||
await command.doneThrowing;
|
||||
|
||||
// This is the main check of this test.
|
||||
//
|
||||
// We are checking that, even though multiple asynchronous errors were
|
||||
// thrown, only a single crash report is sent. This ensures that a
|
||||
// single process crash can't result in multiple crash events.
|
||||
|
||||
// This test only makes sense if we've thrown more than one exception.
|
||||
expect(exceptionCount, greaterThan(1));
|
||||
expect(exceptionCount, command.exceptionCount);
|
||||
|
||||
// Ensure only a single exception analytics event was sent.
|
||||
final List<Event> exceptionEvents = fakeAnalytics.sentEvents
|
||||
.where((e) => e.eventName == DashEvent.exception)
|
||||
.toList();
|
||||
expect(exceptionEvents, hasLength(1));
|
||||
},
|
||||
overrides: <Type, Generator>{
|
||||
Analytics: () => fakeAnalytics,
|
||||
Platform: () => FakePlatform(
|
||||
environment: <String, String>{'FLUTTER_ANALYTICS_LOG_FILE': 'test', 'FLUTTER_ROOT': '/'},
|
||||
),
|
||||
FileSystem: () => fileSystem,
|
||||
ProcessManager: () => FakeProcessManager.any(),
|
||||
CrashReporter: () => WaitingCrashReporter(Future<void>.value()),
|
||||
Artifacts: () => Artifacts.test(),
|
||||
HttpClientFactory: () =>
|
||||
() => FakeHttpClient.any(),
|
||||
},
|
||||
);
|
||||
|
||||
testUsingContext(
|
||||
'create local report',
|
||||
() async {
|
||||
@ -819,6 +881,34 @@ class CrashingFlutterCommand extends FlutterCommand {
|
||||
}
|
||||
}
|
||||
|
||||
class MultipleExceptionCrashingFlutterCommand extends FlutterCommand {
|
||||
final _completer = Completer<void>();
|
||||
|
||||
@override
|
||||
String get description => '';
|
||||
|
||||
@override
|
||||
String get name => 'crash';
|
||||
|
||||
Future<void> get doneThrowing => _completer.future;
|
||||
|
||||
var exceptionCount = 0;
|
||||
|
||||
@override
|
||||
Future<FlutterCommandResult> runCommand() async {
|
||||
Timer.periodic(const Duration(milliseconds: 10), (timer) {
|
||||
exceptionCount++;
|
||||
if (exceptionCount < 5) {
|
||||
throw Exception('ERROR: $exceptionCount');
|
||||
}
|
||||
timer.cancel();
|
||||
_completer.complete();
|
||||
});
|
||||
|
||||
return FlutterCommandResult.success();
|
||||
}
|
||||
}
|
||||
|
||||
class _GitNotFoundFlutterCommand extends FlutterCommand {
|
||||
@override
|
||||
String get description => '';
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user