From 56394c5e49c0888c6667c4e65fe9a995fa5f4373 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Thu, 28 Sep 2023 08:53:49 -0400 Subject: [PATCH 1/9] Catch errors for each line and ignore --- pkgs/unified_analytics/lib/src/log_handler.dart | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/log_handler.dart b/pkgs/unified_analytics/lib/src/log_handler.dart index e6b6cb9fa8..b591497bae 100644 --- a/pkgs/unified_analytics/lib/src/log_handler.dart +++ b/pkgs/unified_analytics/lib/src/log_handler.dart @@ -183,8 +183,13 @@ class LogHandler { // removed later through `whereType` final records = logFile .readAsLinesSync() - .map((String e) => - LogItem.fromRecord(jsonDecode(e) as Map)) + .map((String e) { + try { + return LogItem.fromRecord(jsonDecode(e) as Map); + } on FormatException { + return null; + } + }) .whereType() .toList(); From 48b9bf56952d1c2779354394af47405e85fd7df8 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Fri, 29 Sep 2023 13:49:42 -0400 Subject: [PATCH 2/9] Adding tests --- .../test/log_handler_test.dart | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 pkgs/unified_analytics/test/log_handler_test.dart diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart new file mode 100644 index 0000000000..11a843a4eb --- /dev/null +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -0,0 +1,122 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +import 'package:unified_analytics/src/constants.dart'; +import 'package:unified_analytics/src/enums.dart'; +import 'package:unified_analytics/unified_analytics.dart'; + +void main() { + late Analytics analytics; + late Directory homeDirectory; + late FileSystem fs; + late File logFile; + + final testEvent = Event.hotReloadTime(timeMs: 10); + + setUp(() { + fs = MemoryFileSystem.test(style: FileSystemStyle.posix); + homeDirectory = fs.directory('home'); + logFile = fs.file(p.join( + homeDirectory.path, + kDartToolDirectoryName, + kLogFileName, + )); + + // Create the initialization analytics instance to onboard the tool + final initializationAnalytics = Analytics.test( + tool: DashTool.flutterTool, + homeDirectory: homeDirectory, + measurementId: 'measurementId', + apiSecret: 'apiSecret', + dartVersion: 'dartVersion', + fs: fs, + platform: DevicePlatform.macos, + ); + initializationAnalytics.clientShowedMessage(); + + // This instance is free to send events since the instance above + // has confirmed that the client has shown the message + analytics = Analytics.test( + tool: DashTool.flutterTool, + homeDirectory: homeDirectory, + measurementId: 'measurementId', + apiSecret: 'apiSecret', + dartVersion: 'dartVersion', + fs: fs, + platform: DevicePlatform.macos, + ); + }); + + test('Ensure that log file is created', () { + expect(logFile.existsSync(), true); + }); + + test('LogFileStats is null before events are sent', () { + expect(analytics.logFileStats(), isNull); + }); + + test('LogFileStats returns valid response after sent events', () { + final countOfEventsToSend = 10; + + for (var i = 0; i < countOfEventsToSend; i++) { + analytics.send(testEvent); + } + + expect(analytics.logFileStats(), isNotNull); + expect(logFile.readAsLinesSync().length, countOfEventsToSend); + expect(analytics.logFileStats()!.recordCount, countOfEventsToSend); + }); + + test('The only record in the log file is malformed', () { + // Write invalid json for the only log record + logFile.writeAsStringSync('{{\n'); + + final logFileStats = analytics.logFileStats(); + expect(logFile.readAsLinesSync().length, 1); + expect(logFileStats, isNull, + reason: 'Null should be returned since only ' + 'one record is in there and it is malformed'); + }); + + test('The first record is malformed, but rest are valid', () { + // Write invalid json for the only log record + logFile.writeAsStringSync('{{\n'); + + final countOfEventsToSend = 10; + + for (var i = 0; i < countOfEventsToSend; i++) { + analytics.send(testEvent); + } + final logFileStats = analytics.logFileStats(); + + expect(logFile.readAsLinesSync().length, countOfEventsToSend + 1); + expect(logFileStats, isNotNull); + expect(logFileStats!.recordCount, countOfEventsToSend); + }); + + test('Several records are malformed', () { + final countOfMalformedRecords = 4; + for (var i = 0; i < countOfMalformedRecords; i++) { + final currentContents = logFile.readAsStringSync(); + logFile.writeAsStringSync('$currentContents{{\n'); + } + + final countOfEventsToSend = 10; + + for (var i = 0; i < countOfEventsToSend; i++) { + analytics.send(testEvent); + } + final logFileStats = analytics.logFileStats(); + + expect(logFile.readAsLinesSync().length, + countOfEventsToSend + countOfMalformedRecords); + expect(logFileStats, isNotNull); + expect(logFileStats!.recordCount, countOfEventsToSend); + }); +} From a21e66e5a52ef39bb5bbe07ea07f7f61ba69b927 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Fri, 29 Sep 2023 14:00:40 -0400 Subject: [PATCH 3/9] Additional test with valid json, but missing keys --- .../test/log_handler_test.dart | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index 11a843a4eb..f6a4d97091 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -119,4 +119,22 @@ void main() { expect(logFileStats, isNotNull); expect(logFileStats!.recordCount, countOfEventsToSend); }); + + test('Valid json but invalid keys', () { + + // The second line here is missing the "events" top level + // key which should cause an error for that record only + final contents = ''' +{"client_id":"fe4a035b-bba8-4d4b-a651-ea213e9b8a2c","events":[{"name":"lint_usage_count","params":{"count":1,"name":"prefer_final_fields"}}],"user_properties":{"session_id":{"value":1695147041117},"flutter_channel":{"value":null},"host":{"value":"macOS"},"flutter_version":{"value":"3.14.0-14.0.pre.303"},"dart_version":{"value":"3.2.0-140.0.dev"},"analytics_pkg_version":{"value":"3.1.0"},"tool":{"value":"vscode-plugins"},"local_time":{"value":"2023-09-19 14:44:11.528153 -0400"}}} +{"client_id":"fe4a035b-bba8-4d4b-a651-ea213e9b8a2c","WRONG_EVENT_KEY":[{"name":"lint_usage_count","params":{"count":1,"name":"prefer_for_elements_to_map_fromIterable"}}],"user_properties":{"session_id":{"value":1695147041117},"flutter_channel":{"value":null},"host":{"value":"macOS"},"flutter_version":{"value":"3.14.0-14.0.pre.303"},"dart_version":{"value":"3.2.0-140.0.dev"},"analytics_pkg_version":{"value":"3.1.0"},"tool":{"value":"vscode-plugins"},"local_time":{"value":"2023-09-19 14:44:11.565549 -0400"}}} +{"client_id":"fe4a035b-bba8-4d4b-a651-ea213e9b8a2c","events":[{"name":"lint_usage_count","params":{"count":1,"name":"prefer_function_declarations_over_variables"}}],"user_properties":{"session_id":{"value":1695147041117},"flutter_channel":{"value":null},"host":{"value":"macOS"},"flutter_version":{"value":"3.14.0-14.0.pre.303"},"dart_version":{"value":"3.2.0-140.0.dev"},"analytics_pkg_version":{"value":"3.1.0"},"tool":{"value":"vscode-plugins"},"local_time":{"value":"2023-09-19 14:44:11.589338 -0400"}}} +'''; + logFile.writeAsStringSync(contents); + + final logFileStats = analytics.logFileStats(); + + expect(logFile.readAsLinesSync().length, 3); + expect(logFileStats, isNotNull); + expect(logFileStats!.recordCount, 2); + }); } From 0cad4fb77a88532f9eafa51247fe4e1e3486275e Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Fri, 29 Sep 2023 14:01:44 -0400 Subject: [PATCH 4/9] dart format fix --- pkgs/unified_analytics/test/log_handler_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index f6a4d97091..ff443d4ed0 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -121,7 +121,6 @@ void main() { }); test('Valid json but invalid keys', () { - // The second line here is missing the "events" top level // key which should cause an error for that record only final contents = ''' From a6dbc76e8b5940d0bedb8424a9e02000997f1bf3 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Fri, 6 Oct 2023 11:34:24 -0400 Subject: [PATCH 5/9] Additional test for malformed record getting phased out --- .../test/log_handler_test.dart | 38 ++++++++++++++++--- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index ff443d4ed0..b8fab6ef65 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -61,11 +61,11 @@ void main() { expect(analytics.logFileStats(), isNull); }); - test('LogFileStats returns valid response after sent events', () { + test('LogFileStats returns valid response after sent events', () async { final countOfEventsToSend = 10; for (var i = 0; i < countOfEventsToSend; i++) { - analytics.send(testEvent); + await analytics.send(testEvent); } expect(analytics.logFileStats(), isNotNull); @@ -84,14 +84,14 @@ void main() { 'one record is in there and it is malformed'); }); - test('The first record is malformed, but rest are valid', () { + test('The first record is malformed, but rest are valid', () async { // Write invalid json for the only log record logFile.writeAsStringSync('{{\n'); final countOfEventsToSend = 10; for (var i = 0; i < countOfEventsToSend; i++) { - analytics.send(testEvent); + await analytics.send(testEvent); } final logFileStats = analytics.logFileStats(); @@ -100,7 +100,7 @@ void main() { expect(logFileStats!.recordCount, countOfEventsToSend); }); - test('Several records are malformed', () { + test('Several records are malformed', () async { final countOfMalformedRecords = 4; for (var i = 0; i < countOfMalformedRecords; i++) { final currentContents = logFile.readAsStringSync(); @@ -110,7 +110,7 @@ void main() { final countOfEventsToSend = 10; for (var i = 0; i < countOfEventsToSend; i++) { - analytics.send(testEvent); + await analytics.send(testEvent); } final logFileStats = analytics.logFileStats(); @@ -136,4 +136,30 @@ void main() { expect(logFileStats, isNotNull); expect(logFileStats!.recordCount, 2); }); + + test('Malformed record gets phased out after several events', () async { + // Write invalid json for the only log record + logFile.writeAsStringSync('{{\n'); + + // Send the max number of events minus one so that we have + // 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); + } + final logFileStats = analytics.logFileStats(); + expect(logFile.readAsLinesSync().length, kLogFileLength); + expect(logFileStats, isNotNull); + expect(logFileStats!.recordCount, kLogFileLength - 1, + reason: 'The first record should be malformed'); + expect(logFile.readAsLinesSync()[0].trim(), '{{'); + + // Sending one more event should flush out the malformed record + await analytics.send(testEvent); + + final secondLogFileStats = analytics.logFileStats(); + expect(secondLogFileStats, isNotNull); + expect(secondLogFileStats!.recordCount, kLogFileLength); + expect(logFile.readAsLinesSync()[0].trim(), isNot('{{')); + }); } From 0f377f69c62f45d38e876fd2963c69047c2d4c85 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Fri, 6 Oct 2023 11:37:25 -0400 Subject: [PATCH 6/9] Update documentation --- pkgs/unified_analytics/test/log_handler_test.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index b8fab6ef65..d7be93c098 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -123,6 +123,10 @@ void main() { test('Valid json but invalid keys', () { // The second line here is missing the "events" top level // key which should cause an error for that record only + // + // Important to note that this won't actually cause a FormatException + // like the other malformed records, instead the LogItem.fromRecord + // constructor will return null if all the keys are not available final contents = ''' {"client_id":"fe4a035b-bba8-4d4b-a651-ea213e9b8a2c","events":[{"name":"lint_usage_count","params":{"count":1,"name":"prefer_final_fields"}}],"user_properties":{"session_id":{"value":1695147041117},"flutter_channel":{"value":null},"host":{"value":"macOS"},"flutter_version":{"value":"3.14.0-14.0.pre.303"},"dart_version":{"value":"3.2.0-140.0.dev"},"analytics_pkg_version":{"value":"3.1.0"},"tool":{"value":"vscode-plugins"},"local_time":{"value":"2023-09-19 14:44:11.528153 -0400"}}} {"client_id":"fe4a035b-bba8-4d4b-a651-ea213e9b8a2c","WRONG_EVENT_KEY":[{"name":"lint_usage_count","params":{"count":1,"name":"prefer_for_elements_to_map_fromIterable"}}],"user_properties":{"session_id":{"value":1695147041117},"flutter_channel":{"value":null},"host":{"value":"macOS"},"flutter_version":{"value":"3.14.0-14.0.pre.303"},"dart_version":{"value":"3.2.0-140.0.dev"},"analytics_pkg_version":{"value":"3.1.0"},"tool":{"value":"vscode-plugins"},"local_time":{"value":"2023-09-19 14:44:11.565549 -0400"}}} From 12c923b0185dff2af55d81cc182bf8d196e174b0 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Mon, 9 Oct 2023 13:26:53 -0400 Subject: [PATCH 7/9] Added TODO + add error handling for casting errors --- .../lib/src/log_handler.dart | 5 +++++ .../test/log_handler_test.dart | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/pkgs/unified_analytics/lib/src/log_handler.dart b/pkgs/unified_analytics/lib/src/log_handler.dart index b591497bae..5d4b6fcd68 100644 --- a/pkgs/unified_analytics/lib/src/log_handler.dart +++ b/pkgs/unified_analytics/lib/src/log_handler.dart @@ -184,10 +184,15 @@ class LogHandler { final records = logFile .readAsLinesSync() .map((String e) { + // TODO: eliasyishak, once https://github.com/dart-lang/tools/issues/167 has landed + // ensure we are sending an event for each error with helpful messages try { return LogItem.fromRecord(jsonDecode(e) as Map); } on FormatException { return null; + // ignore: avoid_catching_errors + } on TypeError { + return null; } }) .whereType() diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index d7be93c098..f2b5f349ff 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -166,4 +166,23 @@ void main() { expect(secondLogFileStats!.recordCount, kLogFileLength); expect(logFile.readAsLinesSync()[0].trim(), isNot('{{')); }); + + test('Catching cast errors for each log record silently', () async { + // Write a json array to the log file which will cause + // a cast error when parsing each line + logFile.writeAsStringSync('[{}, 1, 2, 3]\n'); + + final logFileStats = analytics.logFileStats(); + expect(logFileStats, isNull); + + // Ensure it will work as expected after writing correct logs + final countOfEventsToSend = 10; + for (var i = 0; i < countOfEventsToSend; i++) { + await analytics.send(testEvent); + } + final secondLogFileStats = analytics.logFileStats(); + + expect(secondLogFileStats, isNotNull); + expect(secondLogFileStats!.recordCount, countOfEventsToSend); + }); } From e5b55bcdf3d6b2b034b2514acd10217a2023bcd0 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Mon, 9 Oct 2023 13:28:41 -0400 Subject: [PATCH 8/9] Fix format error --- pkgs/unified_analytics/lib/src/log_handler.dart | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/log_handler.dart b/pkgs/unified_analytics/lib/src/log_handler.dart index 5d4b6fcd68..136afc8727 100644 --- a/pkgs/unified_analytics/lib/src/log_handler.dart +++ b/pkgs/unified_analytics/lib/src/log_handler.dart @@ -184,13 +184,14 @@ class LogHandler { final records = logFile .readAsLinesSync() .map((String e) { - // TODO: eliasyishak, once https://github.com/dart-lang/tools/issues/167 has landed - // ensure we are sending an event for each error with helpful messages + // TODO: eliasyishak, once https://github.com/dart-lang/tools/issues/167 + // has landed ensure we are sending an event for each error + // with helpful messages try { return LogItem.fromRecord(jsonDecode(e) as Map); } on FormatException { return null; - // ignore: avoid_catching_errors + // ignore: avoid_catching_errors } on TypeError { return null; } From c341511ed83c94eec4282aaed5cf846c1385a5e2 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Tue, 10 Oct 2023 11:32:38 -0400 Subject: [PATCH 9/9] Clean up --- pkgs/unified_analytics/lib/src/event.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/event.dart b/pkgs/unified_analytics/lib/src/event.dart index 4533bb55bd..3588c8976a 100644 --- a/pkgs/unified_analytics/lib/src/event.dart +++ b/pkgs/unified_analytics/lib/src/event.dart @@ -158,8 +158,8 @@ final class Event { /// [transitiveFileUniqueCount] - the number of unique files reachable from /// the files in each analysis context. /// - /// [transitiveFileUniqueLineCount] - the number of lines in the unique. - /// transitive files + /// [transitiveFileUniqueLineCount] - the number of lines in the unique + /// transitive files. Event.contextStructure({ required int contextsFromBothFiles, required int contextsFromOptionsFiles,