From edded56a89c38090efb4912eff95fd6ead163572 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Tue, 4 Jun 2024 23:37:16 -0700 Subject: [PATCH 1/7] refactor log_handler_test.dart --- .../test/log_handler_test.dart | 82 +++++++++++-------- 1 file changed, 47 insertions(+), 35 deletions(-) diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index 76ae5e64..afe6f878 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -13,52 +13,20 @@ import 'package:unified_analytics/src/utils.dart'; import 'package:unified_analytics/unified_analytics.dart'; void main() { - late FakeAnalytics analytics; - late Directory homeDirectory; - late MemoryFileSystem 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.fake( - tool: DashTool.flutterTool, - homeDirectory: homeDirectory, - 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.fake( - tool: DashTool.flutterTool, - homeDirectory: homeDirectory, - dartVersion: 'dartVersion', - fs: fs, - platform: DevicePlatform.macos, - ); - }); - test('Ensure that log file is created', () { + final (fs, logFile, analytics) = _setUpFakeAnalytics(); expect(logFile.existsSync(), true); }); test('LogFileStats is null before events are sent', () { + final (fs, logFile, analytics) = _setUpFakeAnalytics(); expect(analytics.logFileStats(), isNull); }); test('LogFileStats returns valid response after sent events', () async { + final (fs, logFile, analytics) = _setUpFakeAnalytics(); final countOfEventsToSend = 10; for (var i = 0; i < countOfEventsToSend; i++) { @@ -71,6 +39,8 @@ void main() { }); test('The only record in the log file is malformed', () async { + final (fs, logFile, analytics) = _setUpFakeAnalytics(); + // Write invalid json for the only log record logFile.writeAsStringSync('{{\n'); @@ -93,6 +63,8 @@ void main() { }); test('The first record is malformed, but rest are valid', () async { + final (fs, logFile, analytics) = _setUpFakeAnalytics(); + // Write invalid json for the only log record logFile.writeAsStringSync('{{\n'); @@ -109,6 +81,8 @@ void main() { }); test('Several records are malformed', () async { + final (fs, logFile, analytics) = _setUpFakeAnalytics(); + final countOfMalformedRecords = 4; for (var i = 0; i < countOfMalformedRecords; i++) { final currentContents = logFile.readAsStringSync(); @@ -136,6 +110,8 @@ void main() { }); test('Valid json but invalid keys', () { + final (fs, logFile, analytics) = _setUpFakeAnalytics(); + // The second line here is missing the "events" top level // key which should cause an error for that record only // @@ -157,6 +133,8 @@ void main() { }); test('Malformed record gets phased out after several events', () async { + final (fs, logFile, analytics) = _setUpFakeAnalytics(); + // Write invalid json for the only log record logFile.writeAsStringSync('{{\n'); @@ -192,6 +170,8 @@ void main() { }); test('Catching cast errors for each log record silently', () async { + final (fs, logFile, analytics) = _setUpFakeAnalytics(); + // Write a json array to the log file which will cause // a cast error when parsing each line logFile.writeAsStringSync('[{}, 1, 2, 3]\n'); @@ -269,3 +249,35 @@ void main() { expect(newString, testString); }); } + +(FileSystem fs, File logFile, FakeAnalytics analytics) _setUpFakeAnalytics() { + final fs = MemoryFileSystem.test(style: FileSystemStyle.posix); + final homeDirectory = fs.directory('home'); + final logFile = fs.file(p.join( + homeDirectory.path, + kDartToolDirectoryName, + kLogFileName, + )); + + // Create the initialization analytics instance to onboard the tool + final initializationAnalytics = Analytics.fake( + tool: DashTool.flutterTool, + homeDirectory: homeDirectory, + 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 + final analytics = Analytics.fake( + tool: DashTool.flutterTool, + homeDirectory: homeDirectory, + dartVersion: 'dartVersion', + fs: fs, + platform: DevicePlatform.macos, + ); + + return (fs, logFile, analytics); +} \ No newline at end of file From dd8236624cc3f26cb9d15e631a206c97145d2c2f Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Tue, 4 Jun 2024 23:43:27 -0700 Subject: [PATCH 2/7] Revert "refactor log_handler_test.dart" This reverts commit edded56a89c38090efb4912eff95fd6ead163572. --- .../test/log_handler_test.dart | 82 ++++++++----------- 1 file changed, 35 insertions(+), 47 deletions(-) diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index afe6f878..76ae5e64 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -13,20 +13,52 @@ import 'package:unified_analytics/src/utils.dart'; import 'package:unified_analytics/unified_analytics.dart'; void main() { + late FakeAnalytics analytics; + late Directory homeDirectory; + late MemoryFileSystem 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.fake( + tool: DashTool.flutterTool, + homeDirectory: homeDirectory, + 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.fake( + tool: DashTool.flutterTool, + homeDirectory: homeDirectory, + dartVersion: 'dartVersion', + fs: fs, + platform: DevicePlatform.macos, + ); + }); + test('Ensure that log file is created', () { - final (fs, logFile, analytics) = _setUpFakeAnalytics(); expect(logFile.existsSync(), true); }); test('LogFileStats is null before events are sent', () { - final (fs, logFile, analytics) = _setUpFakeAnalytics(); expect(analytics.logFileStats(), isNull); }); test('LogFileStats returns valid response after sent events', () async { - final (fs, logFile, analytics) = _setUpFakeAnalytics(); final countOfEventsToSend = 10; for (var i = 0; i < countOfEventsToSend; i++) { @@ -39,8 +71,6 @@ void main() { }); test('The only record in the log file is malformed', () async { - final (fs, logFile, analytics) = _setUpFakeAnalytics(); - // Write invalid json for the only log record logFile.writeAsStringSync('{{\n'); @@ -63,8 +93,6 @@ void main() { }); test('The first record is malformed, but rest are valid', () async { - final (fs, logFile, analytics) = _setUpFakeAnalytics(); - // Write invalid json for the only log record logFile.writeAsStringSync('{{\n'); @@ -81,8 +109,6 @@ void main() { }); test('Several records are malformed', () async { - final (fs, logFile, analytics) = _setUpFakeAnalytics(); - final countOfMalformedRecords = 4; for (var i = 0; i < countOfMalformedRecords; i++) { final currentContents = logFile.readAsStringSync(); @@ -110,8 +136,6 @@ void main() { }); test('Valid json but invalid keys', () { - final (fs, logFile, analytics) = _setUpFakeAnalytics(); - // The second line here is missing the "events" top level // key which should cause an error for that record only // @@ -133,8 +157,6 @@ void main() { }); test('Malformed record gets phased out after several events', () async { - final (fs, logFile, analytics) = _setUpFakeAnalytics(); - // Write invalid json for the only log record logFile.writeAsStringSync('{{\n'); @@ -170,8 +192,6 @@ void main() { }); test('Catching cast errors for each log record silently', () async { - final (fs, logFile, analytics) = _setUpFakeAnalytics(); - // Write a json array to the log file which will cause // a cast error when parsing each line logFile.writeAsStringSync('[{}, 1, 2, 3]\n'); @@ -249,35 +269,3 @@ void main() { expect(newString, testString); }); } - -(FileSystem fs, File logFile, FakeAnalytics analytics) _setUpFakeAnalytics() { - final fs = MemoryFileSystem.test(style: FileSystemStyle.posix); - final homeDirectory = fs.directory('home'); - final logFile = fs.file(p.join( - homeDirectory.path, - kDartToolDirectoryName, - kLogFileName, - )); - - // Create the initialization analytics instance to onboard the tool - final initializationAnalytics = Analytics.fake( - tool: DashTool.flutterTool, - homeDirectory: homeDirectory, - 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 - final analytics = Analytics.fake( - tool: DashTool.flutterTool, - homeDirectory: homeDirectory, - dartVersion: 'dartVersion', - fs: fs, - platform: DevicePlatform.macos, - ); - - return (fs, logFile, analytics); -} \ No newline at end of file From 94cf3a3f461183503b4e74f98dd6478707e3f7ec Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Wed, 5 Jun 2024 00:05:03 -0700 Subject: [PATCH 3/7] catch any FileSystemException raised when writing to log file --- .../lib/src/log_handler.dart | 23 +++++++++++-------- .../test/log_handler_test.dart | 15 ++++++++++++ 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/pkgs/unified_analytics/lib/src/log_handler.dart b/pkgs/unified_analytics/lib/src/log_handler.dart index 6a0f0585..d611b73a 100644 --- a/pkgs/unified_analytics/lib/src/log_handler.dart +++ b/pkgs/unified_analytics/lib/src/log_handler.dart @@ -275,15 +275,20 @@ class LogHandler { var records = logFile.readAsLinesSync(); final content = '${jsonEncode(data)}\n'; - // When the record count is less than the max, add as normal; - // else drop the oldest records until equal to max - if (records.length < kLogFileLength) { - logFile.writeAsStringSync(content, mode: FileMode.writeOnlyAppend); - } else { - records.add(content); - records = records.skip(records.length - kLogFileLength).toList(); - - logFile.writeAsStringSync(records.join('\n')); + try { + // When the record count is less than the max, add as normal; + // else drop the oldest records until equal to max + if (records.length < kLogFileLength) { + logFile.writeAsStringSync(content, mode: FileMode.writeOnlyAppend); + } else { + records.add(content); + records = records.skip(records.length - kLogFileLength).toList(); + + logFile.writeAsStringSync(records.join('\n')); + } + } on FileSystemException { + // Logging isn't important enough to warrant raising a + // FileSystemException that will surprise consumers of this package. } } } diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index 76ae5e64..92d3f4e1 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -9,6 +9,7 @@ import 'package:test/test.dart'; import 'package:unified_analytics/src/constants.dart'; import 'package:unified_analytics/src/enums.dart'; +import 'package:unified_analytics/src/log_handler.dart'; import 'package:unified_analytics/src/utils.dart'; import 'package:unified_analytics/unified_analytics.dart'; @@ -191,6 +192,20 @@ void main() { expect(logFile.readAsLinesSync()[0].trim(), isNot('{{')); }); + test('Catches and discards any FileSystemException raised from attempting to write to the log file', () async { + final logFilePath = 'log.txt'; + final fs = MemoryFileSystem.test(opHandle: (context, operation) { + if (context == logFilePath && operation == FileSystemOp.write) { + throw FileSystemException('writeFrom failed', logFilePath, OSError('No space left on device', 28),); + } + }); + final logFile = fs.file(logFilePath); + logFile.createSync(); + final logHandler = LogHandler(logFile: logFile); + + logHandler.save(data: {}); + }); + 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 From 2273f02d8e4c30d809df94a6f7fbed1328d7d0da Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Wed, 5 Jun 2024 00:31:59 -0700 Subject: [PATCH 4/7] bump version patch number --- pkgs/unified_analytics/CHANGELOG.md | 4 ++++ pkgs/unified_analytics/lib/src/constants.dart | 2 +- pkgs/unified_analytics/pubspec.yaml | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkgs/unified_analytics/CHANGELOG.md b/pkgs/unified_analytics/CHANGELOG.md index 3cebcf2d..b28088a0 100644 --- a/pkgs/unified_analytics/CHANGELOG.md +++ b/pkgs/unified_analytics/CHANGELOG.md @@ -1,3 +1,7 @@ +## 6.1.1 + +- Fixed bug where calling `Analytics.send` could result in a `FileSystemException` when unable to write to a log file. + ## 6.1.0 - Added new event constructor `Event.devtoolsEvent` for the single devtools event with a new enum value `DashEvent.devtoolsEvent` diff --git a/pkgs/unified_analytics/lib/src/constants.dart b/pkgs/unified_analytics/lib/src/constants.dart index eb848349..9ac9d1ac 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 = '6.1.0'; +const String kPackageVersion = '6.1.1'; /// 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 ed96d605..11c778b3 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: 6.1.0 +version: 6.1.1 repository: https://github.com/dart-lang/tools/tree/main/pkgs/unified_analytics environment: From 800130953ecdbc11ccd874671837d7f504a3956b Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Wed, 5 Jun 2024 00:36:08 -0700 Subject: [PATCH 5/7] format --- .../test/log_handler_test.dart | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index 92d3f4e1..7f082890 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -192,19 +192,26 @@ void main() { expect(logFile.readAsLinesSync()[0].trim(), isNot('{{')); }); - test('Catches and discards any FileSystemException raised from attempting to write to the log file', () async { - final logFilePath = 'log.txt'; - final fs = MemoryFileSystem.test(opHandle: (context, operation) { - if (context == logFilePath && operation == FileSystemOp.write) { - throw FileSystemException('writeFrom failed', logFilePath, OSError('No space left on device', 28),); - } - }); - final logFile = fs.file(logFilePath); - logFile.createSync(); - final logHandler = LogHandler(logFile: logFile); - - logHandler.save(data: {}); - }); + test( + 'Catches and discards any FileSystemException raised from attempting to write to the log file', + () async { + final logFilePath = 'log.txt'; + final fs = MemoryFileSystem.test(opHandle: (context, operation) { + if (context == logFilePath && operation == FileSystemOp.write) { + throw FileSystemException( + 'writeFrom failed', + logFilePath, + OSError('No space left on device', 28), + ); + } + }); + final logFile = fs.file(logFilePath); + logFile.createSync(); + final logHandler = LogHandler(logFile: logFile); + + logHandler.save(data: {}); + }, + ); test('Catching cast errors for each log record silently', () async { // Write a json array to the log file which will cause From 820b0163826b7dff2455dfaf4ec1c248b5b75173 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Wed, 5 Jun 2024 00:48:48 -0700 Subject: [PATCH 6/7] format --- .../test/log_handler_test.dart | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index 7f082890..0e534fed 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -193,25 +193,24 @@ void main() { }); test( - 'Catches and discards any FileSystemException raised from attempting to write to the log file', - () async { - final logFilePath = 'log.txt'; - final fs = MemoryFileSystem.test(opHandle: (context, operation) { - if (context == logFilePath && operation == FileSystemOp.write) { - throw FileSystemException( - 'writeFrom failed', - logFilePath, - OSError('No space left on device', 28), - ); - } - }); - final logFile = fs.file(logFilePath); - logFile.createSync(); - final logHandler = LogHandler(logFile: logFile); - - logHandler.save(data: {}); - }, - ); + 'Catches and discards any FileSystemException raised from attempting ' + 'to write to the log file', () async { + final logFilePath = 'log.txt'; + final fs = MemoryFileSystem.test(opHandle: (context, operation) { + if (context == logFilePath && operation == FileSystemOp.write) { + throw FileSystemException( + 'writeFrom failed', + logFilePath, + OSError('No space left on device', 28), + ); + } + }); + final logFile = fs.file(logFilePath); + logFile.createSync(); + final logHandler = LogHandler(logFile: logFile); + + logHandler.save(data: {}); + }); test('Catching cast errors for each log record silently', () async { // Write a json array to the log file which will cause From 17a5b9ed4153ff5d90e3e0e370b4d1051397a03a Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Wed, 5 Jun 2024 00:51:11 -0700 Subject: [PATCH 7/7] format --- pkgs/unified_analytics/test/log_handler_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/unified_analytics/test/log_handler_test.dart b/pkgs/unified_analytics/test/log_handler_test.dart index 0e534fed..3cacf472 100644 --- a/pkgs/unified_analytics/test/log_handler_test.dart +++ b/pkgs/unified_analytics/test/log_handler_test.dart @@ -201,7 +201,7 @@ void main() { throw FileSystemException( 'writeFrom failed', logFilePath, - OSError('No space left on device', 28), + const OSError('No space left on device', 28), ); } });