From ed160ff5afa0394b0f5f028d6ba266f5fac37d31 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Tue, 27 Feb 2024 16:10:25 -0500 Subject: [PATCH 1/5] Initialize session handler at end of constructor --- pkgs/unified_analytics/lib/src/analytics.dart | 5 ++- pkgs/unified_analytics/lib/src/session.dart | 19 ++++++----- .../test/unified_analytics_test.dart | 34 ++++++++++++++++++- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index a0d32280c..0c3a98721 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/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 diff --git a/pkgs/unified_analytics/lib/src/session.dart b/pkgs/unified_analytics/lib/src/session.dart index de4999b07..80305ee18 100644 --- a/pkgs/unified_analytics/lib/src/session.dart +++ b/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({ 'session_id': _sessionId, diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 8b01e5d74..76b57a0a9 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -121,7 +121,6 @@ void main() { homeDirectory: home, fs: fs, errorHandler: ErrorHandler(sendFunction: analytics.send), - telemetryEnabled: analytics.telemetryEnabled, ), flutterChannel: flutterChannel, host: platform.label, @@ -183,6 +182,39 @@ 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); + errorEvent!; + 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(); From 9a08a7d4b7074af06475569dc1dbefbd9b062dd4 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Tue, 27 Feb 2024 16:11:45 -0500 Subject: [PATCH 2/5] Remove redundant standalone `UserProperty` from test --- .../test/unified_analytics_test.dart | 40 ++++--------------- 1 file changed, 8 insertions(+), 32 deletions(-) diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 76b57a0a9..0dc33483c 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -14,8 +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'; @@ -45,10 +43,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,25 +108,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), - ), - 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', () { @@ -168,7 +144,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}'); @@ -205,14 +181,14 @@ void main() { analytics.clientShowedMessage(); final errorEvent = analytics.sentEvents - .where((element) => - element.eventName == DashEvent.analyticsException) + .where((element) => element.eventName == DashEvent.analyticsException) .firstOrNull; expect(errorEvent, isNotNull); errorEvent!; expect(errorEvent.eventData['workflow'], 'Session._refreshSessionData'); expect(errorEvent.eventData['error'], 'FormatException'); - expect(errorEvent.eventData['description'], 'message: Unexpected character\nsource: contents'); + expect(errorEvent.eventData['description'], + 'message: Unexpected character\nsource: contents'); }); test('Resetting session file when file is removed', () { @@ -226,7 +202,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}'); @@ -882,7 +858,7 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion clientId: Uuid().generateV4(), eventName: DashEvent.hotReloadTime, eventData: eventData, - userProperty: userProperty, + userProperty: analytics.userProperty, ); // Checks for the top level keys @@ -1136,7 +1112,7 @@ ${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 userPropPayload = userProperty.preparePayload(); + final Map userPropPayload = analytics.userProperty.preparePayload(); const maxUserPropKeys = 25; expect(userPropPayload.keys.length < maxUserPropKeys, true, @@ -1144,7 +1120,7 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion }); test('max 24 characters for user prop keys', () { - final Map userPropPayload = userProperty.preparePayload(); + final Map userPropPayload = analytics.userProperty.preparePayload(); const maxUserPropLength = 24; var userPropLengthValid = true; From d0f6b7924bbb0e5da82ab8d48f89542509ef8008 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Tue, 27 Feb 2024 16:24:01 -0500 Subject: [PATCH 3/5] Bump versions --- pkgs/unified_analytics/CHANGELOG.md | 4 ++++ pkgs/unified_analytics/lib/src/constants.dart | 2 +- pkgs/unified_analytics/pubspec.yaml | 2 +- pkgs/unified_analytics/test/unified_analytics_test.dart | 2 -- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkgs/unified_analytics/CHANGELOG.md b/pkgs/unified_analytics/CHANGELOG.md index 8dcf985be..a35dcd692 100644 --- a/pkgs/unified_analytics/CHANGELOG.md +++ b/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` diff --git a/pkgs/unified_analytics/lib/src/constants.dart b/pkgs/unified_analytics/lib/src/constants.dart index 77ec0212f..4dfca3e2e 100644 --- a/pkgs/unified_analytics/lib/src/constants.dart +++ b/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; diff --git a/pkgs/unified_analytics/pubspec.yaml b/pkgs/unified_analytics/pubspec.yaml index cc57c0d51..8cd92f041 100644 --- a/pkgs/unified_analytics/pubspec.yaml +++ b/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: diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 0dc33483c..9b7514910 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -14,7 +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/user_property.dart'; import 'package:unified_analytics/src/utils.dart'; import 'package:unified_analytics/unified_analytics.dart'; import 'package:yaml/yaml.dart'; @@ -30,7 +29,6 @@ void main() { late File configFile; late File logFile; late File dismissedSurveyFile; - late UserProperty userProperty; const homeDirName = 'home'; const initialTool = DashTool.flutterTool; From 9c3b2e4d3692152992c550ee032ffb344d1f9eb9 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Tue, 27 Feb 2024 16:28:15 -0500 Subject: [PATCH 4/5] `dart format .` --- pkgs/unified_analytics/test/unified_analytics_test.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 9b7514910..5c007d242 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -1110,7 +1110,8 @@ ${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 userPropPayload = analytics.userProperty.preparePayload(); + final Map userPropPayload = + analytics.userProperty.preparePayload(); const maxUserPropKeys = 25; expect(userPropPayload.keys.length < maxUserPropKeys, true, @@ -1118,7 +1119,8 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion }); test('max 24 characters for user prop keys', () { - final Map userPropPayload = analytics.userProperty.preparePayload(); + final Map userPropPayload = + analytics.userProperty.preparePayload(); const maxUserPropLength = 24; var userPropLengthValid = true; From 9fd16b5d9aca8c99a6529425d9797f4d8fda0e61 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Tue, 27 Feb 2024 17:09:26 -0500 Subject: [PATCH 5/5] Cast to non-null in `expect` --- pkgs/unified_analytics/test/unified_analytics_test.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 5c007d242..39046f74d 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -182,8 +182,7 @@ void main() { .where((element) => element.eventName == DashEvent.analyticsException) .firstOrNull; expect(errorEvent, isNotNull); - errorEvent!; - expect(errorEvent.eventData['workflow'], 'Session._refreshSessionData'); + expect(errorEvent!.eventData['workflow'], 'Session._refreshSessionData'); expect(errorEvent.eventData['error'], 'FormatException'); expect(errorEvent.eventData['description'], 'message: Unexpected character\nsource: contents');