From e3a0cd9247bfb353692ab5b43adfb00e4a28571e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20Overg=C3=A5rd=20Nielsen?= Date: Thu, 11 Jun 2026 12:49:55 +0200 Subject: [PATCH] fix(cli_tools): Prevent analytics send failures from crashing the host process Make both providers' `sendEvent` properly async so they return reified `Future`s, and harden `Analytics.track` to swallow errors with try/catch instead of `catchError`, so implementations returning foreign futures cannot crash the process. --- .../lib/src/analytics/analytics.dart | 13 +++++- .../cli_tools/lib/src/analytics/mixpanel.dart | 4 +- .../cli_tools/lib/src/analytics/posthog.dart | 4 +- .../test/analytics/analytics_test.dart | 40 +++++++++++++++++++ 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/packages/cli_tools/lib/src/analytics/analytics.dart b/packages/cli_tools/lib/src/analytics/analytics.dart index 694d8e1..6633025 100644 --- a/packages/cli_tools/lib/src/analytics/analytics.dart +++ b/packages/cli_tools/lib/src/analytics/analytics.dart @@ -21,12 +21,21 @@ abstract class Analytics { final Map properties = const {}, }) { late final Future pendingTrack; - pendingTrack = sendEvent(event: event, properties: properties) - .catchError((final _) {}) + pendingTrack = _quietSendEvent(event: event, properties: properties) .whenComplete(() => _pendingTracks.remove(pendingTrack)); _pendingTracks.add(pendingTrack); } + /// Sends an event, swallowing any errors. + Future _quietSendEvent({ + required final String event, + required final Map properties, + }) async { + try { + await sendEvent(event: event, properties: properties); + } catch (_) {} + } + /// Send an event to the analytics service. Future sendEvent({ required final String event, diff --git a/packages/cli_tools/lib/src/analytics/mixpanel.dart b/packages/cli_tools/lib/src/analytics/mixpanel.dart index 8f112d6..c3ff0fc 100644 --- a/packages/cli_tools/lib/src/analytics/mixpanel.dart +++ b/packages/cli_tools/lib/src/analytics/mixpanel.dart @@ -55,7 +55,7 @@ class MixPanelAnalytics extends Analytics { Future sendEvent({ required final String event, final Map properties = const {}, - }) { + }) async { final payload = jsonEncode({ 'event': event, 'properties': { @@ -68,7 +68,7 @@ class MixPanelAnalytics extends Analytics { }, }); - return http.post( + await http.post( _endpoint, body: 'data=$payload', headers: { diff --git a/packages/cli_tools/lib/src/analytics/posthog.dart b/packages/cli_tools/lib/src/analytics/posthog.dart index fcbbd43..339ccad 100644 --- a/packages/cli_tools/lib/src/analytics/posthog.dart +++ b/packages/cli_tools/lib/src/analytics/posthog.dart @@ -39,7 +39,7 @@ class PostHogAnalytics extends Analytics { Future sendEvent({ required final String event, final Map properties = const {}, - }) { + }) async { final eventData = { 'api_key': _projectApiKey, 'event': event, @@ -54,7 +54,7 @@ class PostHogAnalytics extends Analytics { }, }; - return http + await http .post( _endpoint, headers: {'Content-Type': 'application/json'}, diff --git a/packages/cli_tools/test/analytics/analytics_test.dart b/packages/cli_tools/test/analytics/analytics_test.dart index 6393623..0a6b9e2 100644 --- a/packages/cli_tools/test/analytics/analytics_test.dart +++ b/packages/cli_tools/test/analytics/analytics_test.dart @@ -48,6 +48,26 @@ void main() { }, ); + test( + 'Given an analytics implementation whose sendEvent returns a future with a non-void reified type, ' + 'when the event send fails, ' + 'then the failure is ignored and flush completes.', + () async { + final analytics = NonVoidFutureAnalytics(); + + analytics.track(event: 'test'); + + final flush = expectLater(analytics.flush(), completes); + await flushEventQueue(); + + analytics.completers.single.completeError( + StateError('Failed to send event.'), + ); + + await expectLater(flush, completes); + }, + ); + test( 'Given compound analytics with a provider that fails to flush, ' 'when flushing the compound analytics, ' @@ -79,6 +99,26 @@ class PendingAnalytics extends Analytics { } } +/// Returns futures from [sendEvent] whose reified type is not `Future`. +/// +/// This an example of how NOT to do it. In general don't try to be smart and +/// skip the async on a method returning `Future`. This is just one pitfall +/// of many. +class NonVoidFutureAnalytics extends Analytics { + final completers = >[]; + + @override + Future sendEvent /* no async */ ({ + required final String event, + final Map properties = const {}, + }) { + // note the mismatch Future vs Future + final completer = Completer(); + completers.add(completer); + return completer.future; // no await + } +} + class FailingFlushAnalytics extends Analytics { var didFlush = false;