Hotfix analytics 5.8.8+1 (#265)

* Hotfix 5.8.8+1 (#264)

* Use new private `_sendError` for error handler

* Prep publishing

* Update README to include diff

* Update diff link

* Use current timestamp for session id

* Fix typo

* Remove session id from user properties

* Fix test + skip tests that check logfile writes

* Fix tests + update README with release notes for tests

Author: eliasyishak

pkgs/unified_analytics/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 5.8.8+1
+
+- Edit to error handler to not use default `Analytic.send` method and use new `Analytics._sendError` method that doesn't create a session id
+
 ## 5.8.8
 
 - [Bug fix](https://github.com/dart-lang/tools/issues/252) rewrite the other call site for the session file

pkgs/unified_analytics/README.md

@@ -2,6 +2,12 @@
 [![pub package](https://img.shields.io/pub/v/unified_analytics.svg)](https://pub.dev/packages/unified_analytics)
 [![package publisher](https://img.shields.io/pub/publisher/unified_analytics.svg)](https://pub.dev/packages/unified_analytics/publisher)
 
+## Hotfix `5.8.8+1` special release for Flutter 3.22
+
+[Diff](https://github.com/dart-lang/tools/compare/unified_analytics-v5.8.8...hotfix-analytics-5.8.8+1?expand=1) between `5.8.8` and this hotfix release.
+
+This release also includes altered tests for log file related tests. This release changes the functionality for error events by not persisting them locally.
+
 ## What's this?
 
 This package is intended to be used on Dart and Flutter related

pkgs/unified_analytics/lib/src/analytics.dart

@@ -4,6 +4,7 @@
 
 import 'dart:io' as io;
 
+import 'package:clock/clock.dart';
 import 'package:file/file.dart';
 import 'package:file/local.dart';
 import 'package:file/memory.dart';
@@ -420,7 +421,7 @@ class AnalyticsImpl implements Analytics {
 
     // Initialization for the error handling class that will prevent duplicate
     // [Event.analyticsException] events from being sent to GA4
-    _errorHandler = ErrorHandler(sendFunction: send);
+    _errorHandler = ErrorHandler(sendFunction: _sendError);
 
     // Initialize the user property class that will be attached to
     // each event that is sent to Google Analytics -- it will be responsible
@@ -706,6 +707,45 @@ class AnalyticsImpl implements Analytics {
     _surveyHandler.dismiss(survey, false);
     send(Event.surveyShown(surveyId: survey.uniqueId));
   }
+
+  /// Private method for sending an instance of [Event] that does not create
+  /// a session id, which was found to cause a stack overflow error.
+  void _sendError(Event event) {
+    if (!okToSend) return;
+
+    // Construct the body of the request
+    //
+    // This method omits the session id from the user property payload so that
+    // it doesn't skew session length calculations
+    final body = {
+      'client_id': _clientId,
+      'events': <Map<String, Object?>>[
+        <String, Object?>{
+          'name': event.eventName.label,
+          'params': event.eventData,
+        }
+      ],
+      'user_properties': <String, Map<String, Object?>>{
+        'flutter_channel': {'value': userProperty.flutterChannel},
+        'flutter_version': {'value': userProperty.flutterVersion},
+        'host': {'value': userProperty.host},
+        'dart_version': {'value': userProperty.dartVersion},
+        'analytics_pkg_version': {'value': kPackageVersion},
+        'tool': {'value': userProperty.tool},
+        'local_time': {'value': formatDateTime(clock.now())},
+        'host_os_version': {'value': userProperty.hostOsVersion},
+        'locale': {'value': userProperty.locale},
+        'client_ide': {'value': userProperty.clientIde},
+        'enabled_features': {'value': userProperty.enabledFeatures},
+      },
+    };
+
+    if (_enableAsserts) checkBody(body);
+
+    final gaClientFuture = _gaClient.sendData(body);
+    _futures.add(gaClientFuture);
+    gaClientFuture.whenComplete(() => _futures.remove(gaClientFuture));
+  }
 }
 
 /// This fake instance of [Analytics] is intended to be used by clients of
@@ -760,6 +800,12 @@ class FakeAnalytics extends AnalyticsImpl {
     // for internal methods in the `Analytics` instance
     sentEvents.add(event);
   }
+
+  @override
+  void _sendError(Event event) {
+    super._sendError(event);
+    sentEvents.add(event);
+  }
 }
 
 /// An implementation that will never send events.

pkgs/unified_analytics/lib/src/constants.dart

@@ -82,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 = '5.8.8';
+const String kPackageVersion = '5.8.8+1';
 
 /// 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: 5.8.8
+version: 5.8.8+1
 repository: https://github.com/dart-lang/tools/tree/main/pkgs/unified_analytics
 
 environment:

pkgs/unified_analytics/test/error_handler_test.dart

@@ -259,7 +259,10 @@ void main() {
             (element) => element.eventName == DashEvent.analyticsException),
         hasLength(1),
       );
-      expect(logFile.readAsLinesSync(), hasLength(4));
+
+      // Removing this test case because we are no longer writing error events
+      // to the locally persisted log file
+      // expect(logFile.readAsLinesSync(), hasLength(4));
     });
 
     test('only sends one event for TypeError', () {

pkgs/unified_analytics/test/log_handler_test.dart

@@ -126,10 +126,13 @@ void main() {
     expect(logFile.readAsLinesSync().length,
         countOfEventsToSend + countOfMalformedRecords);
     final logFileStats = analytics.logFileStats();
-    expect(logFile.readAsLinesSync().length,
-        countOfEventsToSend + countOfMalformedRecords + 1,
-        reason:
-            'There should have been on error event sent when getting stats');
+
+    // Removing this test case because we are no longer writing error events
+    // to the locally persisted log file
+    // expect(logFile.readAsLinesSync().length,
+    //     countOfEventsToSend + countOfMalformedRecords + 1,
+    //     reason:
+    //         'There should have been on error event sent when getting stats');
 
     expect(logFileStats, isNotNull);
     expect(logFileStats!.recordCount, countOfEventsToSend);
@@ -172,9 +175,13 @@ void main() {
       analytics.send(testEvent);
     }
     final logFileStats = analytics.logFileStats();
-    expect(analytics.sentEvents.last.eventName, DashEvent.analyticsException,
-        reason: 'Calling for the stats should have caused an error');
-    expect(logFile.readAsLinesSync().length, kLogFileLength);
+
+    // Removing this test case because we are no longer writing error events
+    // to the locally persisted log file
+    // expect(analytics.sentEvents.last.eventName, DashEvent.analyticsException,
+    //     reason: 'Calling for the stats should have caused an error');
+    // expect(logFile.readAsLinesSync().length, kLogFileLength);
+
     expect(logFileStats, isNotNull);
     expect(logFileStats!.recordCount, recordsToSendInitially,
         reason: 'The first record should be malformed');
@@ -186,8 +193,11 @@ void main() {
     final secondLogFileStats = analytics.logFileStats();
     expect(analytics.sentEvents.last, testEvent);
     expect(secondLogFileStats, isNotNull);
-    expect(secondLogFileStats!.recordCount, kLogFileLength);
-    expect(logFile.readAsLinesSync()[0].trim(), isNot('{{'));
+    expect(secondLogFileStats!.recordCount, kLogFileLength - 1);
+
+    // Removing this test case because we are no longer writing error events
+    // to the locally persisted log file
+    // expect(logFile.readAsLinesSync()[0].trim(), isNot('{{'));
   });
 
   test('Catching cast errors for each log record silently', () async {
@@ -206,9 +216,12 @@ void main() {
     final secondLogFileStats = analytics.logFileStats();
 
     expect(secondLogFileStats, isNotNull);
-    expect(secondLogFileStats!.recordCount, countOfEventsToSend + 1,
-        reason: 'Plus one for the error event that is sent '
-            'from the first logFileStats call');
+
+    // Removing this test case because we are no longer writing error events
+    // to the locally persisted log file
+    // expect(secondLogFileStats!.recordCount, countOfEventsToSend + 1,
+    //     reason: 'Plus one for the error event that is sent '
+    //         'from the first logFileStats call');
   });
 
   test(