From 11b72c3a1b3721f2f996694036b68f9b15ed95dc Mon Sep 17 00:00:00 2001
From: eliasyishak <42216813+eliasyishak@users.noreply.github.com>
Date: Thu, 14 Mar 2024 14:29:49 -0400
Subject: [PATCH 1/7] Rewrite the `last_ping` key again

---
 .../lib/src/initializer.dart                  |  4 +--
 pkgs/unified_analytics/lib/src/session.dart   | 15 ++++++++++
 .../test/error_handler_test.dart              | 28 +++++++++++++++++++
 .../test/unified_analytics_test.dart          |  6 ++--
 4 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/pkgs/unified_analytics/lib/src/initializer.dart b/pkgs/unified_analytics/lib/src/initializer.dart
index 4f84ef1d5b..32e837404a 100644
--- a/pkgs/unified_analytics/lib/src/initializer.dart
+++ b/pkgs/unified_analytics/lib/src/initializer.dart
@@ -84,8 +84,8 @@ class Initializer {
   }) {
     final now = sessionIdOverride ?? clock.now();
     sessionFile.createSync(recursive: true);
-    sessionFile
-        .writeAsStringSync('{"session_id": ${now.millisecondsSinceEpoch}}');
+    sessionFile.writeAsStringSync(
+        '{"session_id": ${now.millisecondsSinceEpoch}, "last_ping": null}');
   }
 
   /// This will check that there is a client ID populated in
diff --git a/pkgs/unified_analytics/lib/src/session.dart b/pkgs/unified_analytics/lib/src/session.dart
index d19b749c5e..73927de8ce 100644
--- a/pkgs/unified_analytics/lib/src/session.dart
+++ b/pkgs/unified_analytics/lib/src/session.dart
@@ -118,6 +118,21 @@ class Session {
         description: err.osError?.toString(),
       ));
 
+      // Fallback to setting the session id as the current time
+      _sessionId = now.millisecondsSinceEpoch;
+      // ignore: avoid_catching_errors
+    } on TypeError catch (err) {
+      final now = clock.now();
+      Initializer.createSessionFile(
+        sessionFile: sessionFile,
+        sessionIdOverride: now,
+      );
+
+      _errorHandler.log(Event.analyticsException(
+        workflow: 'Session._refreshSessionData',
+        error: err.runtimeType.toString(),
+      ));
+
       // Fallback to setting the session id as the current time
       _sessionId = now.millisecondsSinceEpoch;
     }
diff --git a/pkgs/unified_analytics/test/error_handler_test.dart b/pkgs/unified_analytics/test/error_handler_test.dart
index c7156e2aaf..eaa63eabde 100644
--- a/pkgs/unified_analytics/test/error_handler_test.dart
+++ b/pkgs/unified_analytics/test/error_handler_test.dart
@@ -190,6 +190,34 @@ void main() {
       expect(sessionFile.readAsStringSync(), isNotEmpty);
     });
 
