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.
2026-07-27 17:44:52 +02:00
parent b202c62641
commit c45113fefe
5 changed files with 229 additions and 28 deletions
@@ -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<int> failActiveDownloadsForStorageFull(String errorMessage) {
+  Future<List<String>> 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 <String>[];

      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;
    });
  }

@@ -371,9 +371,16 @@ Future<AppDatabaseBootstrap> 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 {
@@ -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<bool> _downloadArtwork(String globalKey, MediaItem metadata, MediaServerClient client) async {
+  Future<bool> _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<DownloadSubtitleSpec> 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,
      ),
@@ -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 = <DownloadProgress>[];
+      final progressSubscription = manager.progressStream.listen(events.add);
+      addTearDown(progressSubscription.cancel);
+      addTearDown(manager.dispose);
+
+      await manager.repairPendingSupplementaryDownloads();
+      await Future<void>.delayed(Duration.zero);
+
+      expect(
+        events,
+        isNot(contains(predicate<DownloadProgress>((event) => event.status == DownloadStatus.downloading))),
+      );
+      expect(
+        events,
+        contains(
+          predicate<DownloadProgress>(
+            (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 = <DownloadProgress>[];
+      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<void>.delayed(Duration.zero);
+
+      final downloadingFiles = events
+          .where((event) => event.status == DownloadStatus.downloading)
+          .map((event) => event.currentFile);
+      expect(downloadingFiles, containsAll(<String?>['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 = <DownloadProgress>[];
+      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<void>.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<void> _seedCompletingDownload(_SupplementaryFixture fixture, {required bool downloadSubtitles}) async {
+Future<void> _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<void> _seedCompletingDownload(_SupplementaryFixture fixture, {required bo
  await fixture.db.addToQueue(
    mediaGlobalKey: fixture.metadata.globalKey,
    downloadSubtitles: downloadSubtitles,
-    downloadArtwork: false,
+    downloadArtwork: downloadArtwork,
  );
 }

-Future<void> _seedCompletedPendingDownload(_SupplementaryFixture fixture, {required bool downloadSubtitles}) async {
+Future<void> _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<void> _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<DownloadArtworkSpec> resolveDownloadArtwork(MediaItem item) => const [];
+  List<DownloadArtworkSpec> resolveDownloadArtwork(MediaItem item) {
+    return buildArtworkSpecs(item, (path) => 'https://example.test$path');
+  }
+
+  @override
+  Future<PlaybackExtras> 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);
@@ -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<int> runUpdate(QueryExecutor executor, String statement, List<Object?> args) {
+    final updateFailure = this.updateFailure;
+    if (updateFailure != null) throw updateFailure;
+    return executor.runUpdate(statement, args);
+  }
+
  @override
  Future<void> 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_');