Fix late initialization error for Analytics.userProperty (#239)

* Initialize session handler at end of constructor

* Remove redundant standalone `UserProperty` from test

* Bump versions

* `dart format .`

* Cast to non-null in `expect`

pkgs/unified_analytics/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 5.8.5
+
+- Fix late initialization error for `Analytics.userProperty` [bug](https://github.com/dart-lang/tools/issues/238)
+
 ## 5.8.4
 
 - Exporting all enums from [`enums.dart`](https://github.com/dart-lang/tools/blob/main/pkgs/unified_analytics/lib/src/enums.dart) through `lib/testing.dart`

pkgs/unified_analytics/lib/src/analytics.dart

@@ -430,7 +430,6 @@ class AnalyticsImpl implements Analytics {
       homeDirectory: homeDirectory,
       fs: fs,
       errorHandler: _errorHandler,
-      telemetryEnabled: telemetryEnabled,
     );
     userProperty = UserProperty(
       session: _sessionHandler,
@@ -454,6 +453,10 @@ class AnalyticsImpl implements Analytics {
       homeDirectory: homeDirectory,
       errorHandler: _errorHandler,
     );
+
+    // Initialize the session handler with the session_id and last_ping
+    // variables by parsing the json file
+    _sessionHandler.initialize(telemetryEnabled);
   }
 
   @override

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.4';
+const String kPackageVersion = '5.8.5';
 
 /// The minimum length for a session.
 const int kSessionDurationMinutes = 30;

pkgs/unified_analytics/lib/src/session.dart

@@ -26,16 +26,9 @@ class Session {
     required this.homeDirectory,
     required this.fs,
     required ErrorHandler errorHandler,
-    required bool telemetryEnabled,
   })  : sessionFile = fs.file(p.join(
             homeDirectory.path, kDartToolDirectoryName, kSessionFileName)),
-        _errorHandler = errorHandler {
-    // We must check if telemetry is enabled to refresh the session data
-    // because the refresh method will write to the session file and for
-    // users that have opted out, we have to leave the session file empty
-    // per the privacy document
-    if (telemetryEnabled) _refreshSessionData();
-  }
+        _errorHandler = errorHandler;
 
   /// This will use the data parsed from the
   /// session json file in the dart-tool directory
@@ -70,6 +63,16 @@ class Session {
     return _sessionId;
   }
 
+  /// Preps the [Session] class with the data found in the session json file.
+  ///
+  /// We must check if telemetry is enabled to refresh the session data
+  /// because the refresh method will write to the session file and for
+  /// users that have opted out, we have to leave the session file empty
+  /// per the privacy document
+  void initialize(bool telemetryEnabled) {
+    if (telemetryEnabled) _refreshSessionData();
+  }
+
   /// Return a json formatted representation of the class.
   String toJson() => jsonEncode(<String, int>{
         'session_id': _sessionId,

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.4
+version: 5.8.5
 repository: https://github.com/dart-lang/tools/tree/main/pkgs/unified_analytics
 
 environment:

pkgs/unified_analytics/test/unified_analytics_test.dart

@@ -14,9 +14,6 @@ import 'package:file/memory.dart';
 import 'package:test/test.dart';
 import 'package:unified_analytics/src/constants.dart';
 import 'package:unified_analytics/src/enums.dart';
-import 'package:unified_analytics/src/error_handler.dart';
-import 'package:unified_analytics/src/session.dart';
-import 'package:unified_analytics/src/user_property.dart';
 import 'package:unified_analytics/src/utils.dart';
 import 'package:unified_analytics/unified_analytics.dart';
 import 'package:yaml/yaml.dart';
@@ -32,7 +29,6 @@ void main() {
   late File configFile;
   late File logFile;
   late File dismissedSurveyFile;
-  late UserProperty userProperty;
 
   const homeDirName = 'home';
   const initialTool = DashTool.flutterTool;
@@ -45,10 +41,7 @@ void main() {
   const flutterVersion = 'flutterVersion';
   const dartVersion = 'dartVersion';
   const platform = DevicePlatform.macos;
-  const hostOsVersion = 'Version 14.1 (Build 23B74)';
-  const locale = 'en';
   const clientIde = 'VSCode';
-  const enabledFeatures = 'enable-linux-desktop,cli-animations';
 
   final testEvent = Event.hotReloadTime(timeMs: 50);
 
@@ -113,26 +106,6 @@ void main() {
     dismissedSurveyFile = home
         .childDirectory(kDartToolDirectoryName)
         .childFile(kDismissedSurveyFileName);
-
-    // Create the user property object that is also
-    // created within analytics for testing
-    userProperty = UserProperty(
-      session: Session(
-        homeDirectory: home,
-        fs: fs,
-        errorHandler: ErrorHandler(sendFunction: analytics.send),
-        telemetryEnabled: analytics.telemetryEnabled,
-      ),
-      flutterChannel: flutterChannel,
-      host: platform.label,
-      flutterVersion: flutterVersion,
-      dartVersion: dartVersion,
-      tool: initialTool.label,
-      hostOsVersion: hostOsVersion,
-      locale: locale,
-      clientIde: clientIde,
-      enabledFeatures: enabledFeatures,
-    );
   });
 
   test('Initializer properly sets up on first run', () {
@@ -169,7 +142,7 @@ void main() {
     withClock(Clock.fixed(start), () {
       final timestamp = clock.now().millisecondsSinceEpoch.toString();
       expect(sessionFile.readAsStringSync(), 'contents');
-      userProperty.preparePayload();
+      analytics.userProperty.preparePayload();
       expect(sessionFile.readAsStringSync(),
           '{"session_id":$timestamp,"last_ping":$timestamp}');
 
@@ -183,6 +156,38 @@ void main() {
     });
   });
 
+  test('Handles malformed session file on startup', () {
+    // Ensure that we are able to send an error message on startup if
+    // we encounter an error while parsing the contents of the json file
+    // for session data
+    sessionFile.writeAsStringSync('contents');
+
+    analytics = Analytics.test(
+      tool: initialTool,
+      homeDirectory: home,
+      measurementId: measurementId,
+      apiSecret: apiSecret,
+      flutterChannel: flutterChannel,
+      toolsMessageVersion: toolsMessageVersion,
+      toolsMessage: toolsMessage,
+      flutterVersion: flutterVersion,
+      dartVersion: dartVersion,
+      fs: fs,
+      platform: platform,
+      clientIde: clientIde,
+    ) as FakeAnalytics;
+    analytics.clientShowedMessage();
+
+    final errorEvent = analytics.sentEvents
+        .where((element) => element.eventName == DashEvent.analyticsException)
+        .firstOrNull;
+    expect(errorEvent, isNotNull);
+    expect(errorEvent!.eventData['workflow'], 'Session._refreshSessionData');
+    expect(errorEvent.eventData['error'], 'FormatException');
+    expect(errorEvent.eventData['description'],
+        'message: Unexpected character\nsource: contents');
+  });
+
   test('Resetting session file when file is removed', () {
     // Purposefully write delete the file
     sessionFile.deleteSync();
@@ -194,7 +199,7 @@ void main() {
     withClock(Clock.fixed(start), () {
       final timestamp = clock.now().millisecondsSinceEpoch.toString();
       expect(sessionFile.existsSync(), false);
-      userProperty.preparePayload();
+      analytics.userProperty.preparePayload();
       expect(sessionFile.readAsStringSync(),
           '{"session_id":$timestamp,"last_ping":$timestamp}');
 
@@ -850,7 +855,7 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion
       clientId: Uuid().generateV4(),
       eventName: DashEvent.hotReloadTime,
       eventData: eventData,
-      userProperty: userProperty,
+      userProperty: analytics.userProperty,
     );
 
     // Checks for the top level keys
@@ -1104,15 +1109,17 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion
     // 4. Event names must be 40 characters or fewer, may only contain alpha-numeric
     //    characters and underscores, and must start with an alphabetic character
     test('max 25 user properties per event', () {
-      final Map<String, Object> userPropPayload = userProperty.preparePayload();
+      final Map<String, Object> userPropPayload =
+          analytics.userProperty.preparePayload();
       const maxUserPropKeys = 25;
 
       expect(userPropPayload.keys.length < maxUserPropKeys, true,
           reason: 'There are too many keys in the UserProperty payload');
     });
 
     test('max 24 characters for user prop keys', () {
-      final Map<String, Object> userPropPayload = userProperty.preparePayload();
+      final Map<String, Object> userPropPayload =
+          analytics.userProperty.preparePayload();
       const maxUserPropLength = 24;
 
       var userPropLengthValid = true;