+    test('only sends one event for TypeError', () {
+      // Rewriting the session JSON file with valid JSON that does not contain
+      // the required keys will result in a TypeError
+      sessionFile.writeAsStringSync('{"wrong-key": 123}');
+      analytics.send(testEvent);
+
+      expect(
+        analytics.sentEvents.where(
+            (element) => element.eventName == DashEvent.analyticsException),
+        hasLength(1),
+      );
+      expect(sessionFile.existsSync(), isTrue);
+      expect(sessionFile.readAsStringSync(), isNotEmpty);
+
+      // Write the wrong json string again
+      sessionFile.writeAsStringSync('{"not-correct-key": 123}');
+      analytics.send(testEvent);
+
+      expect(
+        analytics.sentEvents.where(
+            (element) => element.eventName == DashEvent.analyticsException),
+        hasLength(1),
+        reason: 'Only the first error event should exist',
+      );
+      expect(sessionFile.existsSync(), isTrue);
+      expect(sessionFile.readAsStringSync(), isNotEmpty);
+    });
+
     test('sends two unique errors', () {
       // Begin with the session file empty, it should recreate the file
       // and send an error event
diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart
index 8c0849338e..78b03e7c71 100644
--- a/pkgs/unified_analytics/test/unified_analytics_test.dart
+++ b/pkgs/unified_analytics/test/unified_analytics_test.dart
@@ -143,7 +143,8 @@ void main() {
       final timestamp = clock.now().millisecondsSinceEpoch.toString();
       expect(sessionFile.readAsStringSync(), 'contents');
       analytics.userProperty.preparePayload();
-      expect(sessionFile.readAsStringSync(), '{"session_id": $timestamp}');
+      expect(sessionFile.readAsStringSync(),
+          '{"session_id": $timestamp, "last_ping": null}');
 
       // Attempting to fetch the session id when malformed should also
       // send an error event while parsing
@@ -199,7 +200,8 @@ void main() {
       final timestamp = clock.now().millisecondsSinceEpoch.toString();
       expect(sessionFile.existsSync(), false);
       analytics.userProperty.preparePayload();
-      expect(sessionFile.readAsStringSync(), '{"session_id": $timestamp}');
+      expect(sessionFile.readAsStringSync(),
+          '{"session_id": $timestamp, "last_ping": null}');
 
       // Attempting to fetch the session id when malformed should also
       // send an error event while parsing

From 3b9a49abf382f1b0fe926153e682e637a31d3dd6 Mon Sep 17 00:00:00 2001
From: eliasyishak <42216813+eliasyishak@users.noreply.github.com>
Date: Thu, 14 Mar 2024 14:35:20 -0400
Subject: [PATCH 2/7] Apply test fix from #247

---
 pkgs/unified_analytics/test/unified_analytics_test.dart | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart
index 78b03e7c71..9c5fea4bf4 100644
--- a/pkgs/unified_analytics/test/unified_analytics_test.dart
+++ b/pkgs/unified_analytics/test/unified_analytics_test.dart
@@ -1033,6 +1033,14 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion
 
   test('Null values for flutter parameters is reflected properly in log file',
       () {
+    // Because we are using the `MemoryFileSystem.test` constructor,
+    // we don't have a real clock in the filesystem, and because we
+    // are checking the last modified timestamp for the session file
+    // to determine if we need to update the session id, manually setting
+    // that timestamp will ensure we are not updating session id when it
+    // first gets created
+    sessionFile.setLastModifiedSync(DateTime.now());
+
     // Use a for loop two initialize the second analytics instance
     // twice to account for no events being sent on the first instance
     // run for a given tool

From 3848b52019bf0e0080c3149987bbd96ce5ff0fd7 Mon Sep 17 00:00:00 2001
From: eliasyishak <42216813+eliasyishak@users.noreply.github.com>
Date: Thu, 14 Mar 2024 14:45:37 -0400
Subject: [PATCH 3/7] Use timestamp instead of null

---
 pkgs/unified_analytics/lib/src/initializer.dart         | 3 ++-
 pkgs/unified_analytics/test/unified_analytics_test.dart | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/pkgs/unified_analytics/lib/src/initializer.dart b/pkgs/unified_analytics/lib/src/initializer.dart
index 32e837404a..069a8e7876 100644
--- a/pkgs/unified_analytics/lib/src/initializer.dart
+++ b/pkgs/unified_analytics/lib/src/initializer.dart
@@ -85,7 +85,8 @@ class Initializer {
     final now = sessionIdOverride ?? clock.now();
     sessionFile.createSync(recursive: true);
     sessionFile.writeAsStringSync(
-        '{"session_id": ${now.millisecondsSinceEpoch}, "last_ping": null}');
+        '{"session_id": ${now.millisecondsSinceEpoch}, '
+        '"last_ping": ${now.millisecondsSinceEpoch}}');
   }
 
   /// This will check that there is a client ID populated in
diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart
index 9c5fea4bf4..cf6711c874 100644
--- a/pkgs/unified_analytics/test/unified_analytics_test.dart
+++ b/pkgs/unified_analytics/test/unified_analytics_test.dart
@@ -144,7 +144,7 @@ void main() {
       expect(sessionFile.readAsStringSync(), 'contents');
       analytics.userProperty.preparePayload();
       expect(sessionFile.readAsStringSync(),
-          '{"session_id": $timestamp, "last_ping": null}');
+          '{"session_id": $timestamp, "last_ping": $timestamp}');
 
       // Attempting to fetch the session id when malformed should also
       // send an error event while parsing
@@ -201,7 +201,7 @@ void main() {
       expect(sessionFile.existsSync(), false);
       analytics.userProperty.preparePayload();
       expect(sessionFile.readAsStringSync(),
-          '{"session_id": $timestamp, "last_ping": null}');
+          '{"session_id": $timestamp, "last_ping": $timestamp}');
 
       // Attempting to fetch the session id when malformed should also
       // send an error event while parsing

From ed797f568a3be36cf960284a0165270adee2d2a9 Mon Sep 17 00:00:00 2001
From: eliasyishak <42216813+eliasyishak@users.noreply.github.com>
Date: Thu, 14 Mar 2024 14:47:06 -0400
Subject: [PATCH 4/7] Format fix

---
 pkgs/unified_analytics/lib/src/initializer.dart | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pkgs/unified_analytics/lib/src/initializer.dart b/pkgs/unified_analytics/lib/src/initializer.dart
index 069a8e7876..7c97040524 100644
--- a/pkgs/unified_analytics/lib/src/initializer.dart
+++ b/pkgs/unified_analytics/lib/src/initializer.dart
@@ -84,9 +84,9 @@ class Initializer {
   }) {
     final now = sessionIdOverride ?? clock.now();
     sessionFile.createSync(recursive: true);
-    sessionFile.writeAsStringSync(
-        '{"session_id": ${now.millisecondsSinceEpoch}, '
-        '"last_ping": ${now.millisecondsSinceEpoch}}');
+    sessionFile
+        .writeAsStringSync('{"session_id": ${now.millisecondsSinceEpoch}, '
+            '"last_ping": ${now.millisecondsSinceEpoch}}');
   }
 
   /// This will check that there is a client ID populated in

From 7ad01f53a3507e1d7c97079305a010f64b450be8 Mon Sep 17 00:00:00 2001
From: eliasyishak <42216813+eliasyishak@users.noreply.github.com>
Date: Thu, 14 Mar 2024 14:57:19 -0400
Subject: [PATCH 5/7] Update version

---
 pkgs/unified_analytics/CHANGELOG.md           | 4 ++++
 pkgs/unified_analytics/lib/src/constants.dart | 2 +-
 pkgs/unified_analytics/pubspec.yaml           | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/pkgs/unified_analytics/CHANGELOG.md b/pkgs/unified_analytics/CHANGELOG.md
index cb7ff9301c..6827643e3d 100644
--- a/pkgs/unified_analytics/CHANGELOG.md
+++ b/pkgs/unified_analytics/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 5.8.7
+
+- [Bug fix](https://github.com/dart-lang/tools/issues/252) to rewrite the `last_ping` key into the session json file
+
 ## 5.8.6
 
 - Refactored session handler class to use the last modified timestamp as the last ping value
diff --git a/pkgs/unified_analytics/lib/src/constants.dart b/pkgs/unified_analytics/lib/src/constants.dart
index 2ab6bfe2e8..9b0c72bfe8 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.6';
+const String kPackageVersion = '5.8.7';
 
 /// 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 b929cd1837..fa53caafb4 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.6
+version: 5.8.7
 repository: https://github.com/dart-lang/tools/tree/main/pkgs/unified_analytics
 
 environment:

From 1a35ba1d491387328006c504f970fec0dc46a7ba Mon Sep 17 00:00:00 2001
From: eliasyishak <42216813+eliasyishak@users.noreply.github.com>
Date: Thu, 14 Mar 2024 17:03:17 -0400
Subject: [PATCH 6/7] Code review comments

---
 .../lib/src/initializer.dart                  |  3 ++
 pkgs/unified_analytics/lib/src/session.dart   | 14 ----------
 .../test/error_handler_test.dart              | 28 -------------------
 3 files changed, 3 insertions(+), 42 deletions(-)

diff --git a/pkgs/unified_analytics/lib/src/initializer.dart b/pkgs/unified_analytics/lib/src/initializer.dart
index 7c97040524..a9d78cfc62 100644
--- a/pkgs/unified_analytics/lib/src/initializer.dart
+++ b/pkgs/unified_analytics/lib/src/initializer.dart
@@ -84,6 +84,9 @@ class Initializer {
   }) {
     final now = sessionIdOverride ?? clock.now();
     sessionFile.createSync(recursive: true);
+
+    // `last_ping` has been deprecated, remains included for backward
+    // compatibility
     sessionFile
         .writeAsStringSync('{"session_id": ${now.millisecondsSinceEpoch}, '
             '"last_ping": ${now.millisecondsSinceEpoch}}');
diff --git a/pkgs/unified_analytics/lib/src/session.dart b/pkgs/unified_analytics/lib/src/session.dart
index 73927de8ce..1dc6822ce1 100644
--- a/pkgs/unified_analytics/lib/src/session.dart
+++ b/pkgs/unified_analytics/lib/src/session.dart
@@ -121,20 +121,6 @@ class Session {
       // Fallback to setting the session id as the current time
       _sessionId = now.millisecondsSinceEpoch;
       // ignore: avoid_catching_errors
-    } on TypeError catch (err) {
-      final now = clock.now();
-      Initializer.createSessionFile(
-        sessionFile: sessionFile,
-        sessionIdOverride: now,
-      );
-
-      _errorHandler.log(Event.analyticsException(
-        workflow: 'Session._refreshSessionData',
-        error: err.runtimeType.toString(),
-      ));
-
-      // Fallback to setting the session id as the current time
-      _sessionId = now.millisecondsSinceEpoch;
     }
   }
 }
diff --git a/pkgs/unified_analytics/test/error_handler_test.dart b/pkgs/unified_analytics/test/error_handler_test.dart
index eaa63eabde..c7156e2aaf 100644
--- a/pkgs/unified_analytics/test/error_handler_test.dart
+++ b/pkgs/unified_analytics/test/error_handler_test.dart
@@ -190,34 +190,6 @@ void main() {
       expect(sessionFile.readAsStringSync(), isNotEmpty);
     });
 
-    test('only sends one event for TypeError', () {
-      // Rewriting the session JSON file with valid JSON that does not contain
-      // the required keys will result in a TypeError
-      sessionFile.writeAsStringSync('{"wrong-key": 123}');
-      analytics.send(testEvent);
-
-      expect(
-        analytics.sentEvents.where(
-            (element) => element.eventName == DashEvent.analyticsException),
-        hasLength(1),
-      );
-      expect(sessionFile.existsSync(), isTrue);
-      expect(sessionFile.readAsStringSync(), isNotEmpty);
-
-      // Write the wrong json string again
-      sessionFile.writeAsStringSync('{"not-correct-key": 123}');
-      analytics.send(testEvent);
-
-      expect(
-        analytics.sentEvents.where(
-            (element) => element.eventName == DashEvent.analyticsException),
-        hasLength(1),
-        reason: 'Only the first error event should exist',
-      );
-      expect(sessionFile.existsSync(), isTrue);
-      expect(sessionFile.readAsStringSync(), isNotEmpty);
-    });
-
     test('sends two unique errors', () {
       // Begin with the session file empty, it should recreate the file
       // and send an error event

From 7e68e63c3ed5930d75e58e1a2c16cd388a66b0d1 Mon Sep 17 00:00:00 2001
From: eliasyishak <42216813+eliasyishak@users.noreply.github.com>
Date: Thu, 14 Mar 2024 17:05:05 -0400
Subject: [PATCH 7/7] Lingering comment

---
 pkgs/unified_analytics/lib/src/session.dart | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pkgs/unified_analytics/lib/src/session.dart b/pkgs/unified_analytics/lib/src/session.dart
index 1dc6822ce1..d19b749c5e 100644
--- a/pkgs/unified_analytics/lib/src/session.dart
+++ b/pkgs/unified_analytics/lib/src/session.dart
@@ -120,7 +120,6 @@ class Session {
 
       // Fallback to setting the session id as the current time
       _sessionId = now.millisecondsSinceEpoch;
-      // ignore: avoid_catching_errors
     }
   }
 }