Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[unified_analytics 5.8.8] Prevent FileSystemExceptions from happening when logging outgoing events #280

Merged
merged 2 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkgs/unified_analytics/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
9 changes: 7 additions & 2 deletions pkgs/unified_analytics/lib/src/constants.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down Expand Up @@ -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;
Expand Down
37 changes: 25 additions & 12 deletions pkgs/unified_analytics/lib/src/log_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkgs/unified_analytics/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
113 changes: 113 additions & 0 deletions pkgs/unified_analytics/test/log_handler_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}