From c45113fefe6d1bb52e381e36a5d65a252800d381 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:44:52 +0200 Subject: [PATCH] fix(downloads): report accurate status through repair and storage exhaustion Supplementary repair runs over downloads whose video is already complete, so it no longer emits a downloading transition, and artwork updates carry the row's real status instead of asserting downloading. Previously a reconnect left completed downloads stuck at "downloading 0%" until the next DB read. Storage exhaustion fails every active row in one transaction, so failActiveDownloadsForStorageFull now returns the affected keys and each one gets a failed event; only the triggering key was announced before. The post-recovery database open also closes its handle before rethrowing. A failing storage-full write abandoned a drift background isolate and its SQLite handles on every retry. --- lib/database/download_operations.dart | 6 +- lib/main.dart | 13 +- lib/services/download_manager_service.dart | 50 +++++-- .../download_manager_service_test.dart | 126 +++++++++++++++++- test/startup_bootstrap_test.dart | 62 ++++++++- 5 files changed, 229 insertions(+), 28 deletions(-) diff --git a/lib/database/download_operations.dart b/lib/database/download_operations.dart index 116f2159..2df91c5f 100644 --- a/lib/database/download_operations.dart +++ b/lib/database/download_operations.dart @@ -461,12 +461,12 @@ extension DownloadDatabaseOperations on AppDatabase { /// /// Clearing native task ids and queue rows prevents startup recovery from /// immediately re-enqueueing work after partial files have been discarded. - Future failActiveDownloadsForStorageFull(String errorMessage) { + Future> failActiveDownloadsForStorageFull(String errorMessage) { return transaction(() async { final active = await (select( downloadedMedia, )..where((row) => row.status.isIn([DownloadStatus.queued.index, DownloadStatus.downloading.index]))).get(); - if (active.isEmpty) return 0; + if (active.isEmpty) return const []; final globalKeys = active.map((item) => item.globalKey).toList(growable: false); await (update(downloadedMedia)..where((row) => row.globalKey.isIn(globalKeys))).write( @@ -477,7 +477,7 @@ extension DownloadDatabaseOperations on AppDatabase { ), ); await (delete(downloadQueue)..where((row) => row.mediaGlobalKey.isIn(globalKeys))).go(); - return globalKeys.length; + return globalKeys; }); } diff --git a/lib/main.dart b/lib/main.dart index 73c2c501..a961c4bf 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -371,9 +371,16 @@ Future openAppDatabaseWithDownloadRecovery({ await recoverNativeDownloads(); final bootstrap = await openDatabase(); - final failedCount = await bootstrap.database.failActiveDownloadsForStorageFull(storageFullMessage); - appLogger.w('Recovered startup after storage exhaustion; stopped $failedCount active download(s)'); - return bootstrap; + try { + final failedKeys = await bootstrap.database.failActiveDownloadsForStorageFull(storageFullMessage); + appLogger.w('Recovered startup after storage exhaustion; stopped ${failedKeys.length} active download(s)'); + return bootstrap; + } catch (_) { + // The caller only takes ownership once this helper returns, so the freshly + // opened background isolate has to be released here. + await bootstrap.database.close(); + rethrow; + } } Future<_StartupDependencies> _initializeStartup(SettingsService settings) async { diff --git a/lib/services/download_manager_service.dart b/lib/services/download_manager_service.dart index c7770c0a..bd9312fe 100644 --- a/lib/services/download_manager_service.dart +++ b/lib/services/download_manager_service.dart @@ -1332,6 +1332,7 @@ class DownloadManagerService { globalKey, metadata, client, + isRepair: true, downloadArtwork: queueItem.downloadArtwork, downloadSubtitles: queueItem.downloadSubtitles, record: record, @@ -2074,9 +2075,13 @@ class DownloadManagerService { } final errorMessage = t.downloads.storageFull; - final failedCount = await _database.failActiveDownloadsForStorageFull(errorMessage); - _emitProgress(globalKey, DownloadStatus.failed, 0, errorMessage: errorMessage); - appLogger.e('Device storage exhausted; stopped $failedCount active download(s)'); + final failedKeys = await _database.failActiveDownloadsForStorageFull(errorMessage); + for (final key in failedKeys) { + _cancelDownloadTimers(key); + _pendingDownloadContext.remove(key); + _emitProgress(key, DownloadStatus.failed, 0, errorMessage: errorMessage); + } + appLogger.e('Device storage exhausted; stopped ${failedKeys.length} active download(s)'); } bool _isRetryablePrepareFailure(Object error) { @@ -2344,6 +2349,7 @@ class DownloadManagerService { globalKey, metadata, client, + isRepair: false, downloadArtwork: downloadArtwork, downloadSubtitles: downloadSubtitles, record: existingCheck, @@ -2447,6 +2453,7 @@ class DownloadManagerService { String globalKey, MediaItem metadata, MediaServerClient client, { + required bool isRepair, required bool downloadArtwork, required bool downloadSubtitles, required DownloadedMediaItem? record, @@ -2455,7 +2462,7 @@ class DownloadManagerService { }) async { var artworkSettled = !downloadArtwork; if (downloadArtwork) { - final itemArtworkSettled = await _downloadArtwork(globalKey, metadata, client); + final itemArtworkSettled = await _downloadArtwork(globalKey, metadata, client, isRepair: isRepair); final chapterArtworkSettled = metadata.serverId == null ? false : await _downloadChapterThumbnails(ServerId(metadata.serverId!), metadata.id, client); @@ -2479,7 +2486,14 @@ class DownloadManagerService { } } if (subtitles != null) { - subtitlesSettled = await _downloadSubtitles(globalKey, metadata, subtitles, client, showYear: showYear); + subtitlesSettled = await _downloadSubtitles( + globalKey, + metadata, + subtitles, + client, + isRepair: isRepair, + showYear: showYear, + ); } } catch (e, st) { appLogger.w('Could not resolve subtitles for $globalKey', error: e, stackTrace: st); @@ -2489,11 +2503,18 @@ class DownloadManagerService { return (artwork: artworkSettled, subtitles: subtitlesSettled); } - Future _downloadArtwork(String globalKey, MediaItem metadata, MediaServerClient client) async { + Future _downloadArtwork( + String globalKey, + MediaItem metadata, + MediaServerClient client, { + required bool isRepair, + }) async { if (metadata.serverId == null) return false; try { - _emitProgress(globalKey, DownloadStatus.downloading, 0, currentFile: 'artwork'); + if (!isRepair) { + _emitProgress(globalKey, DownloadStatus.downloading, 0, currentFile: 'artwork'); + } final serverId = metadata.serverId!; final specs = client.resolveDownloadArtwork(metadata); @@ -2502,7 +2523,7 @@ class DownloadManagerService { final storedThumbPath = metadata.thumbPath == null ? null : artworkStorageKey(metadata.thumbPath!); await _database.updateArtworkPaths(globalKey: globalKey, thumbPath: storedThumbPath); - _emitProgressWithArtwork(globalKey, thumbPath: storedThumbPath); + _emitProgressWithArtwork(globalKey, isRepair: isRepair, thumbPath: storedThumbPath); appLogger.d(artworkSettled ? 'Artwork downloaded for $globalKey' : 'Artwork remains incomplete for $globalKey'); return artworkSettled; } catch (e, st) { @@ -2565,9 +2586,12 @@ class DownloadManagerService { MediaItem metadata, List subtitles, MediaServerClient client, { + required bool isRepair, int? showYear, }) async { - _emitProgress(globalKey, DownloadStatus.downloading, 0, currentFile: 'subtitles'); + if (!isRepair) { + _emitProgress(globalKey, DownloadStatus.downloading, 0, currentFile: 'subtitles'); + } var allSettled = true; for (final subtitle in subtitles) { @@ -2656,15 +2680,13 @@ class DownloadManagerService { } /// Emit progress update with artwork paths so DownloadProvider can sync - void _emitProgressWithArtwork(String globalKey, {String? thumbPath}) { + void _emitProgressWithArtwork(String globalKey, {required bool isRepair, String? thumbPath}) { if (_disposed) return; - // Emit a progress update containing artwork path - // The status is preserved as downloading since artwork is just one step _progressController.add( DownloadProgress( globalKey: globalKey, - status: DownloadStatus.downloading, - progress: 0, + status: isRepair ? DownloadStatus.completed : DownloadStatus.downloading, + progress: isRepair ? 100 : 0, currentFile: 'artwork', thumbPath: thumbPath, ), diff --git a/test/services/download_manager_service_test.dart b/test/services/download_manager_service_test.dart index 2b592554..cc089c0d 100644 --- a/test/services/download_manager_service_test.dart +++ b/test/services/download_manager_service_test.dart @@ -19,6 +19,7 @@ import 'package:plezy/media/download_resolution.dart'; import 'package:plezy/media/media_backend.dart'; import 'package:plezy/media/media_item.dart'; import 'package:plezy/media/media_kind.dart'; +import 'package:plezy/media/media_source_info.dart'; import 'package:plezy/media/media_server_client.dart'; import 'package:plezy/models/download_models.dart'; import 'package:plezy/services/download_artwork_helpers.dart'; @@ -1188,6 +1189,84 @@ void main() { }); }); + group('deferred supplementary repair', () { + test('completed repair persists artwork without announcing downloading', () async { + final fixture = await _createSupplementaryFixture(thumbPath: '/repair-thumb'); + final client = _SupplementaryClient( + metadata: fixture.metadata, + resolution: () => const DownloadResolution(videoUrl: 'https://example.test/video'), + ); + await _seedCompletedPendingDownload(fixture, downloadSubtitles: false, downloadArtwork: true); + final manager = DownloadManagerService( + database: fixture.db, + storageService: fixture.storage, + clientResolver: (serverId, {clientScopeId}) => client, + http: MediaServerHttpClient(client: FakeHttpClient(200, utf8.encode('image bytes'))), + downloadsSupportedOverride: false, + ); + final events = []; + final progressSubscription = manager.progressStream.listen(events.add); + addTearDown(progressSubscription.cancel); + addTearDown(manager.dispose); + + await manager.repairPendingSupplementaryDownloads(); + await Future.delayed(Duration.zero); + + expect( + events, + isNot(contains(predicate((event) => event.status == DownloadStatus.downloading))), + ); + expect( + events, + contains( + predicate( + (event) => + event.status == DownloadStatus.completed && event.thumbPath == artworkStorageKey('/repair-thumb'), + ), + ), + ); + expect( + (await fixture.db.getDownloadedMedia(fixture.metadata.globalKey))?.thumbPath, + artworkStorageKey('/repair-thumb'), + ); + expect(await fixture.db.getPendingSupplementaryQueueItems(), isEmpty); + }); + + test('normal completion still announces artwork and subtitle steps', () async { + final fixture = await _createSupplementaryFixture(thumbPath: '/normal-thumb'); + final client = _SupplementaryClient( + metadata: fixture.metadata, + resolution: () => const DownloadResolution( + videoUrl: 'https://example.test/video', + externalSubtitles: [DownloadSubtitleSpec(id: 1, url: 'https://example.test/subtitle/1', codec: 'srt')], + ), + ); + await _seedCompletingDownload(fixture, downloadSubtitles: true, downloadArtwork: true); + final manager = DownloadManagerService( + database: fixture.db, + storageService: fixture.storage, + clientResolver: (serverId, {clientScopeId}) => client, + http: MediaServerHttpClient(client: FakeHttpClient(200, utf8.encode('supplementary bytes'))), + downloadsSupportedOverride: false, + ); + final events = []; + final progressSubscription = manager.progressStream.listen(events.add); + addTearDown(progressSubscription.cancel); + addTearDown(manager.dispose); + + await manager.debugHandleTaskStatus( + TaskStatusUpdate(_downloadTask('current-task', fixture.metadata.globalKey), TaskStatus.complete), + ); + await Future.delayed(Duration.zero); + + final downloadingFiles = events + .where((event) => event.status == DownloadStatus.downloading) + .map((event) => event.currentFile); + expect(downloadingFiles, containsAll(['artwork', 'subtitles'])); + expect(await fixture.db.getPendingSupplementaryQueueItems(), isEmpty); + }); + }); + group('deletion cleanup', () { test('missing video still removes partial and subtitle sidecars', () async { resetSharedPreferencesForTest(); @@ -1323,6 +1402,9 @@ void main() { downloadsSupportedOverride: false, ); addTearDown(manager.dispose); + final events = []; + final sub = manager.progressStream.listen(events.add); + addTearDown(sub.cancel); await manager.debugHandleTaskStatus( TaskStatusUpdate( @@ -1331,6 +1413,14 @@ void main() { TaskFileSystemException('write failed: ENOSPC (No space left on device)'), ), ); + await Future.delayed(Duration.zero); + expect( + events + .where((event) => event.status == DownloadStatus.failed && event.errorMessage == t.downloads.storageFull) + .map((event) => event.globalKey) + .toSet(), + {currentKey, queuedKey}, + ); final current = await db.getDownloadedMedia(currentKey); final queued = await db.getDownloadedMedia(queuedKey); @@ -1847,7 +1937,7 @@ void main() { }); } -Future<_SupplementaryFixture> _createSupplementaryFixture() async { +Future<_SupplementaryFixture> _createSupplementaryFixture({String? thumbPath}) async { resetSharedPreferencesForTest(); SettingsService.resetForTesting(); DownloadStorageService.resetForTesting(); @@ -1869,13 +1959,14 @@ Future<_SupplementaryFixture> _createSupplementaryFixture() async { kind: MediaKind.movie, serverId: ServerId('srv'), title: 'Movie', + thumbPath: thumbPath, ), ); fixture.initializeCaches(); await PlexApiCache.instance.put(ServerId('srv'), '/library/metadata/item-1', { 'MediaContainer': { 'Metadata': [ - {'ratingKey': 'item-1', 'type': 'movie', 'title': 'Movie'}, + {'ratingKey': 'item-1', 'type': 'movie', 'title': 'Movie', 'thumb': ?thumbPath}, ], }, }); @@ -1883,7 +1974,11 @@ Future<_SupplementaryFixture> _createSupplementaryFixture() async { return fixture; } -Future _seedCompletingDownload(_SupplementaryFixture fixture, {required bool downloadSubtitles}) async { +Future _seedCompletingDownload( + _SupplementaryFixture fixture, { + required bool downloadSubtitles, + bool downloadArtwork = false, +}) async { await fixture.db.insertDownload( serverId: ServerId('srv'), ratingKey: fixture.metadata.id, @@ -1895,11 +1990,15 @@ Future _seedCompletingDownload(_SupplementaryFixture fixture, {required bo await fixture.db.addToQueue( mediaGlobalKey: fixture.metadata.globalKey, downloadSubtitles: downloadSubtitles, - downloadArtwork: false, + downloadArtwork: downloadArtwork, ); } -Future _seedCompletedPendingDownload(_SupplementaryFixture fixture, {required bool downloadSubtitles}) async { +Future _seedCompletedPendingDownload( + _SupplementaryFixture fixture, { + required bool downloadSubtitles, + bool downloadArtwork = false, +}) async { await fixture.db.insertDownload( serverId: ServerId('srv'), ratingKey: fixture.metadata.id, @@ -1911,7 +2010,7 @@ Future _seedCompletedPendingDownload(_SupplementaryFixture fixture, {requi await fixture.db.addToQueue( mediaGlobalKey: fixture.metadata.globalKey, downloadSubtitles: downloadSubtitles, - downloadArtwork: false, + downloadArtwork: downloadArtwork, ); } @@ -1979,7 +2078,20 @@ class _SupplementaryClient implements MediaServerClient { } @override - List resolveDownloadArtwork(MediaItem item) => const []; + List resolveDownloadArtwork(MediaItem item) { + return buildArtworkSpecs(item, (path) => 'https://example.test$path'); + } + + @override + Future fetchPlaybackExtras( + String itemId, { + String? introPattern, + String? creditsPattern, + bool forceChapterFallback = false, + bool forceRefresh = false, + }) async { + return PlaybackExtras(chapters: const [], markers: const []); + } @override dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); diff --git a/test/startup_bootstrap_test.dart b/test/startup_bootstrap_test.dart index a6f24d63..a4ef9951 100644 --- a/test/startup_bootstrap_test.dart +++ b/test/startup_bootstrap_test.dart @@ -17,9 +17,10 @@ import 'test_helpers/download_fixtures.dart'; import 'test_helpers/prefs.dart'; final class _OpenTrackingInterceptor extends QueryInterceptor { - _OpenTrackingInterceptor({this.failure}); + _OpenTrackingInterceptor({this.failure, this.updateFailure}); final Object? failure; + final Object? updateFailure; var ensureOpenCalls = 0; var ensureOpenCompleted = false; var closed = false; @@ -35,6 +36,13 @@ final class _OpenTrackingInterceptor extends QueryInterceptor { return result; } + @override + Future runUpdate(QueryExecutor executor, String statement, List args) { + final updateFailure = this.updateFailure; + if (updateFailure != null) throw updateFailure; + return executor.runUpdate(statement, args); + } + @override Future close(QueryExecutor inner) async { await inner.close(); @@ -219,6 +227,58 @@ void main() { await tempDir.delete(recursive: true); } }); + + test('post-recovery download failure closes the reopened database before rethrowing', () async { + resetSharedPreferencesForTest(); + final tempDir = await Directory.systemTemp.createTemp('plezy_startup_recovery_update_failure_'); + final file = File('${tempDir.path}/plezy_downloads.db'); + final prefs = await BaseSharedPreferencesService.sharedCache(); + final failedOpen = _OpenTrackingInterceptor( + failure: const FileSystemException('write failed: No space left on device'), + ); + final updateError = StateError('injected post-recovery update failure'); + final reopened = _OpenTrackingInterceptor(updateFailure: updateError); + AppDatabase? seeded; + + try { + seeded = AppDatabase.forTesting(NativeDatabase(file)); + await seeded.insertDownload( + serverId: ServerId('srv'), + ratingKey: 'active', + globalKey: 'srv:active', + type: 'movie', + status: DownloadStatus.downloading.index, + ); + await seeded.close(); + seeded = null; + + var openAttempts = 0; + final open = openAppDatabaseWithDownloadRecovery( + openDatabase: () { + return AppDatabase.open( + isTvos: false, + databaseFile: file, + preferences: prefs, + executorFactory: (databaseFile) { + openAttempts++; + final interceptor = openAttempts == 1 ? failedOpen : reopened; + return NativeDatabase(databaseFile).interceptWith(interceptor); + }, + ); + }, + recoverNativeDownloads: () async {}, + storageFullMessage: 'Storage full', + ); + + await expectLater(open, throwsA(same(updateError))); + expect(openAttempts, 2); + expect(reopened.closed, isTrue); + } finally { + await seeded?.close(); + await tempDir.delete(recursive: true); + } + }); + test('non-storage lazy database-open errors bypass download recovery', () async { resetSharedPreferencesForTest(); final tempDir = await Directory.systemTemp.createTemp('plezy_startup_open_error_');