From f87a22faafc0cdc877f41240c17893b348690f96 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Fri, 26 Jan 2024 16:03:07 -0500 Subject: [PATCH 01/20] Added new event + refactoring sentEvent on impl --- pkgs/unified_analytics/lib/src/analytics.dart | 45 +++++++++++---- pkgs/unified_analytics/lib/src/enums.dart | 4 ++ pkgs/unified_analytics/lib/src/event.dart | 22 ++++++++ .../lib/src/log_handler.dart | 56 +++++++++++-------- pkgs/unified_analytics/test/event_test.dart | 19 ++++++- .../test/log_handler_test.dart | 11 +++- .../test/no_op_analytics_test.dart | 10 ++-- 7 files changed, 127 insertions(+), 40 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index 5d6df92bf..f228dbb65 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -60,7 +60,7 @@ abstract class Analytics { final homeDirectory = getHomeDirectory(fs); if (homeDirectory == null || !checkDirectoryForWritePermissions(homeDirectory)) { - return NoOpAnalytics(); + return const NoOpAnalytics(); } // Resolve the OS using dart:io @@ -225,6 +225,15 @@ abstract class Analytics { /// out of the configuration file. Map get parsedTools; + /// Use this list to check for events that have been emitted when + /// invoking the send method. + /// + /// This list will only get populated when running the [Analytics.test] + /// constructor, [Analytics.development], or an instance of [FakeAnalytics]. + /// It will otherwise remain empty when used in production. + @visibleForTesting + List get sentEvents; + /// Boolean that lets the client know if they should display the message. bool get shouldShowMessage; @@ -357,6 +366,10 @@ class AnalyticsImpl implements Analytics { /// from the [GAClient]. final _futures = >[]; + /// Stores the events that have been sent while the instance exists. + /// Events will only be stored in this list if + final List _sentEvents = []; + AnalyticsImpl({ required this.tool, required Directory homeDirectory, @@ -442,7 +455,11 @@ class AnalyticsImpl implements Analytics { ); // Initialize the log handler to persist events that are being sent - _logHandler = LogHandler(fs: fs, homeDirectory: homeDirectory); + _logHandler = LogHandler( + fs: fs, + homeDirectory: homeDirectory, + analyticsInstance: this, + ); } @override @@ -484,6 +501,9 @@ class AnalyticsImpl implements Analytics { @override Map get parsedTools => _configHandler.parsedTools; + @override + List get sentEvents => _sentEvents; + @override bool get shouldShowMessage => _showMessage; @@ -603,7 +623,15 @@ class AnalyticsImpl implements Analytics { userProperty: userProperty, ); - if (_enableAsserts) checkBody(body); + // This will be set to true by default if running the development or + // test constructors + // + // It will also be enabled by default for the + // [FakeAnalytics] instance + if (_enableAsserts) { + checkBody(body); + _sentEvents.add(event); + } _logHandler.save(data: body); @@ -703,10 +731,6 @@ class AnalyticsImpl implements Analytics { /// workflow. Invoking the [send] method on this instance will not make any /// network requests to Google Analytics. class FakeAnalytics extends AnalyticsImpl { - /// Use this list to check for events that have been emitted when - /// invoking the send method - final List sentEvents = []; - /// Class to use when you want to see which events were sent FakeAnalytics({ required super.tool, @@ -774,13 +798,14 @@ class NoOpAnalytics implements Analytics { final Map> userPropertyMap = const >{}; - factory NoOpAnalytics() => const NoOpAnalytics._(); - - const NoOpAnalytics._(); + const NoOpAnalytics(); @override String get clientId => staticClientId; + @override + List get sentEvents => []; + @override void clientShowedMessage() {} diff --git a/pkgs/unified_analytics/lib/src/enums.dart b/pkgs/unified_analytics/lib/src/enums.dart index 90acc3a12..a3e9dac92 100644 --- a/pkgs/unified_analytics/lib/src/enums.dart +++ b/pkgs/unified_analytics/lib/src/enums.dart @@ -22,6 +22,10 @@ enum DashEvent { label: 'analytics_collection_enabled', description: 'The opt-in status for analytics collection', ), + analyticsException( + label: 'analytics_exception', + description: 'Errors that occur within the package for logging', + ), exception( label: 'exception', description: 'General errors to log', diff --git a/pkgs/unified_analytics/lib/src/event.dart b/pkgs/unified_analytics/lib/src/event.dart index de288ce74..deb38a396 100644 --- a/pkgs/unified_analytics/lib/src/event.dart +++ b/pkgs/unified_analytics/lib/src/event.dart @@ -19,6 +19,28 @@ final class Event { : eventName = DashEvent.analyticsCollectionEnabled, eventData = {'status': status}; + /// Event that is emitted when an error occurs within + /// `package:unified_analytics`, tools that are using this package + /// should not use this event constructor. Instead they should use + /// the more generic [Event.exception] constructor. + /// + /// [workflow] - refers to what process caused the error, such as + /// "LogHandler.logFileStats". + /// + /// [error] - the name of the error, such as "FormatException". + /// + /// [description] - the description of the error being caught. + Event.analyticsException({ + required String workflow, + required String error, + String? description, + }) : eventName = DashEvent.analyticsException, + eventData = { + 'workflow': workflow, + 'error': error, + if (description != null) 'description': description, + }; + /// This is for various workflows within the flutter tool related /// to iOS and macOS workflows. /// diff --git a/pkgs/unified_analytics/lib/src/log_handler.dart b/pkgs/unified_analytics/lib/src/log_handler.dart index 4f5dbf3be..d09ea2ebb 100644 --- a/pkgs/unified_analytics/lib/src/log_handler.dart +++ b/pkgs/unified_analytics/lib/src/log_handler.dart @@ -8,6 +8,7 @@ import 'package:clock/clock.dart'; import 'package:file/file.dart'; import 'package:path/path.dart' as p; +import '../unified_analytics.dart'; import 'constants.dart'; import 'initializer.dart'; @@ -81,22 +82,6 @@ class LogFileStats { required this.eventCount, }); - @override - String toString() { - final encoder = const JsonEncoder.withIndent(' '); - return encoder.convert({ - 'startDateTime': startDateTime.toString(), - 'minsFromStartDateTime': minsFromStartDateTime, - 'endDateTime': endDateTime.toString(), - 'minsFromEndDateTime': minsFromEndDateTime, - 'sessionCount': sessionCount, - 'recordCount': recordCount, - 'eventCount': eventCount, - 'toolCount': toolCount, - 'flutterChannelCount': flutterChannelCount, - }); - } - /// Pass in a string label for one of the instance variables /// and return the integer value of that label. /// @@ -149,6 +134,22 @@ class LogFileStats { return null; } + + @override + String toString() { + final encoder = const JsonEncoder.withIndent(' '); + return encoder.convert({ + 'startDateTime': startDateTime.toString(), + 'minsFromStartDateTime': minsFromStartDateTime, + 'endDateTime': endDateTime.toString(), + 'minsFromEndDateTime': minsFromEndDateTime, + 'sessionCount': sessionCount, + 'recordCount': recordCount, + 'eventCount': eventCount, + 'toolCount': toolCount, + 'flutterChannelCount': flutterChannelCount, + }); + } } /// This class is responsible for writing to a log @@ -160,17 +161,20 @@ class LogHandler { final FileSystem fs; final Directory homeDirectory; final File logFile; + final Analytics _analyticsInstance; /// A log handler constructor that will delegate saving /// logs and retrieving stats from the persisted log. LogHandler({ required this.fs, required this.homeDirectory, - }) : logFile = fs.file(p.join( + Analytics analyticsInstance = const NoOpAnalytics(), + }) : logFile = fs.file(p.join( homeDirectory.path, kDartToolDirectoryName, kLogFileName, - )); + )), + _analyticsInstance = analyticsInstance; /// Get stats from the persisted log file. /// @@ -184,15 +188,21 @@ 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 { + } on FormatException catch (err) { + _analyticsInstance.send(Event.analyticsException( + workflow: 'LogFileStats.logFileStats', + error: err.runtimeType.toString(), + description: 'message: ${err.message}\nsource: ${err.source}', + )); return null; // ignore: avoid_catching_errors - } on TypeError { + } on TypeError catch (err) { + _analyticsInstance.send(Event.analyticsException( + workflow: 'LogFileStats.logFileStats', + error: err.runtimeType.toString(), + )); return null; } }) diff --git a/pkgs/unified_analytics/test/event_test.dart b/pkgs/unified_analytics/test/event_test.dart index dd3f37fa1..4bd3464ba 100644 --- a/pkgs/unified_analytics/test/event_test.dart +++ b/pkgs/unified_analytics/test/event_test.dart @@ -536,6 +536,23 @@ void main() { expect(constructedEvent.eventData.length, 27); }); + test('Event.analyticsException constructed', () { + Event generateEvent() => Event.analyticsException( + workflow: 'workflow', + error: 'error', + description: 'description', + ); + + final constructedEvent = generateEvent(); + + expect(generateEvent, returnsNormally); + expect(constructedEvent.eventName, DashEvent.analyticsException); + expect(constructedEvent.eventData['workflow'], 'workflow'); + expect(constructedEvent.eventData['error'], 'error'); + expect(constructedEvent.eventData['description'], 'description'); + expect(constructedEvent.eventData.length, 3); + }); + test('Confirm all constructors were checked', () { var constructorCount = 0; for (var declaration in reflectClass(Event).declarations.keys) { @@ -544,7 +561,7 @@ void main() { // Change this integer below if your PR either adds or removes // an Event constructor - final eventsAccountedForInTests = 25; + final eventsAccountedForInTests = 26; expect(eventsAccountedForInTests, constructorCount, reason: 'If you added or removed an event constructor, ' 'ensure you have updated ' diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index d7715573a..a20ff8928 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -78,11 +78,20 @@ void main() { // Write invalid json for the only log record logFile.writeAsStringSync('{{\n'); - final logFileStats = analytics.logFileStats(); expect(logFile.readAsLinesSync().length, 1); + final logFileStats = analytics.logFileStats(); expect(logFileStats, isNull, reason: 'Null should be returned since only ' 'one record is in there and it is malformed'); + expect( + analytics.sentEvents, + contains( + Event.analyticsException( + workflow: 'LogFileStats.logFileStats', + error: 'FormatException', + description: 'message: Unexpected character\nsource: {{', + ), + )); }); test('The first record is malformed, but rest are valid', () async { diff --git a/pkgs/unified_analytics/test/no_op_analytics_test.dart b/pkgs/unified_analytics/test/no_op_analytics_test.dart index 4fb6baa37..66c1ef6c8 100644 --- a/pkgs/unified_analytics/test/no_op_analytics_test.dart +++ b/pkgs/unified_analytics/test/no_op_analytics_test.dart @@ -13,7 +13,7 @@ void main() { final testEvent = Event.hotReloadTime(timeMs: 50); test('NoOpAnalytics.telemetryEnabled is always false', () async { - final analytics = NoOpAnalytics(); + final analytics = const NoOpAnalytics(); expect(analytics.telemetryEnabled, isFalse); await analytics.setTelemetry(true); @@ -21,7 +21,7 @@ void main() { }); test('NoOpAnalytics.shouldShowMessage is always false', () async { - final analytics = NoOpAnalytics(); + final analytics = const NoOpAnalytics(); expect(analytics.shouldShowMessage, isFalse); analytics.clientShowedMessage(); @@ -29,7 +29,7 @@ void main() { }); test('NoOpAnalytics.sendEvent() always returns null', () async { - final analytics = NoOpAnalytics(); + final analytics = const NoOpAnalytics(); await analytics.setTelemetry(true); analytics.clientShowedMessage(); @@ -40,7 +40,7 @@ void main() { }); test('NoOpAnalytics.logFileStats() always returns null', () async { - final analytics = NoOpAnalytics(); + final analytics = const NoOpAnalytics(); expect(analytics.logFileStats(), isNull); @@ -64,7 +64,7 @@ void main() { }); test('Fetching the client id', () { - final analytics = NoOpAnalytics(); + final analytics = const NoOpAnalytics(); expect(analytics.clientId, 'xxxx-xxxx'); }); } From 421df72e3c3d660c673b6536524c796f3f7d94b9 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Wed, 31 Jan 2024 14:36:32 -0500 Subject: [PATCH 02/20] Fix tests + limiting one error event for logFileStats --- .../lib/src/log_handler.dart | 27 ++++++++++++------- .../test/log_handler_test.dart | 27 ++++++++++++++----- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/log_handler.dart b/pkgs/unified_analytics/lib/src/log_handler.dart index d09ea2ebb..b236bb675 100644 --- a/pkgs/unified_analytics/lib/src/log_handler.dart +++ b/pkgs/unified_analytics/lib/src/log_handler.dart @@ -182,6 +182,9 @@ class LogHandler { /// developers and will not have any data for flutter /// related metrics. LogFileStats? logFileStats() { + // Ensure we are only sending once per invocation of this method + var eventSent = false; + // Parse each line of the log file through [LogItem], // some returned records may be null if malformed, they will be // removed later through `whereType` @@ -191,18 +194,24 @@ class LogHandler { try { return LogItem.fromRecord(jsonDecode(e) as Map); } on FormatException catch (err) { - _analyticsInstance.send(Event.analyticsException( - workflow: 'LogFileStats.logFileStats', - error: err.runtimeType.toString(), - description: 'message: ${err.message}\nsource: ${err.source}', - )); + if (!eventSent) { + _analyticsInstance.send(Event.analyticsException( + workflow: 'LogFileStats.logFileStats', + error: err.runtimeType.toString(), + description: 'message: ${err.message}\nsource: ${err.source}', + )); + eventSent = true; + } return null; // ignore: avoid_catching_errors } on TypeError catch (err) { - _analyticsInstance.send(Event.analyticsException( - workflow: 'LogFileStats.logFileStats', - error: err.runtimeType.toString(), - )); + if (!eventSent) { + _analyticsInstance.send(Event.analyticsException( + workflow: 'LogFileStats.logFileStats', + error: err.runtimeType.toString(), + )); + eventSent = true; + } return null; } }) diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index a20ff8928..fd6c81f06 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -103,9 +103,9 @@ void main() { for (var i = 0; i < countOfEventsToSend; i++) { analytics.send(testEvent); } + expect(logFile.readAsLinesSync().length, countOfEventsToSend + 1); final logFileStats = analytics.logFileStats(); - expect(logFile.readAsLinesSync().length, countOfEventsToSend + 1); expect(logFileStats, isNotNull); expect(logFileStats!.recordCount, countOfEventsToSend); }); @@ -122,10 +122,15 @@ void main() { for (var i = 0; i < countOfEventsToSend; i++) { analytics.send(testEvent); } - final logFileStats = analytics.logFileStats(); 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'); + expect(logFileStats, isNotNull); expect(logFileStats!.recordCount, countOfEventsToSend); }); @@ -155,16 +160,23 @@ void main() { // Write invalid json for the only log record logFile.writeAsStringSync('{{\n'); - // Send the max number of events minus one so that we have + // Send the max number of events minus two 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++) { + // + // We need to account for the event that is sent when + // calling [logFileStats()] fails and sends an instance + // of [Event.analyticsException] + final recordsToSendInitially = kLogFileLength - 2; + for (var i = 0; i < recordsToSendInitially; i++) { 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); expect(logFileStats, isNotNull); - expect(logFileStats!.recordCount, kLogFileLength - 1, + expect(logFileStats!.recordCount, recordsToSendInitially, reason: 'The first record should be malformed'); expect(logFile.readAsLinesSync()[0].trim(), '{{'); @@ -172,6 +184,7 @@ void main() { analytics.send(testEvent); final secondLogFileStats = analytics.logFileStats(); + expect(analytics.sentEvents.last, testEvent); expect(secondLogFileStats, isNotNull); expect(secondLogFileStats!.recordCount, kLogFileLength); expect(logFile.readAsLinesSync()[0].trim(), isNot('{{')); @@ -193,7 +206,9 @@ void main() { final secondLogFileStats = analytics.logFileStats(); expect(secondLogFileStats, isNotNull); - expect(secondLogFileStats!.recordCount, countOfEventsToSend); + expect(secondLogFileStats!.recordCount, countOfEventsToSend + 1, + reason: 'Plus one for the error event that is sent ' + 'from the first logFileStats call'); }); test( From 6f0e8a4ab6cefaf304dfce6042c8694fe1e5ca20 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Wed, 31 Jan 2024 14:38:04 -0500 Subject: [PATCH 03/20] Make `Analytics` required for `LogHandler` --- pkgs/unified_analytics/lib/src/log_handler.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/unified_analytics/lib/src/log_handler.dart b/pkgs/unified_analytics/lib/src/log_handler.dart index b236bb675..a75f5b2a1 100644 --- a/pkgs/unified_analytics/lib/src/log_handler.dart +++ b/pkgs/unified_analytics/lib/src/log_handler.dart @@ -168,7 +168,7 @@ class LogHandler { LogHandler({ required this.fs, required this.homeDirectory, - Analytics analyticsInstance = const NoOpAnalytics(), + required Analytics analyticsInstance, }) : logFile = fs.file(p.join( homeDirectory.path, kDartToolDirectoryName, From ac0f3ad490ae887ab61a4c33e35a7951afcb16b0 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Wed, 31 Jan 2024 14:55:34 -0500 Subject: [PATCH 04/20] Make error sent a field in class --- pkgs/unified_analytics/lib/src/log_handler.dart | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/log_handler.dart b/pkgs/unified_analytics/lib/src/log_handler.dart index a75f5b2a1..3ba0f172b 100644 --- a/pkgs/unified_analytics/lib/src/log_handler.dart +++ b/pkgs/unified_analytics/lib/src/log_handler.dart @@ -163,6 +163,9 @@ class LogHandler { final File logFile; final Analytics _analyticsInstance; + /// Ensure we are only sending once per invocation for this class. + var _errorEventSent = false; + /// A log handler constructor that will delegate saving /// logs and retrieving stats from the persisted log. LogHandler({ @@ -182,9 +185,6 @@ class LogHandler { /// developers and will not have any data for flutter /// related metrics. LogFileStats? logFileStats() { - // Ensure we are only sending once per invocation of this method - var eventSent = false; - // Parse each line of the log file through [LogItem], // some returned records may be null if malformed, they will be // removed later through `whereType` @@ -194,23 +194,23 @@ class LogHandler { try { return LogItem.fromRecord(jsonDecode(e) as Map); } on FormatException catch (err) { - if (!eventSent) { + if (!_errorEventSent) { _analyticsInstance.send(Event.analyticsException( workflow: 'LogFileStats.logFileStats', error: err.runtimeType.toString(), description: 'message: ${err.message}\nsource: ${err.source}', )); - eventSent = true; + _errorEventSent = true; } return null; // ignore: avoid_catching_errors } on TypeError catch (err) { - if (!eventSent) { + if (!_errorEventSent) { _analyticsInstance.send(Event.analyticsException( workflow: 'LogFileStats.logFileStats', error: err.runtimeType.toString(), )); - eventSent = true; + _errorEventSent = true; } return null; } From 42161bcf0cbf61bf6e36676ff09fa4568e540955 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Wed, 31 Jan 2024 15:27:39 -0500 Subject: [PATCH 05/20] Events added for error handling in session handler --- pkgs/unified_analytics/lib/src/analytics.dart | 6 ++- pkgs/unified_analytics/lib/src/session.dart | 34 ++++++++++++++-- .../test/unified_analytics_test.dart | 39 ++++++++++++++++++- 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index 83c666db2..969c62913 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -431,7 +431,11 @@ class AnalyticsImpl implements Analytics { // each event that is sent to Google Analytics -- it will be responsible // for getting the session id or rolling the session if the duration // exceeds [kSessionDurationMinutes] - _sessionHandler = Session(homeDirectory: homeDirectory, fs: fs); + _sessionHandler = Session( + homeDirectory: homeDirectory, + fs: fs, + analyticsInstance: this, + ); userProperty = UserProperty( session: _sessionHandler, flutterChannel: flutterChannel, diff --git a/pkgs/unified_analytics/lib/src/session.dart b/pkgs/unified_analytics/lib/src/session.dart index f5dc66808..27468a892 100644 --- a/pkgs/unified_analytics/lib/src/session.dart +++ b/pkgs/unified_analytics/lib/src/session.dart @@ -9,22 +9,30 @@ import 'package:clock/clock.dart'; import 'package:file/file.dart'; import 'package:path/path.dart' as p; +import 'analytics.dart'; import 'constants.dart'; +import 'event.dart'; import 'initializer.dart'; class Session { final Directory homeDirectory; final FileSystem fs; final File sessionFile; + final Analytics _analyticsInstance; late int _sessionId; late int _lastPing; + /// Ensure we are only sending once per invocation for this class. + var _errorEventSent = false; + Session({ required this.homeDirectory, required this.fs, - }) : sessionFile = fs.file(p.join( - homeDirectory.path, kDartToolDirectoryName, kSessionFileName)) { + required Analytics analyticsInstance, + }) : sessionFile = fs.file(p.join( + homeDirectory.path, kDartToolDirectoryName, kSessionFileName)), + _analyticsInstance = analyticsInstance { _refreshSessionData(); } @@ -87,13 +95,31 @@ class Session { try { parseContents(); - } on FormatException { + } on FormatException catch (err) { Initializer.createSessionFile(sessionFile: sessionFile); + if (!_errorEventSent) { + _analyticsInstance.send(Event.analyticsException( + workflow: 'Session._refreshSessionData', + error: err.runtimeType.toString(), + description: 'message: ${err.message}\nsource: ${err.source}', + )); + _errorEventSent = true; + } + parseContents(); - } on PathNotFoundException { + } on FileSystemException catch (err) { Initializer.createSessionFile(sessionFile: sessionFile); + if (!_errorEventSent) { + _analyticsInstance.send(Event.analyticsException( + workflow: 'Session._refreshSessionData', + error: err.runtimeType.toString(), + description: err.osError?.toString(), + )); + _errorEventSent = true; + } + parseContents(); } } diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 22e088596..8a28bb2f9 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -116,7 +116,11 @@ void main() { // Create the user property object that is also // created within analytics for testing userProperty = UserProperty( - session: Session(homeDirectory: home, fs: fs), + session: Session( + homeDirectory: home, + fs: fs, + analyticsInstance: analytics, + ), flutterChannel: flutterChannel, host: platform.label, flutterVersion: flutterVersion, @@ -166,6 +170,39 @@ void main() { userProperty.preparePayload(); expect(sessionFile.readAsStringSync(), '{"session_id":$timestamp,"last_ping":$timestamp}'); + + // Attempting to fetch the session id when malformed should also + // send an error event while parsing + final lastEvent = analytics.sentEvents.last; + expect(lastEvent, isNotNull); + expect(lastEvent.eventName, DashEvent.analyticsException); + expect(lastEvent.eventData['workflow']!, 'Session._refreshSessionData'); + expect(lastEvent.eventData['error']!, 'FormatException'); + }); + }); + + test('Resetting session file when file is removed', () { + // Purposefully write delete the file + sessionFile.deleteSync(); + + // Define the initial time to start + final start = DateTime(1995, 3, 3, 12, 0); + + // Set the clock to the start value defined above + withClock(Clock.fixed(start), () { + final timestamp = clock.now().millisecondsSinceEpoch.toString(); + expect(sessionFile.existsSync(), false); + userProperty.preparePayload(); + expect(sessionFile.readAsStringSync(), + '{"session_id":$timestamp,"last_ping":$timestamp}'); + + // Attempting to fetch the session id when malformed should also + // send an error event while parsing + final lastEvent = analytics.sentEvents.last; + expect(lastEvent, isNotNull); + expect(lastEvent.eventName, DashEvent.analyticsException); + expect(lastEvent.eventData['workflow']!, 'Session._refreshSessionData'); + expect(lastEvent.eventData['error']!, 'FileSystemException'); }); }); From 5ec6a9958b37bc11c7423f932593dce493489ca7 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Wed, 31 Jan 2024 15:28:58 -0500 Subject: [PATCH 06/20] Remove unnecessary `io` import --- 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 27468a892..33d3ecdad 100644 --- a/pkgs/unified_analytics/lib/src/session.dart +++ b/pkgs/unified_analytics/lib/src/session.dart @@ -3,7 +3,6 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:convert'; -import 'dart:io'; import 'package:clock/clock.dart'; import 'package:file/file.dart'; From 2bee9ccec98054b0e50b4180e4b82739a1915dc0 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:55:06 -0500 Subject: [PATCH 07/20] Refactoring `legacyOptOut` to use loop --- pkgs/unified_analytics/lib/src/utils.dart | 162 +++++++++------------- 1 file changed, 68 insertions(+), 94 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/utils.dart b/pkgs/unified_analytics/lib/src/utils.dart index 455eb1e1f..3044a50ad 100644 --- a/pkgs/unified_analytics/lib/src/utils.dart +++ b/pkgs/unified_analytics/lib/src/utils.dart @@ -129,112 +129,86 @@ Directory? getHomeDirectory(FileSystem fs) { return fs.directory(home); } -/// Returns `true` if user has opted out of legacy analytics in Dart or Flutter. +/// Returns `true` if user has opted out of legacy analytics in +/// Dart or Flutter. /// /// Checks legacy opt-out status for the Flutter /// and Dart in the following locations. /// /// Dart: `$HOME/.dart/dartdev.json` +/// ``` +/// { +/// "firstRun": false, +/// "enabled": false, <-- THIS USER HAS OPTED OUT +/// "disclosureShown": true, +/// "clientId": "52710e60-7c70-4335-b3a4-9d922630f12a" +/// } +/// ``` /// /// Flutter: `$HOME/.flutter` +/// ``` +/// { +/// "firstRun": false, +/// "clientId": "4c3a3d1e-e545-47e7-b4f8-10129f6ab169", +/// "enabled": false <-- THIS USER HAS OPTED OUT +/// } +/// ``` /// /// Devtools: `$HOME/.flutter-devtools/.devtools` -bool legacyOptOut({ - required FileSystem fs, - required Directory home, -}) { - final dartLegacyConfigFile = - fs.file(p.join(home.path, '.dart', 'dartdev.json')); - final flutterLegacyConfigFile = fs.file(p.join(home.path, '.flutter')); - final devtoolsLegacyConfigFile = - fs.file(p.join(home.path, '.flutter-devtools', '.devtools')); - - // Example of what the file looks like for dart - // - // { - // "firstRun": false, - // "enabled": false, <-- THIS USER HAS OPTED OUT - // "disclosureShown": true, - // "clientId": "52710e60-7c70-4335-b3a4-9d922630f12a" - // } - if (dartLegacyConfigFile.existsSync()) { - try { - // Read in the json object into a Map and check for - // the enabled key being set to false; this means the user - // has opted out of analytics for dart - final dartObj = jsonDecode(dartLegacyConfigFile.readAsStringSync()) - as Map; - if (dartObj.containsKey('enabled') && dartObj['enabled'] == false) { - return true; - } - } on FormatException { - // In the case of an error when parsing the json file, return true - // which will result in the user being opted out of unified_analytics - // - // A corrupted file could mean they opted out previously but for some - // reason, the file was written incorrectly - return true; - } on FileSystemException { - return true; - } - } - - // Example of what the file looks like for flutter - // - // { - // "firstRun": false, - // "clientId": "4c3a3d1e-e545-47e7-b4f8-10129f6ab169", - // "enabled": false <-- THIS USER HAS OPTED OUT - // } - if (flutterLegacyConfigFile.existsSync()) { - try { - // Same process as above for dart - final flutterObj = jsonDecode(dartLegacyConfigFile.readAsStringSync()) - as Map; - if (flutterObj.containsKey('enabled') && flutterObj['enabled'] == false) { +/// ``` +/// { +/// "analyticsEnabled": false, <-- THIS USER HAS OPTED OUT +/// "isFirstRun": false, +/// "lastReleaseNotesVersion": "2.31.0", +/// "2023-Q4": { +/// "surveyActionTaken": false, +/// "surveyShownCount": 0 +/// } +/// } +/// ``` +bool legacyOptOut({required FileSystem fs, required Directory home}) { + // List of Maps for each of the config file, `key` refers to the + // key in the json file that indicates if the user has been opted + // out or not + final legacyConfigFiles = [ + { + 'tool': DashTool.dartTool, + 'file': fs.file(p.join(home.path, '.dart', 'dartdev.json')), + 'key': 'enabled', + }, + { + 'tool': DashTool.flutterTool, + 'file': fs.file(p.join(home.path, '.flutter')), + 'key': 'enabled', + }, + { + 'tool': DashTool.devtools, + 'file': fs.file(p.join(home.path, '.flutter-devtools', '.devtools')), + 'key': 'analyticsEnabled', + }, + ]; + for (final legacyConfigObj in legacyConfigFiles) { + final legacyFile = legacyConfigObj['file']! as File; + final lookupKey = legacyConfigObj['key']! as String; + + if (legacyFile.existsSync()) { + try { + final legacyFileObj = + jsonDecode(legacyFile.readAsStringSync()) as Map; + if (legacyFileObj.containsKey(lookupKey) && + legacyFileObj[lookupKey] == false) { + return true; + } + } on FormatException { + // In the case of an error when parsing the json file, return true + // which will result in the user being opted out of unified_analytics + // + // A corrupted file could mean they opted out previously but for some + // reason, the file was written incorrectly return true; - } - } on FormatException { - // In the case of an error when parsing the json file, return true - // which will result in the user being opted out of unified_analytics - // - // A corrupted file could mean they opted out previously but for some - // reason, the file was written incorrectly - return true; - } on FileSystemException { - return true; - } - } - - // Example of what the file looks like for devtools - // - // { - // "analyticsEnabled": false, <-- THIS USER HAS OPTED OUT - // "isFirstRun": false, - // "lastReleaseNotesVersion": "2.31.0", - // "2023-Q4": { - // "surveyActionTaken": false, - // "surveyShownCount": 0 - // } - // } - if (devtoolsLegacyConfigFile.existsSync()) { - try { - final devtoolsObj = - jsonDecode(devtoolsLegacyConfigFile.readAsStringSync()) - as Map; - if (devtoolsObj.containsKey('analyticsEnabled') && - devtoolsObj['analyticsEnabled'] == false) { + } on FileSystemException { return true; } - } on FormatException { - // In the case of an error when parsing the json file, return true - // which will result in the user being opted out of unified_analytics - // - // A corrupted file could mean they opted out previously but for some - // reason, the file was written incorrectly - return true; - } on FileSystemException { - return true; } } From e313828709724607f0e5bca8ed9c948599e8055b Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Thu, 1 Feb 2024 17:33:13 -0500 Subject: [PATCH 08/20] Only expose `sentEvents` on the `FakeAnalytics` instance --- pkgs/unified_analytics/lib/src/analytics.dart | 36 +++++-------------- .../test/log_handler_test.dart | 4 +-- .../test/unified_analytics_test.dart | 4 +-- 3 files changed, 13 insertions(+), 31 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index 969c62913..a231bd324 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -187,7 +187,7 @@ abstract class Analytics { int toolsMessageVersion = kToolsMessageVersion, String toolsMessage = kToolsMessage, }) => - AnalyticsImpl( + FakeAnalytics( tool: tool, homeDirectory: homeDirectory, flutterChannel: flutterChannel, @@ -203,7 +203,6 @@ abstract class Analytics { initializedSurveys: [], ), gaClient: gaClient ?? const FakeGAClient(), - enableAsserts: true, clientIde: clientIde, enabledFeatures: enabledFeatures, ); @@ -225,15 +224,6 @@ abstract class Analytics { /// out of the configuration file. Map get parsedTools; - /// Use this list to check for events that have been emitted when - /// invoking the send method. - /// - /// This list will only get populated when running the [Analytics.test] - /// constructor, [Analytics.development], or an instance of [FakeAnalytics]. - /// It will otherwise remain empty when used in production. - @visibleForTesting - List get sentEvents; - /// Boolean that lets the client know if they should display the message. bool get shouldShowMessage; @@ -356,10 +346,6 @@ class AnalyticsImpl implements Analytics { /// from the [GAClient]. final _futures = >[]; - /// Stores the events that have been sent while the instance exists. - /// Events will only be stored in this list if - final List _sentEvents = []; - AnalyticsImpl({ required this.tool, required Directory homeDirectory, @@ -496,9 +482,6 @@ class AnalyticsImpl implements Analytics { @override Map get parsedTools => _configHandler.parsedTools; - @override - List get sentEvents => _sentEvents; - @override bool get shouldShowMessage => _showMessage; @@ -622,12 +605,8 @@ class AnalyticsImpl implements Analytics { // This will be set to true by default if running the development or // test constructors - // - // It will also be enabled by default for the - // [FakeAnalytics] instance if (_enableAsserts) { checkBody(body); - _sentEvents.add(event); } _logHandler.save(data: body); @@ -728,6 +707,10 @@ class AnalyticsImpl implements Analytics { /// workflow. Invoking the [send] method on this instance will not make any /// network requests to Google Analytics. class FakeAnalytics extends AnalyticsImpl { + /// Use this list to check for events that have been emitted when + /// invoking the send method + final List sentEvents = []; + /// Class to use when you want to see which events were sent FakeAnalytics({ required super.tool, @@ -740,10 +723,12 @@ class FakeAnalytics extends AnalyticsImpl { super.flutterVersion, super.clientIde, super.enabledFeatures, + int? toolsMessageVersion, + GAClient? gaClient, }) : super( - gaClient: const FakeGAClient(), + gaClient: gaClient ?? const FakeGAClient(), enableAsserts: true, - toolsMessageVersion: kToolsMessageVersion, + toolsMessageVersion: toolsMessageVersion ?? kToolsMessageVersion, ); @override @@ -800,9 +785,6 @@ class NoOpAnalytics implements Analytics { @override String get clientId => staticClientId; - @override - List get sentEvents => []; - @override void clientShowedMessage() {} diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index fd6c81f06..a1f494a6e 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -13,7 +13,7 @@ import 'package:unified_analytics/src/utils.dart'; import 'package:unified_analytics/unified_analytics.dart'; void main() { - late Analytics analytics; + late FakeAnalytics analytics; late Directory homeDirectory; late FileSystem fs; late File logFile; @@ -51,7 +51,7 @@ void main() { dartVersion: 'dartVersion', fs: fs, platform: DevicePlatform.macos, - ); + ) as FakeAnalytics; }); test('Ensure that log file is created', () { diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 8a28bb2f9..e0bbffbec 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -25,7 +25,7 @@ void main() { late Directory home; late Directory dartToolDirectory; late Analytics initializationAnalytics; - late Analytics analytics; + late FakeAnalytics analytics; late File clientIdFile; late File sessionFile; late File configFile; @@ -96,7 +96,7 @@ void main() { fs: fs, platform: platform, clientIde: clientIde, - ); + ) as FakeAnalytics; analytics.clientShowedMessage(); // The 3 files that should have been generated From b0960ead3a414a9b99f5b3e2e67523750d34f1d6 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Thu, 1 Feb 2024 17:36:38 -0500 Subject: [PATCH 09/20] Bump version --- pkgs/unified_analytics/CHANGELOG.md | 5 +++++ pkgs/unified_analytics/lib/src/analytics.dart | 2 -- pkgs/unified_analytics/lib/src/constants.dart | 2 +- pkgs/unified_analytics/pubspec.yaml | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkgs/unified_analytics/CHANGELOG.md b/pkgs/unified_analytics/CHANGELOG.md index 25ed04f8c..3605705b7 100644 --- a/pkgs/unified_analytics/CHANGELOG.md +++ b/pkgs/unified_analytics/CHANGELOG.md @@ -1,3 +1,8 @@ +## 5.8.2 + +- Added new event `Event.analyticsException` to track internal errors for this package +- Redirecting the `Analytics.test` factory to return an instance of `FakeAnalytics` + ## 5.8.1 - Refactor logic for `okToSend` and `shouldShowMessage` diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index a231bd324..3c0df0787 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -603,8 +603,6 @@ class AnalyticsImpl implements Analytics { userProperty: userProperty, ); - // This will be set to true by default if running the development or - // test constructors if (_enableAsserts) { checkBody(body); } diff --git a/pkgs/unified_analytics/lib/src/constants.dart b/pkgs/unified_analytics/lib/src/constants.dart index 48606f032..cc7137997 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.1'; +const String kPackageVersion = '5.8.2'; /// 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 ef8808f6d..eace0d02f 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.1 +version: 5.8.2 repository: https://github.com/dart-lang/tools/tree/main/pkgs/unified_analytics environment: From 5307fde758ccfc7f6fd81eeae2715b7a51152040 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Thu, 1 Feb 2024 17:37:03 -0500 Subject: [PATCH 10/20] Misc --- pkgs/unified_analytics/lib/src/analytics.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index 3c0df0787..dd0b1a889 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -603,9 +603,7 @@ class AnalyticsImpl implements Analytics { userProperty: userProperty, ); - if (_enableAsserts) { - checkBody(body); - } + if (_enableAsserts) checkBody(body); _logHandler.save(data: body); From 48abbd8d6e7f698e2d198135bcd66859865b1cd9 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Thu, 1 Feb 2024 17:45:44 -0500 Subject: [PATCH 11/20] Convert to wip --- pkgs/unified_analytics/CHANGELOG.md | 2 +- pkgs/unified_analytics/lib/src/constants.dart | 2 +- pkgs/unified_analytics/pubspec.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkgs/unified_analytics/CHANGELOG.md b/pkgs/unified_analytics/CHANGELOG.md index 3605705b7..6e6e6b9ee 100644 --- a/pkgs/unified_analytics/CHANGELOG.md +++ b/pkgs/unified_analytics/CHANGELOG.md @@ -1,4 +1,4 @@ -## 5.8.2 +## 5.8.2-wip - Added new event `Event.analyticsException` to track internal errors for this package - Redirecting the `Analytics.test` factory to return an instance of `FakeAnalytics` diff --git a/pkgs/unified_analytics/lib/src/constants.dart b/pkgs/unified_analytics/lib/src/constants.dart index cc7137997..77c8892d6 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.2'; +const String kPackageVersion = '5.8.2-wip'; /// 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 eace0d02f..b090399e4 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.2 +version: 5.8.2-wip repository: https://github.com/dart-lang/tools/tree/main/pkgs/unified_analytics environment: From 6079595a795df3ab3f68e76e70fe3308185fc840 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Thu, 8 Feb 2024 15:08:21 -0500 Subject: [PATCH 12/20] Pass send method instead of `Analytics` + nits --- pkgs/unified_analytics/lib/src/analytics.dart | 7 +++++-- pkgs/unified_analytics/lib/src/enums.dart | 2 +- pkgs/unified_analytics/lib/src/event.dart | 6 ++++-- pkgs/unified_analytics/lib/src/log_handler.dart | 13 +++++++------ pkgs/unified_analytics/lib/src/session.dart | 10 +++++----- .../test/unified_analytics_test.dart | 2 +- 6 files changed, 23 insertions(+), 17 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index dd0b1a889..cd43af12b 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -25,6 +25,9 @@ import 'survey_handler.dart'; import 'user_property.dart'; import 'utils.dart'; +/// For passing the [Analytics.send] method to classes created by [Analytics]. +typedef SendFunction = void Function(Event event); + abstract class Analytics { /// The default factory constructor that will return an implementation /// of the [Analytics] abstract class using the [LocalFileSystem]. @@ -420,7 +423,7 @@ class AnalyticsImpl implements Analytics { _sessionHandler = Session( homeDirectory: homeDirectory, fs: fs, - analyticsInstance: this, + sendFunction: send, ); userProperty = UserProperty( session: _sessionHandler, @@ -442,7 +445,7 @@ class AnalyticsImpl implements Analytics { _logHandler = LogHandler( fs: fs, homeDirectory: homeDirectory, - analyticsInstance: this, + sendFunction: send, ); } diff --git a/pkgs/unified_analytics/lib/src/enums.dart b/pkgs/unified_analytics/lib/src/enums.dart index a3e9dac92..1bceab116 100644 --- a/pkgs/unified_analytics/lib/src/enums.dart +++ b/pkgs/unified_analytics/lib/src/enums.dart @@ -24,7 +24,7 @@ enum DashEvent { ), analyticsException( label: 'analytics_exception', - description: 'Errors that occur within the package for logging', + description: 'Errors that are encountered within package:unified_analytics', ), exception( label: 'exception', diff --git a/pkgs/unified_analytics/lib/src/event.dart b/pkgs/unified_analytics/lib/src/event.dart index deb38a396..bfca01ff8 100644 --- a/pkgs/unified_analytics/lib/src/event.dart +++ b/pkgs/unified_analytics/lib/src/event.dart @@ -21,8 +21,10 @@ final class Event { /// Event that is emitted when an error occurs within /// `package:unified_analytics`, tools that are using this package - /// should not use this event constructor. Instead they should use - /// the more generic [Event.exception] constructor. + /// should not use this event constructor. + /// + /// Tools using this package should instead use the more generic + /// [Event.exception] constructor. /// /// [workflow] - refers to what process caused the error, such as /// "LogHandler.logFileStats". diff --git a/pkgs/unified_analytics/lib/src/log_handler.dart b/pkgs/unified_analytics/lib/src/log_handler.dart index 3ba0f172b..092b80c6d 100644 --- a/pkgs/unified_analytics/lib/src/log_handler.dart +++ b/pkgs/unified_analytics/lib/src/log_handler.dart @@ -8,8 +8,9 @@ import 'package:clock/clock.dart'; import 'package:file/file.dart'; import 'package:path/path.dart' as p; -import '../unified_analytics.dart'; +import 'analytics.dart'; import 'constants.dart'; +import 'event.dart'; import 'initializer.dart'; /// Data class that will be returned when analyzing the @@ -161,7 +162,7 @@ class LogHandler { final FileSystem fs; final Directory homeDirectory; final File logFile; - final Analytics _analyticsInstance; + final SendFunction _sendFunction; /// Ensure we are only sending once per invocation for this class. var _errorEventSent = false; @@ -171,13 +172,13 @@ class LogHandler { LogHandler({ required this.fs, required this.homeDirectory, - required Analytics analyticsInstance, + required SendFunction sendFunction, }) : logFile = fs.file(p.join( homeDirectory.path, kDartToolDirectoryName, kLogFileName, )), - _analyticsInstance = analyticsInstance; + _sendFunction = sendFunction; /// Get stats from the persisted log file. /// @@ -195,7 +196,7 @@ class LogHandler { return LogItem.fromRecord(jsonDecode(e) as Map); } on FormatException catch (err) { if (!_errorEventSent) { - _analyticsInstance.send(Event.analyticsException( + _sendFunction(Event.analyticsException( workflow: 'LogFileStats.logFileStats', error: err.runtimeType.toString(), description: 'message: ${err.message}\nsource: ${err.source}', @@ -206,7 +207,7 @@ class LogHandler { // ignore: avoid_catching_errors } on TypeError catch (err) { if (!_errorEventSent) { - _analyticsInstance.send(Event.analyticsException( + _sendFunction(Event.analyticsException( workflow: 'LogFileStats.logFileStats', error: err.runtimeType.toString(), )); diff --git a/pkgs/unified_analytics/lib/src/session.dart b/pkgs/unified_analytics/lib/src/session.dart index 33d3ecdad..58777fa5a 100644 --- a/pkgs/unified_analytics/lib/src/session.dart +++ b/pkgs/unified_analytics/lib/src/session.dart @@ -17,7 +17,7 @@ class Session { final Directory homeDirectory; final FileSystem fs; final File sessionFile; - final Analytics _analyticsInstance; + final SendFunction _sendFunction; late int _sessionId; late int _lastPing; @@ -28,10 +28,10 @@ class Session { Session({ required this.homeDirectory, required this.fs, - required Analytics analyticsInstance, + required SendFunction sendFunction, }) : sessionFile = fs.file(p.join( homeDirectory.path, kDartToolDirectoryName, kSessionFileName)), - _analyticsInstance = analyticsInstance { + _sendFunction = sendFunction { _refreshSessionData(); } @@ -98,7 +98,7 @@ class Session { Initializer.createSessionFile(sessionFile: sessionFile); if (!_errorEventSent) { - _analyticsInstance.send(Event.analyticsException( + _sendFunction(Event.analyticsException( workflow: 'Session._refreshSessionData', error: err.runtimeType.toString(), description: 'message: ${err.message}\nsource: ${err.source}', @@ -111,7 +111,7 @@ class Session { Initializer.createSessionFile(sessionFile: sessionFile); if (!_errorEventSent) { - _analyticsInstance.send(Event.analyticsException( + _sendFunction(Event.analyticsException( workflow: 'Session._refreshSessionData', error: err.runtimeType.toString(), description: err.osError?.toString(), diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index e0bbffbec..868ae16f4 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -119,7 +119,7 @@ void main() { session: Session( homeDirectory: home, fs: fs, - analyticsInstance: analytics, + sendFunction: analytics.send, ), flutterChannel: flutterChannel, host: platform.label, From 0120151485a747950ed03d522c4fc712c5edd116 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Fri, 9 Feb 2024 12:45:50 -0500 Subject: [PATCH 13/20] `ErrorHandler` class created + used in session --- pkgs/unified_analytics/lib/src/analytics.dart | 8 +++- .../lib/src/error_handler.dart | 32 ++++++++++++++++ pkgs/unified_analytics/lib/src/session.dart | 37 +++++++------------ .../test/unified_analytics_test.dart | 3 +- 4 files changed, 55 insertions(+), 25 deletions(-) create mode 100644 pkgs/unified_analytics/lib/src/error_handler.dart diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index cd43af12b..0f3164b1b 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -16,6 +16,7 @@ import 'asserts.dart'; import 'config_handler.dart'; import 'constants.dart'; import 'enums.dart'; +import 'error_handler.dart'; import 'event.dart'; import 'ga_client.dart'; import 'initializer.dart'; @@ -327,6 +328,7 @@ class AnalyticsImpl implements Analytics { late final UserProperty userProperty; late final LogHandler _logHandler; late final Session _sessionHandler; + late final ErrorHandler _errorHandler; final int toolsMessageVersion; /// Tells the client if they need to show a message to the @@ -416,6 +418,10 @@ class AnalyticsImpl implements Analytics { p.join(homeDirectory.path, kDartToolDirectoryName, kClientIdFileName)); _clientId = _clientIdFile.readAsStringSync(); + // Initialization for the error handling class that will prevent duplicate + // [Event.analyticsException] events from being sent to GA4 + _errorHandler = ErrorHandler(sendFunction: send); + // Initialize the user property class that will be attached to // each event that is sent to Google Analytics -- it will be responsible // for getting the session id or rolling the session if the duration @@ -423,7 +429,7 @@ class AnalyticsImpl implements Analytics { _sessionHandler = Session( homeDirectory: homeDirectory, fs: fs, - sendFunction: send, + errorHandler: _errorHandler, ); userProperty = UserProperty( session: _sessionHandler, diff --git a/pkgs/unified_analytics/lib/src/error_handler.dart b/pkgs/unified_analytics/lib/src/error_handler.dart new file mode 100644 index 000000000..5937db93a --- /dev/null +++ b/pkgs/unified_analytics/lib/src/error_handler.dart @@ -0,0 +1,32 @@ +// 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 'analytics.dart'; +import 'enums.dart'; +import 'event.dart'; + +class ErrorHandler { + /// Stores each of the events that have been sent to GA4 so that the + /// same error doesn't get sent twice. + final Set _sentErrorEvents = {}; + final SendFunction _sendFunction; + + /// Handles any errors encountered within package:unified_analytics. + ErrorHandler({required SendFunction sendFunction}) + : _sendFunction = sendFunction; + + /// Sends the encountered error [Event.analyticsException] to GA4 backend. + /// + /// This method will not send the event to GA4 if it has already been + /// sent before during the current process. + void log(Event event) { + if (event.eventName != DashEvent.analyticsException || + _sentErrorEvents.contains(event)) { + return; + } + + _sendFunction(event); + _sentErrorEvents.add(event); + } +} diff --git a/pkgs/unified_analytics/lib/src/session.dart b/pkgs/unified_analytics/lib/src/session.dart index 58777fa5a..3e603aeec 100644 --- a/pkgs/unified_analytics/lib/src/session.dart +++ b/pkgs/unified_analytics/lib/src/session.dart @@ -8,8 +8,8 @@ import 'package:clock/clock.dart'; import 'package:file/file.dart'; import 'package:path/path.dart' as p; -import 'analytics.dart'; import 'constants.dart'; +import 'error_handler.dart'; import 'event.dart'; import 'initializer.dart'; @@ -17,21 +17,18 @@ class Session { final Directory homeDirectory; final FileSystem fs; final File sessionFile; - final SendFunction _sendFunction; + final ErrorHandler _errorHandler; late int _sessionId; late int _lastPing; - /// Ensure we are only sending once per invocation for this class. - var _errorEventSent = false; - Session({ required this.homeDirectory, required this.fs, - required SendFunction sendFunction, + required ErrorHandler errorHandler, }) : sessionFile = fs.file(p.join( homeDirectory.path, kDartToolDirectoryName, kSessionFileName)), - _sendFunction = sendFunction { + _errorHandler = errorHandler { _refreshSessionData(); } @@ -97,27 +94,21 @@ class Session { } on FormatException catch (err) { Initializer.createSessionFile(sessionFile: sessionFile); - if (!_errorEventSent) { - _sendFunction(Event.analyticsException( - workflow: 'Session._refreshSessionData', - error: err.runtimeType.toString(), - description: 'message: ${err.message}\nsource: ${err.source}', - )); - _errorEventSent = true; - } + _errorHandler.log(Event.analyticsException( + workflow: 'Session._refreshSessionData', + error: err.runtimeType.toString(), + description: 'message: ${err.message}\nsource: ${err.source}', + )); parseContents(); } on FileSystemException catch (err) { Initializer.createSessionFile(sessionFile: sessionFile); - if (!_errorEventSent) { - _sendFunction(Event.analyticsException( - workflow: 'Session._refreshSessionData', - error: err.runtimeType.toString(), - description: err.osError?.toString(), - )); - _errorEventSent = true; - } + _errorHandler.log(Event.analyticsException( + workflow: 'Session._refreshSessionData', + error: err.runtimeType.toString(), + description: err.osError?.toString(), + )); parseContents(); } diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 868ae16f4..5068c61bd 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -14,6 +14,7 @@ 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'; @@ -119,7 +120,7 @@ void main() { session: Session( homeDirectory: home, fs: fs, - sendFunction: analytics.send, + errorHandler: ErrorHandler(sendFunction: analytics.send), ), flutterChannel: flutterChannel, host: platform.label, From ef19bb6b69fbbf56f0f927ebeafcbdae421aa7d9 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Fri, 9 Feb 2024 13:21:37 -0500 Subject: [PATCH 14/20] Use `ErrorHandler` with `LogHandler` --- pkgs/unified_analytics/lib/src/analytics.dart | 2 +- pkgs/unified_analytics/lib/src/log_handler.dart | 12 ++++++------ .../test/unified_analytics_test.dart | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index 0f3164b1b..ec0f57a1f 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -451,7 +451,7 @@ class AnalyticsImpl implements Analytics { _logHandler = LogHandler( fs: fs, homeDirectory: homeDirectory, - sendFunction: send, + errorHandler: _errorHandler, ); } diff --git a/pkgs/unified_analytics/lib/src/log_handler.dart b/pkgs/unified_analytics/lib/src/log_handler.dart index 092b80c6d..bf94462a6 100644 --- a/pkgs/unified_analytics/lib/src/log_handler.dart +++ b/pkgs/unified_analytics/lib/src/log_handler.dart @@ -8,8 +8,8 @@ import 'package:clock/clock.dart'; import 'package:file/file.dart'; import 'package:path/path.dart' as p; -import 'analytics.dart'; import 'constants.dart'; +import 'error_handler.dart'; import 'event.dart'; import 'initializer.dart'; @@ -162,7 +162,7 @@ class LogHandler { final FileSystem fs; final Directory homeDirectory; final File logFile; - final SendFunction _sendFunction; + final ErrorHandler _errorHandler; /// Ensure we are only sending once per invocation for this class. var _errorEventSent = false; @@ -172,13 +172,13 @@ class LogHandler { LogHandler({ required this.fs, required this.homeDirectory, - required SendFunction sendFunction, + required ErrorHandler errorHandler, }) : logFile = fs.file(p.join( homeDirectory.path, kDartToolDirectoryName, kLogFileName, )), - _sendFunction = sendFunction; + _errorHandler = errorHandler; /// Get stats from the persisted log file. /// @@ -196,7 +196,7 @@ class LogHandler { return LogItem.fromRecord(jsonDecode(e) as Map); } on FormatException catch (err) { if (!_errorEventSent) { - _sendFunction(Event.analyticsException( + _errorHandler.log(Event.analyticsException( workflow: 'LogFileStats.logFileStats', error: err.runtimeType.toString(), description: 'message: ${err.message}\nsource: ${err.source}', @@ -207,7 +207,7 @@ class LogHandler { // ignore: avoid_catching_errors } on TypeError catch (err) { if (!_errorEventSent) { - _sendFunction(Event.analyticsException( + _errorHandler.log(Event.analyticsException( workflow: 'LogFileStats.logFileStats', error: err.runtimeType.toString(), )); diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 5068c61bd..9eb1353f1 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -100,7 +100,7 @@ void main() { ) as FakeAnalytics; analytics.clientShowedMessage(); - // The 3 files that should have been generated + // The 5 files that should have been generated clientIdFile = home .childDirectory(kDartToolDirectoryName) .childFile(kClientIdFileName); From 103980a735aff9c6d259d3d4bbb8c986ec1a5b73 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Fri, 9 Feb 2024 14:12:12 -0500 Subject: [PATCH 15/20] Check telemetry status in `Session` --- pkgs/unified_analytics/lib/src/analytics.dart | 1 + pkgs/unified_analytics/lib/src/session.dart | 8 +++++++- pkgs/unified_analytics/test/unified_analytics_test.dart | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index ec0f57a1f..a0d32280c 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -430,6 +430,7 @@ class AnalyticsImpl implements Analytics { homeDirectory: homeDirectory, fs: fs, errorHandler: _errorHandler, + telemetryEnabled: telemetryEnabled, ); userProperty = UserProperty( session: _sessionHandler, diff --git a/pkgs/unified_analytics/lib/src/session.dart b/pkgs/unified_analytics/lib/src/session.dart index 3e603aeec..5213c5c78 100644 --- a/pkgs/unified_analytics/lib/src/session.dart +++ b/pkgs/unified_analytics/lib/src/session.dart @@ -26,10 +26,16 @@ 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 { - _refreshSessionData(); + + // 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(); } /// This will use the data parsed from the diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 9eb1353f1..2b794b671 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -121,6 +121,7 @@ void main() { homeDirectory: home, fs: fs, errorHandler: ErrorHandler(sendFunction: analytics.send), + telemetryEnabled: analytics.telemetryEnabled, ), flutterChannel: flutterChannel, host: platform.label, From 74e7f8bf2f7e6c11b4ede7c9849003b9712fcb51 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Fri, 9 Feb 2024 14:41:03 -0500 Subject: [PATCH 16/20] Tests added for the survey handler --- .../lib/src/error_handler.dart | 2 +- pkgs/unified_analytics/lib/src/session.dart | 1 - .../test/error_handler_test.dart | 195 ++++++++++++++++++ 3 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 pkgs/unified_analytics/test/error_handler_test.dart diff --git a/pkgs/unified_analytics/lib/src/error_handler.dart b/pkgs/unified_analytics/lib/src/error_handler.dart index 5937db93a..a183cef4a 100644 --- a/pkgs/unified_analytics/lib/src/error_handler.dart +++ b/pkgs/unified_analytics/lib/src/error_handler.dart @@ -9,7 +9,7 @@ import 'event.dart'; class ErrorHandler { /// Stores each of the events that have been sent to GA4 so that the /// same error doesn't get sent twice. - final Set _sentErrorEvents = {}; + final List _sentErrorEvents = []; final SendFunction _sendFunction; /// Handles any errors encountered within package:unified_analytics. diff --git a/pkgs/unified_analytics/lib/src/session.dart b/pkgs/unified_analytics/lib/src/session.dart index 5213c5c78..77640d68a 100644 --- a/pkgs/unified_analytics/lib/src/session.dart +++ b/pkgs/unified_analytics/lib/src/session.dart @@ -30,7 +30,6 @@ class Session { }) : 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 diff --git a/pkgs/unified_analytics/test/error_handler_test.dart b/pkgs/unified_analytics/test/error_handler_test.dart new file mode 100644 index 000000000..3cd435d71 --- /dev/null +++ b/pkgs/unified_analytics/test/error_handler_test.dart @@ -0,0 +1,195 @@ +// 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 'dart:io' as io; + +import 'package:file/file.dart'; +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/unified_analytics.dart'; + +void main() { + late FileSystem fs; + late Directory home; + late FakeAnalytics initializationAnalytics; + late FakeAnalytics analytics; + late File sessionFile; + late File logFile; + + const homeDirName = 'home'; + const initialTool = DashTool.flutterTool; + const measurementId = 'measurementId'; + const apiSecret = 'apiSecret'; + const toolsMessageVersion = 1; + const toolsMessage = 'toolsMessage'; + const flutterChannel = 'flutterChannel'; + const flutterVersion = 'flutterVersion'; + const dartVersion = 'dartVersion'; + const platform = DevicePlatform.macos; + const clientIde = 'VSCode'; + + final testEvent = Event.codeSizeAnalysis(platform: 'platform'); + + setUp(() { + // Setup the filesystem with the home directory + final fsStyle = + io.Platform.isWindows ? FileSystemStyle.windows : FileSystemStyle.posix; + fs = MemoryFileSystem.test(style: fsStyle); + home = fs.directory(homeDirName); + + // This is the first analytics instance that will be used to demonstrate + // that events will not be sent with the first run of analytics + initializationAnalytics = Analytics.test( + tool: initialTool, + homeDirectory: home, + measurementId: measurementId, + apiSecret: apiSecret, + flutterChannel: flutterChannel, + toolsMessageVersion: toolsMessageVersion, + toolsMessage: toolsMessage, + flutterVersion: flutterVersion, + dartVersion: dartVersion, + fs: fs, + platform: platform, + ) as FakeAnalytics; + expect(initializationAnalytics.shouldShowMessage, true); + initializationAnalytics.clientShowedMessage(); + expect(initializationAnalytics.shouldShowMessage, false); + + // The main analytics instance, other instances can be spawned within tests + // to test how to instances running together work + // + // This instance should have the same parameters as the one above for + // [initializationAnalytics] + 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(); + + // The files that should have been generated that will be used for tests + sessionFile = + home.childDirectory(kDartToolDirectoryName).childFile(kSessionFileName); + logFile = + home.childDirectory(kDartToolDirectoryName).childFile(kLogFileName); + }); + + group('Session handler:', () { + test('no error when opted out already and opting in', () async { + // When we opt out from an analytics instance, we clear the contents of + // session file, as required by the privacy document. When creating a + // second instance of [Analytics], it should not detect that the file is + // empty and recreate it, it should remain opted out and no error event + // should have been sent + await analytics.setTelemetry(false); + expect(analytics.telemetryEnabled, false); + expect(sessionFile.readAsStringSync(), isEmpty); + + final secondAnalytics = 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; + expect(sessionFile.readAsStringSync(), isEmpty); + expect(secondAnalytics.telemetryEnabled, false); + + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + isEmpty, + ); + expect( + secondAnalytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + isEmpty, + ); + + await secondAnalytics.setTelemetry(true); + expect(sessionFile.readAsStringSync(), isNotEmpty, + reason: 'Toggling telemetry should bring back the session data'); + }); + test('only sends one event for FormatException', () { + // Begin with the session file empty, it should recreate the file + // and send an error event + sessionFile.writeAsStringSync(''); + expect(sessionFile.readAsStringSync(), isEmpty); + analytics.send(testEvent); + expect(sessionFile.readAsStringSync(), isNotEmpty); + + final matchingEvents = analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException); + expect(matchingEvents, hasLength(1)); + + // Making the file empty again and sending an event should not send + // an additional event + sessionFile.writeAsStringSync(''); + expect(sessionFile.readAsStringSync(), isEmpty); + analytics.send(testEvent); + expect(sessionFile.readAsStringSync(), isNotEmpty); + + final secondMatchingEvents = analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException); + expect(secondMatchingEvents, hasLength(1), + reason: 'We should not have added a new error event'); + }); + + test('only sends one event for FileSystemException', () { + // Deleting the session file should cause the file system exception and + // sending a new event should log the error the first time and recreate + // the file. If we delete the file again and attempt to send an event, + // the session file should get recreated without sending a second error. + sessionFile.deleteSync(); + expect(sessionFile.existsSync(), isFalse); + analytics.send(testEvent); + + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + hasLength(1), + ); + expect(sessionFile.existsSync(), isTrue); + expect(sessionFile.readAsStringSync(), isNotEmpty); + + // Remove the file again and send an event + sessionFile.deleteSync(); + expect(sessionFile.existsSync(), isFalse); + 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); + }); + }); + + group('Log handler:', () { + // TODO: add tests for the log handler + }); +} From b67ce4b2b9a6ed7450eeceecc567e93a37b6ede2 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Fri, 9 Feb 2024 14:43:49 -0500 Subject: [PATCH 17/20] Fix error --- pkgs/unified_analytics/test/error_handler_test.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkgs/unified_analytics/test/error_handler_test.dart b/pkgs/unified_analytics/test/error_handler_test.dart index 3cd435d71..809e1fa53 100644 --- a/pkgs/unified_analytics/test/error_handler_test.dart +++ b/pkgs/unified_analytics/test/error_handler_test.dart @@ -190,6 +190,8 @@ void main() { }); group('Log handler:', () { - // TODO: add tests for the log handler + test('only sends one event for FormatException', () { + expect(logFile.existsSync(), isTrue); + }); }); } From 1bd099feffb1cb157fced06b3ab800dbe55c8328 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Tue, 13 Feb 2024 10:35:36 -0500 Subject: [PATCH 18/20] Tests added for log handler exceptions --- .../lib/src/log_handler.dart | 171 ++++++++---------- .../test/error_handler_test.dart | 54 ++++++ 2 files changed, 131 insertions(+), 94 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/log_handler.dart b/pkgs/unified_analytics/lib/src/log_handler.dart index bf94462a6..c406cce94 100644 --- a/pkgs/unified_analytics/lib/src/log_handler.dart +++ b/pkgs/unified_analytics/lib/src/log_handler.dart @@ -164,9 +164,6 @@ class LogHandler { final File logFile; final ErrorHandler _errorHandler; - /// Ensure we are only sending once per invocation for this class. - var _errorEventSent = false; - /// A log handler constructor that will delegate saving /// logs and retrieving stats from the persisted log. LogHandler({ @@ -195,24 +192,20 @@ class LogHandler { try { return LogItem.fromRecord(jsonDecode(e) as Map); } on FormatException catch (err) { - if (!_errorEventSent) { - _errorHandler.log(Event.analyticsException( - workflow: 'LogFileStats.logFileStats', - error: err.runtimeType.toString(), - description: 'message: ${err.message}\nsource: ${err.source}', - )); - _errorEventSent = true; - } + _errorHandler.log(Event.analyticsException( + workflow: 'LogFileStats.logFileStats', + error: err.runtimeType.toString(), + description: 'message: ${err.message}\nsource: ${err.source}', + )); + return null; // ignore: avoid_catching_errors } on TypeError catch (err) { - if (!_errorEventSent) { - _errorHandler.log(Event.analyticsException( - workflow: 'LogFileStats.logFileStats', - error: err.runtimeType.toString(), - )); - _errorEventSent = true; - } + _errorHandler.log(Event.analyticsException( + workflow: 'LogFileStats.logFileStats', + error: err.runtimeType.toString(), + )); + return null; } }) @@ -397,82 +390,72 @@ class LogItem { return null; } - // Using a try/except here to parse out the fields if possible, - // if not, it will quietly return null and won't get processed - // downstream - try { - // Parse out values from the top level key = 'events' and return - // a map for the one event in the value - final eventProp = - (record['events']! as List).first as Map; - final eventName = eventProp['name'] as String; - - // Parse the data out of the `user_properties` value - final userProps = record['user_properties'] as Map; - - // Parse out the values from the top level key = 'user_properties` - final sessionId = - (userProps['session_id']! as Map)['value'] as int?; - final flutterChannel = (userProps['flutter_channel']! - as Map)['value'] as String?; - final host = - (userProps['host']! as Map)['value'] as String?; - final flutterVersion = (userProps['flutter_version']! - as Map)['value'] as String?; - final dartVersion = (userProps['dart_version']! - as Map)['value'] as String?; - final tool = - (userProps['tool']! as Map)['value'] as String?; - final localTimeString = (userProps['local_time']! - as Map)['value'] as String?; - final hostOsVersion = (userProps['host_os_version']! - as Map)['value'] as String?; - final locale = - (userProps['locale']! as Map)['value'] as String?; - final clientIde = (userProps['client_ide']! - as Map)['value'] as String?; - - // If any of the above values are null, return null since that - // indicates the record is malformed; note that `flutter_version`, - // `flutter_channel`, and `client_ide` are nullable fields in the log file - final values = [ - // Values associated with the top level key = 'events' - eventName, - - // Values associated with the top level key = 'events' - sessionId, - host, - dartVersion, - tool, - localTimeString, - hostOsVersion, - locale, - ]; - for (var value in values) { - if (value == null) return null; - } - - // Parse the local time from the string extracted - final localTime = DateTime.parse(localTimeString!).toLocal(); - - return LogItem( - eventName: eventName, - sessionId: sessionId!, - flutterChannel: flutterChannel, - host: host!, - flutterVersion: flutterVersion, - dartVersion: dartVersion!, - tool: tool!, - localTime: localTime, - hostOsVersion: hostOsVersion!, - locale: locale!, - clientIde: clientIde, - ); - // ignore: avoid_catching_errors - } on TypeError { - return null; - } on FormatException { - return null; + // Parse out values from the top level key = 'events' and return + // a map for the one event in the value + final eventProp = + (record['events']! as List).first as Map; + final eventName = eventProp['name'] as String; + + // Parse the data out of the `user_properties` value + final userProps = record['user_properties'] as Map; + + // Parse out the values from the top level key = 'user_properties` + final sessionId = + (userProps['session_id']! as Map)['value'] as int?; + final flutterChannel = (userProps['flutter_channel']! + as Map)['value'] as String?; + final host = + (userProps['host']! as Map)['value'] as String?; + final flutterVersion = (userProps['flutter_version']! + as Map)['value'] as String?; + final dartVersion = (userProps['dart_version']! + as Map)['value'] as String?; + final tool = + (userProps['tool']! as Map)['value'] as String?; + final localTimeString = + (userProps['local_time']! as Map)['value'] as String?; + final hostOsVersion = (userProps['host_os_version']! + as Map)['value'] as String?; + final locale = + (userProps['locale']! as Map)['value'] as String?; + final clientIde = + (userProps['client_ide']! as Map)['value'] as String?; + + // If any of the above values are null, return null since that + // indicates the record is malformed; note that `flutter_version`, + // `flutter_channel`, and `client_ide` are nullable fields in the log file + final values = [ + // Values associated with the top level key = 'events' + eventName, + + // Values associated with the top level key = 'events' + sessionId, + host, + dartVersion, + tool, + localTimeString, + hostOsVersion, + locale, + ]; + for (var value in values) { + if (value == null) return null; } + + // Parse the local time from the string extracted + final localTime = DateTime.parse(localTimeString!).toLocal(); + + return LogItem( + eventName: eventName, + sessionId: sessionId!, + flutterChannel: flutterChannel, + host: host!, + flutterVersion: flutterVersion, + dartVersion: dartVersion!, + tool: tool!, + localTime: localTime, + hostOsVersion: hostOsVersion!, + locale: locale!, + clientIde: clientIde, + ); } } diff --git a/pkgs/unified_analytics/test/error_handler_test.dart b/pkgs/unified_analytics/test/error_handler_test.dart index 809e1fa53..453a9aa25 100644 --- a/pkgs/unified_analytics/test/error_handler_test.dart +++ b/pkgs/unified_analytics/test/error_handler_test.dart @@ -192,6 +192,60 @@ void main() { group('Log handler:', () { test('only sends one event for FormatException', () { expect(logFile.existsSync(), isTrue); + + // Write invalid lines to the log file to have a FormatException + // thrown when trying to parse the log file + logFile.writeAsStringSync(''' +{{} +{{} +'''); + + // Send one event so that the logFileStats method returns a valid value + analytics.send(testEvent); + expect(analytics.sentEvents, hasLength(1)); + expect(logFile.readAsLinesSync(), hasLength(3)); + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + isEmpty, + ); + + // This call below will cause a FormatException while parsing the log file + final logFileStats = analytics.logFileStats(); + expect(logFileStats, isNotNull); + expect(logFileStats!.recordCount, 1, + reason: 'The error event is not counted'); + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + hasLength(1), + ); + expect(logFile.readAsLinesSync(), hasLength(4)); + }); + + test('only sends one event for TypeError', () { + expect(logFile.existsSync(), isTrue); + // Write valid json but have one of the types wrong for + // the keys so that we throw a TypeError while casting the values + // + // In the json below, we have made the session id value a string when + // it should be an integer + logFile.writeAsStringSync(''' +{"client_id":"fcd6c0d5-6582-4c36-b09e-3ecedee9145c","events":[{"name":"command_usage_values","params":{"workflow":"doctor","commandHasTerminal":true}}],"user_properties":{"session_id":{"value":"this should be a string"},"flutter_channel":{"value":"master"},"host":{"value":"macOS"},"flutter_version":{"value":"3.20.0-2.0.pre.9"},"dart_version":{"value":"3.4.0 (build 3.4.0-99.0.dev)"},"analytics_pkg_version":{"value":"5.8.1"},"tool":{"value":"flutter-tool"},"local_time":{"value":"2024-02-07 15:46:19.920784 -0500"},"host_os_version":{"value":"Version 14.3 (Build 23D56)"},"locale":{"value":"en"},"client_ide":{"value":null},"enabled_features":{"value":"enable-native-assets"}}} +'''); + expect(logFile.readAsLinesSync(), hasLength(1)); + + // Send the test event so that the LogFileStats object is not null + analytics.send(testEvent); + + final logFileStats = analytics.logFileStats(); + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + hasLength(1), + ); + expect(logFileStats, isNotNull); + expect(logFileStats!.recordCount, 1); }); }); } From 369147b14fb8d4550834c934d9d42d547e33df6c Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Tue, 13 Feb 2024 15:19:26 -0500 Subject: [PATCH 19/20] Use set for sent error messages --- pkgs/unified_analytics/lib/src/error_handler.dart | 2 +- pkgs/unified_analytics/lib/src/event.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/error_handler.dart b/pkgs/unified_analytics/lib/src/error_handler.dart index a183cef4a..5937db93a 100644 --- a/pkgs/unified_analytics/lib/src/error_handler.dart +++ b/pkgs/unified_analytics/lib/src/error_handler.dart @@ -9,7 +9,7 @@ import 'event.dart'; class ErrorHandler { /// Stores each of the events that have been sent to GA4 so that the /// same error doesn't get sent twice. - final List _sentErrorEvents = []; + final Set _sentErrorEvents = {}; final SendFunction _sendFunction; /// Handles any errors encountered within package:unified_analytics. diff --git a/pkgs/unified_analytics/lib/src/event.dart b/pkgs/unified_analytics/lib/src/event.dart index bfca01ff8..5ad7d13ce 100644 --- a/pkgs/unified_analytics/lib/src/event.dart +++ b/pkgs/unified_analytics/lib/src/event.dart @@ -709,7 +709,7 @@ final class Event { }; @override - int get hashCode => eventData.hashCode; + int get hashCode => Object.hash(eventName, jsonEncode(eventData)); @override bool operator ==(Object other) => From 39cb8098fe912a1c76e2764953cef40e7a9a3285 Mon Sep 17 00:00:00 2001 From: eliasyishak <42216813+eliasyishak@users.noreply.github.com> Date: Tue, 13 Feb 2024 15:33:55 -0500 Subject: [PATCH 20/20] Test added to check for 2 unique error events --- .../test/error_handler_test.dart | 104 +++++++++++++++++- 1 file changed, 98 insertions(+), 6 deletions(-) diff --git a/pkgs/unified_analytics/test/error_handler_test.dart b/pkgs/unified_analytics/test/error_handler_test.dart index 453a9aa25..c7156e2aa 100644 --- a/pkgs/unified_analytics/test/error_handler_test.dart +++ b/pkgs/unified_analytics/test/error_handler_test.dart @@ -139,9 +139,10 @@ void main() { analytics.send(testEvent); expect(sessionFile.readAsStringSync(), isNotEmpty); - final matchingEvents = analytics.sentEvents.where( - (element) => element.eventName == DashEvent.analyticsException); - expect(matchingEvents, hasLength(1)); + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + hasLength(1)); // Making the file empty again and sending an event should not send // an additional event @@ -150,9 +151,10 @@ void main() { analytics.send(testEvent); expect(sessionFile.readAsStringSync(), isNotEmpty); - final secondMatchingEvents = analytics.sentEvents.where( - (element) => element.eventName == DashEvent.analyticsException); - expect(secondMatchingEvents, hasLength(1), + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + hasLength(1), reason: 'We should not have added a new error event'); }); @@ -187,6 +189,43 @@ void main() { 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 + sessionFile.writeAsStringSync(''); + expect(sessionFile.readAsStringSync(), isEmpty); + analytics.send(testEvent); + expect(sessionFile.readAsStringSync(), isNotEmpty); + + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + hasLength(1)); + + // Deleting the file now before sending an additional event should + // cause a different test error + sessionFile.deleteSync(); + expect(sessionFile.existsSync(), isFalse); + + analytics.send(testEvent); + expect(sessionFile.readAsStringSync(), isNotEmpty); + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + hasLength(2)); + expect(analytics.sentEvents, hasLength(4)); + + sessionFile.deleteSync(); + expect(sessionFile.existsSync(), isFalse); + + analytics.send(testEvent); + expect(sessionFile.readAsStringSync(), isNotEmpty); + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + hasLength(2)); + }); }); group('Log handler:', () { @@ -247,5 +286,58 @@ void main() { expect(logFileStats, isNotNull); expect(logFileStats!.recordCount, 1); }); + + test('sends two unique errors', () { + expect(logFile.existsSync(), isTrue); + + // Write invalid lines to the log file to have a FormatException + // thrown when trying to parse the log file + logFile.writeAsStringSync(''' +{{} +{{} +'''); + + // Send one event so that the logFileStats method returns a valid value + analytics.send(testEvent); + expect(analytics.sentEvents, hasLength(1)); + expect(logFile.readAsLinesSync(), hasLength(3)); + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + isEmpty, + ); + + // This will cause the first error + analytics.logFileStats(); + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + hasLength(1), + ); + + // Overwrite the contents of the log file now to include something that + // will cause a TypeError by changing the expected value for session id + // from integer to a string + logFile.writeAsStringSync(''' +{"client_id":"fcd6c0d5-6582-4c36-b09e-3ecedee9145c","events":[{"name":"command_usage_values","params":{"workflow":"doctor","commandHasTerminal":true}}],"user_properties":{"session_id":{"value":"this should be a string"},"flutter_channel":{"value":"master"},"host":{"value":"macOS"},"flutter_version":{"value":"3.20.0-2.0.pre.9"},"dart_version":{"value":"3.4.0 (build 3.4.0-99.0.dev)"},"analytics_pkg_version":{"value":"5.8.1"},"tool":{"value":"flutter-tool"},"local_time":{"value":"2024-02-07 15:46:19.920784 -0500"},"host_os_version":{"value":"Version 14.3 (Build 23D56)"},"locale":{"value":"en"},"client_ide":{"value":null},"enabled_features":{"value":"enable-native-assets"}}} +'''); + expect(logFile.readAsLinesSync(), hasLength(1)); + + // This will cause the second error + analytics.logFileStats(); + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + hasLength(2), + ); + + // Attempting to cause the same error won't send another error event + analytics.logFileStats(); + expect( + analytics.sentEvents.where( + (element) => element.eventName == DashEvent.analyticsException), + hasLength(2), + ); + }); }); }