diff --git a/lib/services/logger/file_log_writer.dart b/lib/services/logger/file_log_writer.dart new file mode 100644 index 00000000..92f0e449 --- /dev/null +++ b/lib/services/logger/file_log_writer.dart @@ -0,0 +1,29 @@ +import 'dart:io'; + +/// Best-effort file output for errors that have already been logged to console. +/// +/// File errors must not escape into the same zone that reports app errors. +class FileLogWriter { + final Future _file; + + FileLogWriter(Future Function() getFile) : _file = _resolve(getFile); + + static Future _resolve(Future Function() getFile) async { + try { + return await getFile(); + } catch (_) { + return null; + } + } + + Future write(String message) async { + final file = await _file; + if (file == null) return; + + try { + await file.writeAsString(message, mode: FileMode.writeOnlyAppend); + } catch (_) { + // The console remains available if permissions change after startup. + } + } +} diff --git a/lib/services/logger/logger.dart b/lib/services/logger/logger.dart index 1f15bf92..8f8c99b1 100644 --- a/lib/services/logger/logger.dart +++ b/lib/services/logger/logger.dart @@ -11,6 +11,8 @@ import 'package:path_provider/path_provider.dart'; import 'package:spotube/utils/platform.dart'; import 'package:logging/logging.dart' as logging; +import 'file_log_writer.dart'; + final _loggingToLoggerLevel = { logging.Level.ALL: Level.all, logging.Level.FINEST: Level.trace, @@ -26,7 +28,7 @@ final _loggingToLoggerLevel = { class AppLogger { static late final Logger log; - static late final File logFile; + static FileLogWriter? _fileLogWriter; static initialize(bool verbose) { log = Logger( @@ -39,17 +41,15 @@ class AppLogger { logging.hierarchicalLoggingEnabled = true; logging.Logger('YoutubeExplode.StreamsClient') ..level = logging.Level.SEVERE - ..onRecord.listen( - (record) { - log.log( - _loggingToLoggerLevel[record.level] ?? Level.info, - record.message, - error: record.error, - stackTrace: record.stackTrace, - time: record.time, - ); - }, - ); + ..onRecord.listen((record) { + log.log( + _loggingToLoggerLevel[record.level] ?? Level.info, + record.message, + error: record.error, + stackTrace: record.stackTrace, + time: record.time, + ); + }); } static R? runZoned(R Function() body) { @@ -70,17 +70,14 @@ class AppLogger { Isolate.current.addErrorListener( RawReceivePort((pair) async { final isolateError = pair as List; - reportError( - isolateError.first.toString(), - isolateError.last, - ); + reportError(isolateError.first.toString(), isolateError.last); }).sendPort, ); } _initInternalPackageLoggers(); - getLogsPath().then((value) => logFile = value); + _fileLogWriter = FileLogWriter(getLogsPath); return body(); }, @@ -91,7 +88,11 @@ class AppLogger { } static Future getLogsPath() async { - String dir = (await getApplicationDocumentsDirectory()).path; + // Documents can be protected by Windows Controlled Folder Access. + String dir = (kIsWindows + ? await getApplicationSupportDirectory() + : await getApplicationDocumentsDirectory()) + .path; if (kIsAndroid) { dir = (await getExternalStorageDirectory())?.path ?? ""; } @@ -119,11 +120,10 @@ class AppLogger { log.e(message, error: error, stackTrace: stackTrace); if (kReleaseMode) { - await logFile.writeAsString( + await _fileLogWriter?.write( "[${DateTime.now()}]---------------------\n" "$error\n$stackTrace\n" "----------------------------------------\n", - mode: FileMode.writeOnlyAppend, ); } } diff --git a/pubspec.lock b/pubspec.lock index 81372cdd..1dbac799 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -1767,7 +1767,7 @@ packages: source: hosted version: "2.2.1" path_provider_platform_interface: - dependency: transitive + dependency: "direct dev" description: name: path_provider_platform_interface sha256: "88f5779f72ba699763fa3a3b06aa4bf6de76c8e5de842cf6f29e2e06476c2334" diff --git a/pubspec.yaml b/pubspec.yaml index dd91f204..2bca12dd 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -164,6 +164,7 @@ dev_dependencies: sdk: flutter integration_test: sdk: flutter + path_provider_platform_interface: ^2.1.2 json_serializable: ^6.6.2 freezed: ^2.5.2 process_run: ^0.14.2 diff --git a/test/services/logger/file_log_writer_test.dart b/test/services/logger/file_log_writer_test.dart new file mode 100644 index 00000000..6bfea283 --- /dev/null +++ b/test/services/logger/file_log_writer_test.dart @@ -0,0 +1,87 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotube/services/logger/file_log_writer.dart'; + +void main() { + late Directory directory; + + setUp(() async { + directory = await Directory.systemTemp.createTemp('spotube-log-test-'); + }); + + tearDown(() async { + await directory.delete(recursive: true); + }); + + test('appends errors without replacing earlier entries', () async { + final file = File('${directory.path}/errors.log'); + final writer = FileLogWriter(() async => file); + + await writer.write('first\n'); + await writer.write('second\n'); + + expect(await file.readAsString(), 'first\nsecond\n'); + }); + + test('waits for file initialization before writing', () async { + final file = File('${directory.path}/errors.log'); + final ready = Completer(); + final writer = FileLogWriter(() => ready.future); + final write = writer.write('startup error\n'); + + expect(await file.exists(), isFalse); + ready.complete(file); + await write; + + expect(await file.readAsString(), 'startup error\n'); + }); + + test('contains synchronous initialization errors', () async { + final writer = FileLogWriter(() { + throw const FileSystemException('Permission denied'); + }); + + await expectLater(writer.write('error\n'), completes); + }); + + test( + 'contains asynchronous initialization errors before any report', + () async { + final writer = FileLogWriter(() async { + throw const FileSystemException('Permission denied'); + }); + + // Initialization must be safe even when no error report is being written. + await Future.delayed(Duration.zero); + await expectLater(writer.write('error\n'), completes); + }, + ); + + test( + 'contains initialization failures with a pending error report', + () async { + final ready = Completer(); + final writer = FileLogWriter(() => ready.future); + final write = writer.write('startup error\n'); + + ready.completeError(const FileSystemException('Permission denied')); + + await expectLater(write, completes); + }, + ); + + test('contains write errors after successful initialization', () async { + final file = File('${directory.path}/errors.log'); + final writer = FileLogWriter(() async => file); + await writer.write('first\n'); + + // A directory at the log path fails writes on every supported desktop OS. + await file.delete(); + await Directory(file.path).create(); + + await expectLater(writer.write('second\n'), completes); + await expectLater(writer.write('third\n'), completes); + }); +} diff --git a/test/services/logger/logger_test.dart b/test/services/logger/logger_test.dart new file mode 100644 index 00000000..c7296df0 --- /dev/null +++ b/test/services/logger/logger_test.dart @@ -0,0 +1,49 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:path/path.dart'; +import 'package:path_provider_platform_interface/path_provider_platform_interface.dart'; +import 'package:spotube/services/logger/logger.dart'; + +class _ProtectedDocumentsPathProvider extends PathProviderPlatform { + final String supportPath; + int documentsRequests = 0; + + _ProtectedDocumentsPathProvider(this.supportPath); + + @override + Future getApplicationSupportPath() async => supportPath; + + @override + Future getApplicationDocumentsPath() async { + documentsRequests++; + throw const FileSystemException('Documents access denied'); + } +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + test( + 'Windows logging does not require access to Documents', + () async { + final directory = await Directory.systemTemp.createTemp( + 'spotube-path-test-', + ); + final original = PathProviderPlatform.instance; + final provider = _ProtectedDocumentsPathProvider(directory.path); + PathProviderPlatform.instance = provider; + addTearDown(() async { + PathProviderPlatform.instance = original; + await directory.delete(recursive: true); + }); + + final file = await AppLogger.getLogsPath(); + + expect(file.path, join(directory.path, '.spotube_logs')); + expect(await file.exists(), isTrue); + expect(provider.documentsRequests, 0); + }, + skip: !Platform.isWindows, + ); +}