diff --git a/packages/flutter_tools/lib/runner.dart b/packages/flutter_tools/lib/runner.dart index 75c1090f48e..4e89497c08d 100644 --- a/packages/flutter_tools/lib/runner.dart +++ b/packages/flutter_tools/lib/runner.dart @@ -46,6 +46,9 @@ Future 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(() async { @@ -149,6 +152,13 @@ Future 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? _alreadyHandlingToolError; + Future _handleToolError( Object error, StackTrace? stackTrace, @@ -159,6 +169,30 @@ Future _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 _handleToolErrorImpl( + Object error, + StackTrace? stackTrace, + bool verbose, + List args, + bool reportCrashes, + String Function() getFlutterVersion, + ShutdownHooks shutdownHooks, { + required bool usingLocalEngine, + required FeatureFlags featureFlags, }) async { if (error is UsageException) { globals.printError('${error.message}\n'); diff --git a/packages/flutter_tools/test/general.shard/runner/runner_test.dart b/packages/flutter_tools/test/general.shard/runner/runner_test.dart index dfe6f03cef4..7e4a7e1b9ee 100644 --- a/packages/flutter_tools/test/general.shard/runner/runner_test.dart +++ b/packages/flutter_tools/test/general.shard/runner/runner_test.dart @@ -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?>( + () { + unawaited( + runner.run( + ['crash'], + () => [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 exceptionEvents = fakeAnalytics.sentEvents + .where((e) => e.eventName == DashEvent.exception) + .toList(); + expect(exceptionEvents, hasLength(1)); + }, + overrides: { + Analytics: () => fakeAnalytics, + Platform: () => FakePlatform( + environment: {'FLUTTER_ANALYTICS_LOG_FILE': 'test', 'FLUTTER_ROOT': '/'}, + ), + FileSystem: () => fileSystem, + ProcessManager: () => FakeProcessManager.any(), + CrashReporter: () => WaitingCrashReporter(Future.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(); + + @override + String get description => ''; + + @override + String get name => 'crash'; + + Future get doneThrowing => _completer.future; + + var exceptionCount = 0; + + @override + Future 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 => '';