From 6eb5c847af4093e10e09238a5cc163c71da3b860 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Tue, 14 Apr 2026 21:54:00 +0200 Subject: [PATCH] fix: download cleanup, retry circuit breaker, SAF resume --- lib/services/download_manager_service.dart | 66 ++++++++++++++++++++-- pubspec.lock | 9 +-- pubspec.yaml | 5 +- 3 files changed, 69 insertions(+), 11 deletions(-) diff --git a/lib/services/download_manager_service.dart b/lib/services/download_manager_service.dart index 07e2042a..835827b4 100644 --- a/lib/services/download_manager_service.dart +++ b/lib/services/download_manager_service.dart @@ -97,6 +97,11 @@ class DownloadManagerService { // Keyed by globalKey; each timer fires a fresh re-enqueue after a delay. final Map _autoRetryTimers = {}; + // Circuit breaker: consecutive instant failures in _processQueue. + // Stops the queue when all items fail with the same error (e.g. DNS). + int _consecutiveQueueFailures = 0; + static const _maxConsecutiveFailures = 3; + /// Public method to check if downloads should be blocked due to cellular-only setting /// Can be used by DownloadProvider to show user-friendly error static Future shouldBlockDownloadOnCellular() async { @@ -411,6 +416,11 @@ class DownloadManagerService { await _initializeFileDownloader(); while (true) { + if (_consecutiveQueueFailures >= _maxConsecutiveFailures) { + appLogger.w('Circuit breaker: $_consecutiveQueueFailures consecutive failures, pausing queue'); + break; + } + final nextItem = await _database.getNextQueueItem(); if (nextItem == null) break; @@ -421,25 +431,43 @@ class DownloadManagerService { appLogger.d('Skipping queued download ${nextItem.mediaGlobalKey}: server offline'); continue; } - await _prepareAndEnqueueDownload(nextItem.mediaGlobalKey, itemClient, nextItem); + final enqueued = await _prepareAndEnqueueDownload(nextItem.mediaGlobalKey, itemClient, nextItem); + if (enqueued) { + _consecutiveQueueFailures = 0; + } else { + _consecutiveQueueFailures++; + } } } finally { _isProcessingQueue = false; } } + /// Cancel any lingering background task and reset progress before re-enqueuing. + Future _cleanupStaleDownload(String globalKey) async { + final existingTaskId = await _database.getBgTaskId(globalKey); + if (existingTaskId != null) { + await FileDownloader().cancelTaskWithId(existingTaskId); + await _database.updateBgTaskId(globalKey, null); + appLogger.d('Cancelled stale bg task $existingTaskId for $globalKey'); + } + await _database.updateDownloadProgress(globalKey, 0, 0, 0); + } + /// Resolve metadata, video URL, and file path, then enqueue a background download task. - Future _prepareAndEnqueueDownload(String globalKey, PlexClient client, DownloadQueueItem queueItem) async { + /// Returns true if successfully enqueued, false if it failed immediately. + Future _prepareAndEnqueueDownload(String globalKey, PlexClient client, DownloadQueueItem queueItem) async { try { // Guard: don't re-enqueue an item that's already completed or was deleted final existing = await _database.getDownloadedMedia(globalKey); if (existing == null || existing.status == DownloadStatus.completed.index) { appLogger.d('Skipping enqueue for $globalKey: already completed or deleted'); await _database.removeFromQueue(globalKey); - return; + return true; } appLogger.i('Preparing download for $globalKey'); + if (existing.bgTaskId != null) await _cleanupStaleDownload(globalKey); await _transitionStatus(globalKey, DownloadStatus.downloading); final parsed = parseGlobalKey(globalKey); @@ -519,6 +547,7 @@ class DownloadManagerService { updates: Updates.statusAndProgress, requiresWiFi: requiresWiFi, retries: _nativeRetries, + allowPause: true, metaData: globalKey, displayName: displayName, ); @@ -549,6 +578,13 @@ class DownloadManagerService { downloadFilePath = await _storageService.getVideoFilePath(serverId, metadata.ratingKey, ext); } + // Clean up partial files from previous attempts to prevent + // background_downloader from creating numbered copies (File (1).mp4) + await Future.wait([ + _deleteFileIfExists(File(downloadFilePath), 'stale video before re-download'), + _deleteFileIfExists(File('$downloadFilePath.part'), 'stale .part before re-download'), + ]); + await File(downloadFilePath).parent.create(recursive: true); final task = DownloadTask( @@ -580,11 +616,13 @@ class DownloadManagerService { if (!success) throw Exception('Failed to enqueue download task'); appLogger.i('Enqueued download task ${task.taskId} for $globalKey'); } + return true; } catch (e) { appLogger.e('Failed to prepare download for $globalKey', error: e); await _transitionStatus(globalKey, DownloadStatus.failed, errorMessage: e.toString()); await _database.removeFromQueue(globalKey); _pendingDownloadContext.remove(globalKey); + return false; } } @@ -698,8 +736,17 @@ class DownloadManagerService { } final retryCount = existing?.retryCount ?? 0; + // DNS/connection errors fail instantly and exhaust native retries in milliseconds, + // creating a retry storm. Treat them as permanent failures. + final isNetworkError = errorMessage.contains('Unable to resolve host') || + errorMessage.contains('No address associated with hostname') || + errorMessage.contains('Network is unreachable') || + errorMessage.contains('Connection refused'); + final client = _getClient(parseGlobalKey(globalKey)?.serverId); - if (retryCount < _maxAppRetries && client != null) { + final hadProgress = (existing?.downloadedBytes ?? 0) > 0; + + if (!isNetworkError && retryCount < _maxAppRetries && client != null) { // App-level auto-retry: schedule a fresh download after a delay. // Each new task gets 5 native retries with Range-based resume. appLogger.w( @@ -714,9 +761,13 @@ class DownloadManagerService { _performAutoRetry(globalKey); }); - // Advance the queue while we wait for the retry timer - _processQueue(client); + // Only advance the queue if the download actually started transferring. + // Instant failures (DNS, connection) would just cause the next item to fail too. + if (hadProgress) _processQueue(client); } else { + if (isNetworkError) { + appLogger.w('Network error for $globalKey, failing permanently (no auto-retry): $errorMessage'); + } await _onDownloadPermanentlyFailed(globalKey, errorMessage); } } @@ -768,6 +819,7 @@ class DownloadManagerService { /// Handle a completed video download — store path, download supplementary content, mark done. Future _onDownloadComplete(String globalKey, Task task) async { + _consecutiveQueueFailures = 0; // Prevent duplicate concurrent completions (e.g. trackTasks replaying events) if (_completingKeys.contains(globalKey)) { appLogger.d('Already processing completion for $globalKey, skipping'); @@ -1216,6 +1268,7 @@ class DownloadManagerService { // Native resume failed or not supported (SAF mode) — re-enqueue from scratch await _database.updateBgTaskId(globalKey, null); + await _database.updateDownloadProgress(globalKey, 0, 0, 0); await _transitionStatus(globalKey, DownloadStatus.queued); await _database.addToQueue(mediaGlobalKey: globalKey); final resolvedClient = _getClient(parseGlobalKey(globalKey)?.serverId) ?? client; @@ -1227,6 +1280,7 @@ class DownloadManagerService { _autoRetryTimers.remove(globalKey)?.cancel(); await _database.clearDownloadError(globalKey); await _database.updateBgTaskId(globalKey, null); + await _database.updateDownloadProgress(globalKey, 0, 0, 0); await _transitionStatus(globalKey, DownloadStatus.queued); await _database.addToQueue(mediaGlobalKey: globalKey); final resolvedClient = _getClient(parseGlobalKey(globalKey)?.serverId) ?? client; diff --git a/pubspec.lock b/pubspec.lock index 717ba842..4aea441c 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -88,10 +88,11 @@ packages: background_downloader: dependency: "direct main" description: - name: background_downloader - sha256: "4cb23d9ad4f5060944f38164e7b90d4bf99b57b2472a3bd4676e59b2db4afd06" - url: "https://pub.dev" - source: hosted + path: "." + ref: "2017dfd" + resolved-ref: "2017dfdb3c423d92eb78f48413c1f7dc7ecaf783" + url: "https://github.com/edde746/background_downloader" + source: git version: "9.5.4" boolean_selector: dependency: transitive diff --git a/pubspec.yaml b/pubspec.yaml index 5e2622a5..463d2f7f 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -52,7 +52,10 @@ dependencies: in_app_review: ^2.0.11 dart_discord_presence: ^1.2.0 flutter_svg: ^2.2.3 - background_downloader: ^9.5.2 + background_downloader: + git: + url: https://github.com/edde746/background_downloader + ref: 2017dfd sentry_flutter: git: url: https://github.com/edde746/sentry-dart