Use futures list internally to manage send events (#184)

* Refactor to use internal `_futures` list of sent events

* Update tests

* Bump major version to 5.0.0

* Provide delay duration when closing http connection

* Nit fixes from review

* Convert to wip

Author: eliasyishak

pkgs/unified_analytics/CHANGELOG.md

@@ -1,6 +1,7 @@
-## 4.0.2-wip
+## 5.0.0-wip
 
-- Update to the latest version of `package:dart_flutter_team_lints`.
+- Update to the latest version of `package:dart_flutter_team_lints`
+- Using internal futures list to store send events
 
 ## 4.0.1
 

pkgs/unified_analytics/example/unified_analytics_example.dart

@@ -53,8 +53,8 @@ void main() async {
   final hotReloadEvent = Event.hotReloadTime(timeMs: runTime);
 
   // Make a call to the [Analytics] api to send the data
-  await analytics.send(hotReloadEvent);
+  analytics.send(hotReloadEvent);
 
   // Close the client connection on exit
-  analytics.close();
+  await analytics.close();
 }

pkgs/unified_analytics/lib/src/analytics.dart

@@ -225,7 +225,13 @@ abstract class Analytics {
   ///
   /// Prevents the tool from hanging when if there are still requests
   /// that need to be sent off.
-  void close();
+  ///
+  /// Providing [delayDuration] in milliseconds will allow the instance
+  /// to wait the provided time before closing the http connection. Keeping
+  /// the connection open for some time will allow any pending events that
+  /// are waiting to be sent to the Google Analytics server. Default value
+  /// of 250 ms applied.
+  Future<void> close({int delayDuration = kDelayDuration});
 
   /// Method to fetch surveys from the endpoint [kContextualSurveyUrl].
   ///
@@ -248,7 +254,7 @@ abstract class Analytics {
   /// ```dart
   /// analytics.send(Event.memory(periodSec: 123));
   /// ```
-  Future<Response>? send(Event event);
+  void send(Event event);
 
   /// Pass a boolean to either enable or disable telemetry and make
   /// the necessary changes in the persisted configuration file.
@@ -326,6 +332,10 @@ class AnalyticsImpl implements Analytics {
   /// Telemetry suppression flag that is set via [Analytics.suppressTelemetry].
   bool _telemetrySuppressed = false;
 
+  /// The list of futures that will contain all of the send events
+  /// from the [GAClient].
+  final _futures = <Future<Response>>[];
+
   AnalyticsImpl({
     required this.tool,
     required Directory homeDirectory,
@@ -472,7 +482,13 @@ class AnalyticsImpl implements Analytics {
   }
 
   @override
-  void close() => _gaClient.close();
+  Future<void> close({int delayDuration = kDelayDuration}) async {
+    await Future.wait(_futures).timeout(
+      Duration(milliseconds: delayDuration),
+      onTimeout: () => [],
+    );
+    _gaClient.close();
+  }
 
   @override
   Future<List<Survey>> fetchAvailableSurveys() async {
@@ -543,8 +559,8 @@ class AnalyticsImpl implements Analytics {
   LogFileStats? logFileStats() => _logHandler.logFileStats();
 
   @override
-  Future<Response>? send(Event event) {
-    if (!okToSend) return null;
+  void send(Event event) {
+    if (!okToSend) return;
 
     // Construct the body of the request
     final body = generateRequestBody(
@@ -558,8 +574,9 @@ class AnalyticsImpl implements Analytics {
 
     _logHandler.save(data: body);
 
-    // Pass to the google analytics client to send
-    return _gaClient.sendData(body);
+    final gaClientFuture = _gaClient.sendData(body);
+    _futures.add(gaClientFuture);
+    gaClientFuture.whenComplete(() => _futures.remove(gaClientFuture));
   }
 
   @override
@@ -670,8 +687,8 @@ class FakeAnalytics extends AnalyticsImpl {
         );
 
   @override
-  Future<Response>? send(Event event) {
-    if (!okToSend) return null;
+  void send(Event event) {
+    if (!okToSend) return;
 
     // Construct the body of the request
     final body = generateRequestBody(
@@ -688,7 +705,6 @@ class FakeAnalytics extends AnalyticsImpl {
     // Using this list to validate that events are being sent
     // for internal methods in the `Analytics` instance
     sentEvents.add(event);
-    return _gaClient.sendData(body);
   }
 }
 
@@ -730,7 +746,7 @@ class NoOpAnalytics implements Analytics {
   void clientShowedMessage() {}
 
   @override
-  void close() {}
+  Future<void> close({int delayDuration = kDelayDuration}) async {}
 
   @override
   Future<List<Survey>> fetchAvailableSurveys() async => const <Survey>[];

pkgs/unified_analytics/lib/src/constants.dart

@@ -59,6 +59,10 @@ const String kContextualSurveyUrl =
 /// will be located.
 const String kDartToolDirectoryName = '.dart-tool';
 
+/// The default time to wait before closing the http connection to allow for
+/// pending events to be sent.
+const int kDelayDuration = 250;
+
 /// Name of the file where we persist dismissed survey ids.
 const String kDismissedSurveyFileName =
     'dart-flutter-telemetry-dismissed-surveys.json';
@@ -78,7 +82,7 @@ const int kLogFileLength = 2500;
 const String kLogFileName = 'dart-flutter-telemetry.log';
 
 /// The current version of the package, should be in line with pubspec version.
-const String kPackageVersion = '4.0.2-wip';
+const String kPackageVersion = '5.0.0-wip';
 
 /// The minimum length for a session.
 const int kSessionDurationMinutes = 30;

pkgs/unified_analytics/pubspec.yaml

@@ -4,7 +4,7 @@ description: >-
   to Google Analytics.
 # When updating this, keep the version consistent with the changelog and the
 # value in lib/src/constants.dart.
-version: 4.0.2-wip
+version: 5.0.0-wip
 repository: https://github.com/dart-lang/tools/tree/main/pkgs/unified_analytics
 
 environment:

pkgs/unified_analytics/test/log_handler_test.dart

@@ -65,7 +65,7 @@ void main() {
     final countOfEventsToSend = 10;
 
     for (var i = 0; i < countOfEventsToSend; i++) {
-      await analytics.send(testEvent);
+      analytics.send(testEvent);
     }
 
     expect(analytics.logFileStats(), isNotNull);
@@ -91,7 +91,7 @@ void main() {
     final countOfEventsToSend = 10;
 
     for (var i = 0; i < countOfEventsToSend; i++) {
-      await analytics.send(testEvent);
+      analytics.send(testEvent);
     }
     final logFileStats = analytics.logFileStats();
 
@@ -110,7 +110,7 @@ void main() {
     final countOfEventsToSend = 10;
 
     for (var i = 0; i < countOfEventsToSend; i++) {
-      await analytics.send(testEvent);
+      analytics.send(testEvent);
     }
     final logFileStats = analytics.logFileStats();
 
@@ -149,7 +149,7 @@ void main() {
     // one malformed record on top of the logs and the rest
     // are valid log records
     for (var i = 0; i < kLogFileLength - 1; i++) {
-      await analytics.send(testEvent);
+      analytics.send(testEvent);
     }
     final logFileStats = analytics.logFileStats();
     expect(logFile.readAsLinesSync().length, kLogFileLength);
@@ -159,7 +159,7 @@ void main() {
     expect(logFile.readAsLinesSync()[0].trim(), '{{');
 
     // Sending one more event should flush out the malformed record
-    await analytics.send(testEvent);
+    analytics.send(testEvent);
 
     final secondLogFileStats = analytics.logFileStats();
     expect(secondLogFileStats, isNotNull);
@@ -178,7 +178,7 @@ void main() {
     // Ensure it will work as expected after writing correct logs
     final countOfEventsToSend = 10;
     for (var i = 0; i < countOfEventsToSend; i++) {
-      await analytics.send(testEvent);
+      analytics.send(testEvent);
     }
     final secondLogFileStats = analytics.logFileStats();
 

pkgs/unified_analytics/test/suppression_test.dart

@@ -76,7 +76,7 @@ void main() {
 
   test('Suppression works as expected', () async {
     analytics.suppressTelemetry();
-    await analytics.send(testEvent);
+    analytics.send(testEvent);
 
     final logFileStats = analytics.logFileStats();
 
@@ -86,7 +86,7 @@ void main() {
 
   test('Second instance is not suppressed', () async {
     analytics.suppressTelemetry();
-    await analytics.send(testEvent);
+    analytics.send(testEvent);
 
     final logFileStats = analytics.logFileStats();
 
@@ -110,7 +110,7 @@ void main() {
 
     // Using a new event here to differentiate from the first one
     final newEvent = Event.commandExecuted(count: 2, name: 'commandName');
-    await secondAnalytics.send(newEvent);
+    secondAnalytics.send(newEvent);
 
     // Both instances of `Analytics` should now have data retrieved
     // from `LogFileStats()` even though only the second instance

pkgs/unified_analytics/test/survey_handler_test.dart

@@ -358,7 +358,7 @@ void main() {
 
         // Simulate 60 events to send so that the first condition is satisified
         for (var i = 0; i < 60; i++) {
-          await analytics.send(testEvent);
+          analytics.send(testEvent);
         }
 
         final fetchedSurveys = await analytics.fetchAvailableSurveys();
@@ -405,7 +405,7 @@ void main() {
 
         // Simulate 60 events to send so that the first condition is satisified
         for (var i = 0; i < 60; i++) {
-          await analytics.send(testEvent);
+          analytics.send(testEvent);
         }
 
         final fetchedSurveys = await analytics.fetchAvailableSurveys();
@@ -448,7 +448,7 @@ void main() {
 
         // Simulate 60 events to send so that the first condition is satisified
         for (var i = 0; i < 60; i++) {
-          await analytics.send(testEvent);
+          analytics.send(testEvent);
         }
 
         await analytics.setTelemetry(false);
@@ -515,7 +515,7 @@ void main() {
 
         // Simulate 60 events to send so that the first condition is satisified
         for (var i = 0; i < 60; i++) {
-          await analytics.send(testEvent);
+          analytics.send(testEvent);
         }
 
         final fetchedSurveys = await analytics.fetchAvailableSurveys();
@@ -617,7 +617,7 @@ void main() {
 
         // Simulate 60 events to send so that the first condition is satisified
         for (var i = 0; i < 60; i++) {
-          await analytics.send(testEvent);
+          analytics.send(testEvent);
         }
 
         final fetchedSurveys = await analytics.fetchAvailableSurveys();
@@ -686,7 +686,7 @@ void main() {
 
         // Simulate 60 events to send so that the first condition is satisified
         for (var i = 0; i < 60; i++) {
-          await analytics.send(testEvent);
+          analytics.send(testEvent);
         }
 
         final fetchedSurveys = await analytics.fetchAvailableSurveys();
@@ -743,7 +743,7 @@ void main() {
 
         // Simulate 60 events to send so that the first condition is satisified
         for (var i = 0; i < 60; i++) {
-          await analytics.send(testEvent);
+          analytics.send(testEvent);
         }
 
         // Setting to false will prevent anything from getting returned
@@ -758,7 +758,7 @@ void main() {
         // at least 50 records for one of the conditions
         await analytics.setTelemetry(true);
         for (var i = 0; i < 60; i++) {
-          await analytics.send(testEvent);
+          analytics.send(testEvent);
         }
         fetchedSurveys = await analytics.fetchAvailableSurveys();
         expect(fetchedSurveys.length, 1);
@@ -805,7 +805,7 @@ void main() {
 
         // Simulate 60 events to send so that the first condition is satisified
         for (var i = 0; i < 60; i++) {
-          await analytics.send(testEvent);
+          analytics.send(testEvent);
         }
 
         final fetchedSurveys = await analytics.fetchAvailableSurveys();
@@ -851,7 +851,7 @@ void main() {
 
         // Simulate 60 events to send so that the first condition is satisified
         for (var i = 0; i < 60; i++) {
-          await analytics.send(testEvent);
+          analytics.send(testEvent);
         }
 
         final fetchedSurveys = await analytics.fetchAvailableSurveys();