fix(cli_tools): Prevent analytics send failures from crashing the host process (#125)

Make both providers' `sendEvent` properly async so they return reified
`Future<void>`s, and harden `Analytics.track` to swallow errors with
try/catch instead of `catchError`, so implementations returning foreign
futures cannot crash the process.

Author: nielsenko

packages/cli_tools/lib/src/analytics/analytics.dart

@@ -21,12 +21,21 @@ abstract class Analytics {
     final Map<String, dynamic> properties = const {},
   }) {
     late final Future<void> 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<void> _quietSendEvent({
+    required final String event,
+    required final Map<String, dynamic> properties,
+  }) async {
+    try {
+      await sendEvent(event: event, properties: properties);
+    } catch (_) {}
+  }
+
   /// Send an event to the analytics service.
   Future<void> sendEvent({
     required final String event,

packages/cli_tools/lib/src/analytics/mixpanel.dart

@@ -55,7 +55,7 @@ class MixPanelAnalytics extends Analytics {
   Future<void> sendEvent({
     required final String event,
     final Map<String, dynamic> 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: {

packages/cli_tools/lib/src/analytics/posthog.dart

@@ -39,7 +39,7 @@ class PostHogAnalytics extends Analytics {
   Future<void> sendEvent({
     required final String event,
     final Map<String, dynamic> 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'},

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<void>`.
+///
+/// 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<void>`. This is just one pitfall
+/// of many.
+class NonVoidFutureAnalytics extends Analytics {
+  final completers = <Completer<String>>[];
+
+  @override
+  Future<void> sendEvent /* no async */ ({
+    required final String event,
+    final Map<String, dynamic> properties = const {},
+  }) {
+    // note the mismatch Future<void> vs Future<String>
+    final completer = Completer<String>();
+    completers.add(completer);
+    return completer.future; // no await
+  }
+}
+
 class FailingFlushAnalytics extends Analytics {
   var didFlush = false;