[unified_analytics 5.8.8] Prevent FileSystemExceptions from happeni…

…ng when logging outgoing events (#280)

pkgs/unified_analytics/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 5.8.8+2
+
+- Avoid opening large telemetry log files to prevent out of memory errors.
+- Fixed bug where calling `Analytics.send` could result in a `FileSystemException` when unable to write to a log file.
+
 ## 5.8.8+1
 
 - Edit to error handler to not use default `Analytic.send` method and use new `Analytics._sendError` method that doesn't create a session id

pkgs/unified_analytics/lib/src/constants.dart

@@ -26,7 +26,7 @@ const String kConfigString = '''
 # All other lines are configuration lines. They have
 # the form "name=value". If multiple lines contain
 # the same configuration name with different values,
-# the parser will default to a conservative value. 
+# the parser will default to a conservative value.
 
 # DISABLING TELEMETRY REPORTING
 #
@@ -78,11 +78,16 @@ const String kGoogleAnalyticsMeasurementId = 'G-04BXPVBCWJ';
 /// How many data records to store in the log file.
 const int kLogFileLength = 2500;
 
+/// The maximum allowed size of the telemetry log file.
+///
+/// 25 MiB.
+const int kMaxLogFileSize = 25 * (1 << 20);
+
 /// Filename for the log file to persist sent events on user's machine.
 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.8+1';
+const String kPackageVersion = '5.8.8+2';
 
 /// The minimum length for a session.
 const int kSessionDurationMinutes = 30;

pkgs/unified_analytics/lib/src/log_handler.dart

@@ -282,18 +282,31 @@ class LogHandler {
   /// This will keep the max number of records limited to equal to
   /// or less than [kLogFileLength] records.
   void save({required Map<String, Object?> data}) {
-    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 {
+      final stat = logFile.statSync();
+      List<String> records;
+      if (stat.size > kMaxLogFileSize) {
+        logFile.deleteSync();
+        logFile.createSync();
+        records = [];
+      } else {
+        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'));
+      }
+    } on FileSystemException {
+      // Logging isn't important enough to warrant raising a
+      // FileSystemException that will surprise consumers of this package.
     }
   }
 }

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.8+1
+version: 5.8.8+2
 repository: https://github.com/dart-lang/tools/tree/main/pkgs/unified_analytics
 
 environment:

pkgs/unified_analytics/test/log_handler_test.dart

@@ -2,13 +2,17 @@
 // 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:convert';
+
 import 'package:file/file.dart';
 import 'package:file/memory.dart';
 import 'package:path/path.dart' as p;
+import 'package:test/fake.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/log_handler.dart';
 import 'package:unified_analytics/src/utils.dart';
 import 'package:unified_analytics/unified_analytics.dart';
 
@@ -200,6 +204,67 @@ 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,
+          const OSError('No space left on device', 28),
+        );
+      }
+    });
+    final logFile = fs.file(logFilePath);
+    logFile.createSync();
+    final logHandler = LogHandler(logFile: logFile);
+
+    logHandler.save(data: {});
+  });
+
+  test('deletes log file larger than kMaxLogFileSize', () async {
+    var deletedLargeLogFile = false;
+    var wroteDataToLogFile = false;
+    const data = <String, Object?>{};
+    final logFile = _FakeFile('log.txt')
+      .._deleteSyncImpl = (() => deletedLargeLogFile = true)
+      .._createSyncImpl = () {}
+      .._statSyncImpl = (() => _FakeFileStat(kMaxLogFileSize + 1))
+      .._writeAsStringSync = (contents, {mode = FileMode.append}) {
+        expect(contents.trim(), data.toString());
+        expect(mode, FileMode.writeOnlyAppend);
+        wroteDataToLogFile = true;
+      };
+    final logHandler = LogHandler(logFile: logFile);
+
+    logHandler.save(data: data);
+    expect(deletedLargeLogFile, isTrue);
+    expect(wroteDataToLogFile, isTrue);
+  });
+
+  test('does not delete log file if smaller than kMaxLogFileSize', () async {
+    var wroteDataToLogFile = false;
+    const data = <String, Object?>{};
+    final logFile = _FakeFile('log.txt')
+      .._deleteSyncImpl =
+          (() => fail('called logFile.deleteSync() when file was less than '
+              'kMaxLogFileSize'))
+      .._createSyncImpl = () {}
+      .._readAsLinesSyncImpl = (() => ['three', 'previous', 'lines'])
+      .._statSyncImpl = (() => _FakeFileStat(kMaxLogFileSize - 1))
+      .._writeAsStringSync = (contents, {mode = FileMode.append}) {
+        expect(contents.trim(), data.toString());
+        expect(mode, FileMode.writeOnlyAppend);
+        wroteDataToLogFile = true;
+      };
+    final logHandler = LogHandler(logFile: logFile);
+
+    logHandler.save(data: data);
+    expect(wroteDataToLogFile, isTrue);
+  });
+
   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
@@ -280,3 +345,51 @@ void main() {
     expect(newString, testString);
   });
 }
+
+class _FakeFileStat extends Fake implements FileStat {
+  _FakeFileStat(this.size);
+
+  @override
+  final int size;
+}
+
+class _FakeFile extends Fake implements File {
+  _FakeFile(this.path);
+
+  List<String> Function()? _readAsLinesSyncImpl;
+
+  @override
+  List<String> readAsLinesSync({Encoding encoding = utf8}) =>
+      _readAsLinesSyncImpl!();
+
+  @override
+  final String path;
+
+  FileStat Function()? _statSyncImpl;
+
+  @override
+  FileStat statSync() => _statSyncImpl!();
+
+  void Function()? _deleteSyncImpl;
+
+  @override
+  void deleteSync({bool recursive = false}) => _deleteSyncImpl!();
+
+  void Function()? _createSyncImpl;
+
+  @override
+  void createSync({bool recursive = false, bool exclusive = false}) {
+    return _createSyncImpl!();
+  }
+
+  void Function(String contents, {FileMode mode})? _writeAsStringSync;
+
+  @override
+  void writeAsStringSync(
+    String contents, {
+    FileMode mode = FileMode.write,
+    Encoding encoding = utf8,
+    bool flush = false,
+  }) =>
+      _writeAsStringSync!(contents, mode: mode);
+}