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] Suppress any FileSystemException thrown during Analytics.send #274

Merged
merged 7 commits into from
Jun 5, 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
4 changes: 4 additions & 0 deletions pkgs/unified_analytics/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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`
Expand Down
2 changes: 1 addition & 1 deletion pkgs/unified_analytics/lib/src/constants.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
23 changes: 14 additions & 9 deletions pkgs/unified_analytics/lib/src/log_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +278 to +291
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered adding logic that would retry once after purging some number of records, but I'm not sure if the complexity of this would be worth it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's drop it. The log file is way less important than:

  1. ensuring the client tools don't crash
  2. sending analytics

}
}
}
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: 6.1.0
version: 6.1.1
repository: https://github.com/dart-lang/tools/tree/main/pkgs/unified_analytics

environment:
Expand Down
21 changes: 21 additions & 0 deletions pkgs/unified_analytics/test/log_handler_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -191,6 +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,
const 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
Expand Down