From cec3bdfb711eb654bb1d3e8fd10e16e39b21d28f Mon Sep 17 00:00:00 2001 From: flutteractionsbot <154381524+flutteractionsbot@users.noreply.github.com> Date: Tue, 18 Nov 2025 07:12:40 -0800 Subject: [PATCH] [CP-beta][ Tool ] Only process a single unhandled tool exception (#178470) This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request) Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request. ### Issue Link: What is the link to the issue this cherry-pick is addressing? https://github.com/flutter/flutter/issues/178318 ### Changelog Description: Explain this cherry pick in one line that is accessible to most Flutter developers. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples Certain `flutter` crash scenarios can result in multiple crash reports being submitted for a single process crash. ### Impact Description: What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping production app (the app crashes on launch) Crash analytics data can be skewed to make certain bugs appear much more severe then they actually are. ### Workaround: Is there a workaround for this issue? Don't crash? :) ### Risk: What is the risk level of this cherry-pick? ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: What are the steps to validate that this fix works? Run attached unit test. --- packages/flutter_tools/lib/runner.dart | 34 +++++++ .../general.shard/runner/runner_test.dart | 90 +++++++++++++++++++ 2 files changed, 124 insertions(+) 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 => '';