diff --git a/go_backend/exports_download.go b/go_backend/exports_download.go index e782209d..3732936b 100644 --- a/go_backend/exports_download.go +++ b/go_backend/exports_download.go @@ -473,6 +473,26 @@ func classifyDownloadErrorType(msg string) string { return "unknown" } +// isOutputStorageWriteFailure distinguishes an unwritable destination from a +// provider-specific failure. Provider fallback cannot repair the former: all +// providers receive the same output path, so continuing only delays the +// storage fallback and can replace the useful permission error with an +// unrelated error from the last provider. +func isOutputStorageWriteFailure(errorType, message string) bool { + if strings.EqualFold(strings.TrimSpace(errorType), "permission") { + return true + } + lowerMsg := strings.ToLower(strings.TrimSpace(message)) + if lowerMsg == "" { + return false + } + return strings.Contains(lowerMsg, "operation not permitted") || + strings.Contains(lowerMsg, "permission denied") || + strings.Contains(lowerMsg, "read-only file system") || + strings.Contains(lowerMsg, "failed to create file") || + strings.Contains(lowerMsg, "failed to create directory") +} + func messageHasHTTPStatusCode(lowerMsg, code string) bool { return strings.Contains(lowerMsg, "http "+code) || strings.Contains(lowerMsg, "http status "+code) || diff --git a/go_backend/exports_supplement_test.go b/go_backend/exports_supplement_test.go index 00bfc760..6e9db8b7 100644 --- a/go_backend/exports_supplement_test.go +++ b/go_backend/exports_supplement_test.go @@ -44,6 +44,46 @@ func TestDownloadErrorClassificationDetectsVerificationRequired(t *testing.T) { } } +func TestOutputStorageWriteFailureDetection(t *testing.T) { + cases := []struct { + name string + errorType string + message string + want bool + }{ + { + name: "typed permission failure", + errorType: "permission", + message: "backend omitted details", + want: true, + }, + { + name: "android operation not permitted", + message: "failed to create file: open /storage/song.partial: operation not permitted", + want: true, + }, + { + name: "read only destination", + message: "open /music/song.flac: read-only file system", + want: true, + }, + { + name: "provider API error", + errorType: "api_error", + message: "HTTP 404 for /download", + want: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := isOutputStorageWriteFailure(tc.errorType, tc.message); got != tc.want { + t.Fatalf("isOutputStorageWriteFailure(%q, %q) = %v, want %v", tc.errorType, tc.message, got, tc.want) + } + }) + } +} + func TestGetProviderMetadataPrefersEnabledDeezerExtension(t *testing.T) { dir := t.TempDir() if err := InitExtensionSystem(filepath.Join(dir, "extensions"), filepath.Join(dir, "data")); err != nil { diff --git a/go_backend/extension_fallback.go b/go_backend/extension_fallback.go index 1c0e7896..23ba278e 100644 --- a/go_backend/extension_fallback.go +++ b/go_backend/extension_fallback.go @@ -255,12 +255,37 @@ func attemptVerifiedResumeBeforeMetadata( Service: selectedProvider, }, false } + if lastErr != nil && isOutputStorageWriteFailure(errorType, lastErr.Error()) { + return buildOutputStorageFailureResponse( + selectedProvider, + lastErr, + lastRetryAfterSeconds, + ), false + } // A failed fast attempt may still succeed after source enrichment resolves a // better provider-native ID. The normal path below retains strict/stop- // fallback semantics for that prepared retry. return nil, false } +func buildOutputStorageFailureResponse( + providerID string, + err error, + retryAfterSeconds int, +) *DownloadResponse { + message := "Output storage is not writable" + if err != nil { + message = err.Error() + } + return &DownloadResponse{ + Success: false, + Error: "Download failed: " + message, + ErrorType: "permission", + RetryAfterSeconds: retryAfterSeconds, + Service: providerID, + } +} + func DownloadWithExtensionFallback(req DownloadRequest) (*DownloadResponse, error) { pipelineStartedAt := time.Now() defer func() { @@ -511,6 +536,14 @@ func DownloadWithExtensionFallback(req DownloadRequest) (*DownloadResponse, erro if sourceErrType == "" && lastErr != nil { sourceErrType = classifyDownloadErrorType(lastErr.Error()) } + if lastErr != nil && isOutputStorageWriteFailure(sourceErrType, lastErr.Error()) { + GoLog("[DownloadWithExtensionFallback] Source extension %s hit an unwritable output path; stopping provider fallback\n", req.Source) + return buildOutputStorageFailureResponse( + req.Source, + lastErr, + lastRetryAfterSeconds, + ), nil + } if strings.EqualFold(sourceErrType, "verification_required") { GoLog("[DownloadWithExtensionFallback] Source extension %s requires verification, not trying other providers\n", req.Source) cachePreparedDownloadRequest(preparationKey, req) @@ -649,6 +682,14 @@ func DownloadWithExtensionFallback(req DownloadRequest) (*DownloadResponse, erro if effType == "" { effType = classifyDownloadErrorType(lastErr.Error()) } + if isOutputStorageWriteFailure(effType, lastErr.Error()) { + GoLog("[DownloadWithExtensionFallback] %s hit an unwritable output path; stopping provider fallback\n", providerID) + return buildOutputStorageFailureResponse( + providerID, + lastErr, + lastRetryAfterSeconds, + ), nil + } if strings.EqualFold(effType, "verification_required") { GoLog("[DownloadWithExtensionFallback] %s requires verification; pausing fallback to open the challenge\n", providerID) cachePreparedDownloadRequest(preparationKey, req) diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index 59522695..76dece7a 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -83,6 +83,28 @@ double nativeWorkerFinalizingProgress(double progress) { return min(normalized, 0.99); } +/// Whether a backend failure means the selected destination could not be +/// written. This intentionally stays provider-agnostic: once an output path is +/// unwritable, trying more audio providers against that same path only wastes +/// time and hides the actionable storage failure behind a later network error. +bool isStorageWriteFailure({String? errorType, String? errorMessage}) { + if (errorType?.trim().toLowerCase() == 'permission') return true; + + final message = errorMessage?.trim().toLowerCase() ?? ''; + if (message.isEmpty) return false; + return message.contains('operation not permitted') || + message.contains('permission denied') || + message.contains('read-only file system') || + message.contains('failed to create file') || + message.contains('failed to create directory') || + message.contains('failed to access saf directory') || + message.contains('failed to create saf file') || + message.contains('failed to open saf file') || + message.contains('failed to publish saf') || + message.contains('failed to publish deferred saf') || + message.contains('failed to copy extension output to saf'); +} + final _invalidFolderChars = RegExp(r'[<>:"/\\|?*]'); final _trimDotsAndSpacesRegex = RegExp(r'^[. ]+|[. ]+$'); final _trimUnderscoresAndSpacesRegex = RegExp(r'^[_ ]+|[_ ]+$'); @@ -154,6 +176,8 @@ class DownloadQueueNotifier extends Notifier { int _queueItemSequence = 0; bool _isLoaded = false; final Set _ensuredDirs = {}; + Future? _appFolderStorageFallback; + final Set _rejectedAppFolderRoots = {}; int _idleProgressPollTick = 0; bool _networkPausedByWifiOnly = false; List? _lastConnectivityResults; @@ -1187,10 +1211,67 @@ class DownloadQueueNotifier extends Notifier { Future _processQueue() async { if (state.isProcessing) return; - final settings = ref.read(settingsProvider); + var settings = ref.read(settingsProvider); updateSettings(settings); - final isSafMode = _isSafMode(settings); + var isSafMode = _isSafMode(settings); var iosDownloadBookmarkActive = false; + + // Validate SAF before handing the batch to either queue implementation. + // A restored/missing tree URI and an OEM-revoked grant both fall back to a + // verified writable app folder instead of failing the whole queue. + if (Platform.isAndroid && settings.storageMode == 'saf') { + var safAccessible = settings.downloadTreeUri.isNotEmpty; + if (safAccessible) { + try { + safAccessible = await PlatformBridge.validateSafTreeAccess( + settings.downloadTreeUri, + ); + } catch (e) { + safAccessible = false; + _log.w('SAF access validation threw an error: $e'); + } + } + if (!safAccessible) { + _log.w( + 'SAF grant is missing or no longer writable; using app-folder fallback', + ); + try { + await _activateAppFolderStorageFallback(); + settings = ref.read(settingsProvider); + isSafMode = false; + } catch (e) { + _log.e('Could not activate app-folder storage fallback: $e'); + for (final item in state.items) { + if (item.status == DownloadStatus.queued) { + updateItemStatus( + item.id, + DownloadStatus.failed, + error: safPermissionLostErrorMessage, + errorType: DownloadErrorType.permission, + ); + } + } + return; + } + } + } + if (Platform.isAndroid && + !isSafMode && + state.outputDir.isNotEmpty && + !await _isDirectoryWritable(Directory(state.outputDir))) { + final failedOutputDir = state.outputDir; + _log.w( + 'Configured app-folder path is not writable; resolving a safe fallback', + ); + try { + await _activateAppFolderStorageFallback( + failedOutputDir: failedOutputDir, + ); + settings = ref.read(settingsProvider); + } catch (e) { + _log.e('No writable app-folder fallback is available: $e'); + } + } if (settings.downloadNetworkMode == 'wifi_only') { final connectivityResult = await Connectivity().checkConnectivity(); final hasWifi = connectivityResult.contains(ConnectivityResult.wifi); @@ -1284,30 +1365,6 @@ class DownloadQueueNotifier extends Notifier { _log.d('Output directory: ${state.outputDir}'); } else { _log.d('Output directory: SAF (tree_uri=${settings.downloadTreeUri})'); - try { - final accessible = await PlatformBridge.validateSafTreeAccess( - settings.downloadTreeUri, - ); - if (!accessible) { - throw const FileSystemException( - 'Persisted SAF tree grant is missing or no longer writable', - ); - } - } catch (e) { - _log.e('SAF permission validation failed: $e'); - _log.w('SAF tree URI may be invalid or permission revoked'); - for (final item in state.items) { - if (item.status == DownloadStatus.queued) { - updateItemStatus( - item.id, - DownloadStatus.failed, - error: safPermissionLostErrorMessage, - ); - } - } - state = state.copyWith(isProcessing: false); - return; - } } if (!isSafMode && diff --git a/lib/providers/download_queue_provider_native_worker.dart b/lib/providers/download_queue_provider_native_worker.dart index d7cf44b4..264a27e5 100644 --- a/lib/providers/download_queue_provider_native_worker.dart +++ b/lib/providers/download_queue_provider_native_worker.dart @@ -31,6 +31,57 @@ class _NativeWorkerStartupTimeout implements Exception { } extension _DownloadQueueNativeWorker on DownloadQueueNotifier { + Future _recoverNativeWorkerStorageFailure({ + required _NativeWorkerRequestContext context, + required String errorType, + required String errorMessage, + required Map contexts, + required Set reconciledIds, + }) async { + if (!isStorageWriteFailure( + errorType: errorType, + errorMessage: errorMessage, + )) { + return false; + } + + final failedOutputDir = context.storageMode == 'saf' + ? null + : context.outputDir; + final fallbackRoot = await _activateAppFolderStorageFallback( + failedOutputDir: failedOutputDir, + ); + if (context.storageMode != 'saf' && + _pathIsInside(context.outputDir, fallbackRoot)) { + // This request already used the verified fallback. Do not create an + // infinite retry loop if the failure has another device-specific cause. + return false; + } + + _log.w( + 'Native worker could not write its destination; cancelling this run ' + 'and retrying pending items in $fallbackRoot', + ); + try { + await PlatformBridge.cancelNativeDownloadWorker(); + } catch (e) { + _log.w('Failed to cancel native worker for storage fallback: $e'); + } + + for (final pendingId in contexts.keys) { + if (reconciledIds.contains(pendingId)) continue; + final pending = _findItemById(pendingId); + if (pending == null || + pending.status == DownloadStatus.completed || + pending.status == DownloadStatus.skipped) { + continue; + } + reconciledIds.add(pendingId); + updateItemStatus(pendingId, DownloadStatus.queued, progress: 0.0); + } + return true; + } + Future _persistNativeFinalizedHistoryFallback( _NativeWorkerRequestContext context, Map result, @@ -844,10 +895,10 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { } if (status == 'failed' || status == 'skipped') { - reconciledIds.add(itemId); final result = itemSnapshot['result']; final error = itemSnapshot['error']?.toString(); if (status == 'skipped') { + reconciledIds.add(itemId); updateItemStatus(itemId, DownloadStatus.skipped); } else { final resultMap = result is Map @@ -864,6 +915,20 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { final errorType = backendErrorType == DownloadErrorType.unknown ? _downloadErrorTypeFromMessage(errorMsg) : backendErrorType; + try { + if (await _recoverNativeWorkerStorageFailure( + context: context, + errorType: resultMap?['error_type']?.toString() ?? '', + errorMessage: errorMsg, + contexts: contexts, + reconciledIds: reconciledIds, + )) { + continue; + } + } catch (e) { + _log.e('Could not recover native-worker storage failure: $e'); + } + reconciledIds.add(itemId); if (errorType == DownloadErrorType.verificationRequired) { _log.i( 'Android native worker requires verification for ${current.track.name}; switching back to interactive queue', diff --git a/lib/providers/download_queue_provider_paths.dart b/lib/providers/download_queue_provider_paths.dart index 8071306c..e0c4ffb8 100644 --- a/lib/providers/download_queue_provider_paths.dart +++ b/lib/providers/download_queue_provider_paths.dart @@ -32,6 +32,123 @@ extension _DownloadQueuePaths on DownloadQueueNotifier { return musicDir; } + Future _ensureAndroidAppSpecificOutputDir() async { + final externalDir = await getExternalStorageDirectory(); + if (externalDir == null) return null; + final outputDir = Directory( + '${externalDir.path}${Platform.pathSeparator}$_defaultOutputFolderName', + ); + if (!await outputDir.exists()) { + await outputDir.create(recursive: true); + } + return outputDir; + } + + bool _pathIsInside(String path, String directory) { + String normalize(String value) => + value.replaceAll('\\', '/').replaceAll(RegExp(r'/+$'), ''); + final normalizedPath = normalize(path); + final normalizedDirectory = normalize(directory); + return normalizedPath == normalizedDirectory || + normalizedPath.startsWith('$normalizedDirectory/'); + } + + Future _isDirectoryWritable(Directory directory) async { + File? probe; + try { + if (!await directory.exists()) { + await directory.create(recursive: true); + } + probe = File( + '${directory.path}${Platform.pathSeparator}' + '.spotiflac-write-${DateTime.now().microsecondsSinceEpoch}', + ); + await probe.writeAsBytes(const [0], flush: true); + return await probe.exists(); + } catch (_) { + return false; + } finally { + if (probe != null) { + try { + if (await probe.exists()) await probe.delete(); + } catch (_) {} + } + } + } + + Future _findWritableAppFolder({String? failedOutputDir}) async { + final candidates = Function()>[ + if (Platform.isAndroid) _ensureDefaultAndroidMusicOutputDir, + if (Platform.isAndroid) _ensureAndroidAppSpecificOutputDir, + () async => _ensureDefaultDocumentsOutputDir(), + ]; + + for (final resolveCandidate in candidates) { + try { + final candidate = await resolveCandidate(); + if (candidate == null) continue; + if (_rejectedAppFolderRoots.contains(candidate.path)) continue; + if (failedOutputDir != null && + failedOutputDir.isNotEmpty && + _pathIsInside(failedOutputDir, candidate.path)) { + _rejectedAppFolderRoots.add(candidate.path); + continue; + } + if (await _isDirectoryWritable(candidate)) return candidate; + } catch (e) { + _log.w('App-folder fallback candidate is not writable: $e'); + } + } + + throw const FileSystemException( + 'No writable app-managed download folder is available', + ); + } + + /// Switches future requests to a verified writable app-managed directory. + /// Concurrent SAF failures share one resolution so an album download does + /// not run several write probes or race settings updates. + Future _activateAppFolderStorageFallback({ + String? failedOutputDir, + }) async { + final existing = _appFolderStorageFallback; + if (existing != null) { + final existingPath = await existing; + if (failedOutputDir == null || + failedOutputDir.isEmpty || + !_pathIsInside(failedOutputDir, existingPath)) { + return existingPath; + } + _rejectedAppFolderRoots.add(existingPath); + _appFolderStorageFallback = null; + } + + final fallback = () async { + final directory = await _findWritableAppFolder( + failedOutputDir: failedOutputDir, + ); + state = state.copyWith(outputDir: directory.path); + _ensuredDirs.add(directory.path); + await ref + .read(settingsProvider.notifier) + .useAppFolderStorage(directory.path); + _log.w( + 'Storage access failed; switched downloads to app folder: ' + '${directory.path}', + ); + return directory.path; + }(); + _appFolderStorageFallback = fallback; + try { + return await fallback; + } catch (_) { + if (identical(_appFolderStorageFallback, fallback)) { + _appFolderStorageFallback = null; + } + rethrow; + } + } + Future _initOutputDir() async { if (state.outputDir.isEmpty) { try { diff --git a/lib/providers/download_queue_provider_single_item.dart b/lib/providers/download_queue_provider_single_item.dart index f60cc002..dd34505e 100644 --- a/lib/providers/download_queue_provider_single_item.dart +++ b/lib/providers/download_queue_provider_single_item.dart @@ -5,15 +5,11 @@ part of 'download_queue_provider.dart'; /// the native download call, decrypt/convert/embed finalization, and the /// history hand-off. Queue scheduling stays in the main file. extension _SingleItemDownload on DownloadQueueNotifier { - bool _isSafWriteFailure(Map result) { - final error = (result['error'] ?? result['message'] ?? '') - .toString() - .toLowerCase(); - if (error.isEmpty) return false; - return error.contains('saf') || - error.contains('content uri') || - error.contains('permission denied') || - error.contains('documentfile'); + bool _isStorageWriteFailure(Map result) { + return isStorageWriteFailure( + errorType: result['error_type']?.toString(), + errorMessage: (result['error'] ?? result['message'])?.toString(), + ); } Future _runPostProcessingHooks(String filePath, Track track) async { @@ -568,38 +564,43 @@ class _DownloadRun { outputDir: effectiveOutputDir, ); - if (effectiveSafMode && - result['success'] != true && - n._isSafWriteFailure(result)) { + if (result['success'] != true && n._isStorageWriteFailure(result)) { if (n._isLocallyCancelled(item.id)) { - _log.i('Download was cancelled before SAF fallback, skipping'); + _log.i('Download was cancelled before storage fallback, skipping'); return false; } - _log.w('SAF write failed, retrying with app-private storage'); - final fallbackDir = - appOutputDir ?? - await n._buildOutputDir( - trackToDownload, - settings.folderOrganization, - separateSingles: settings.separateSingles, - albumFolderStructure: settings.albumFolderStructure, - createPlaylistFolder: settings.createPlaylistFolder, - useAlbumArtistForFolders: settings.useAlbumArtistForFolders, - usePrimaryArtistOnly: settings.usePrimaryArtistOnly, - filterContributingArtistsInAlbumArtist: - settings.filterContributingArtistsInAlbumArtist, - playlistName: item.playlistName, - ); - appOutputDir = fallbackDir; - final fallbackResult = await _invokeDownload( - useSaf: false, - outputDir: fallbackDir, - ); - if (fallbackResult['success'] == true) { - effectiveSafMode = false; - effectiveOutputDir = fallbackDir; - finalSafFileName = null; + _log.w('Storage write failed, retrying with a writable app folder'); + try { + await n._activateAppFolderStorageFallback( + failedOutputDir: effectiveSafMode ? null : effectiveOutputDir, + ); + final fallbackDir = + appOutputDir ?? + await n._buildOutputDir( + trackToDownload, + settings.folderOrganization, + separateSingles: settings.separateSingles, + albumFolderStructure: settings.albumFolderStructure, + createPlaylistFolder: settings.createPlaylistFolder, + useAlbumArtistForFolders: settings.useAlbumArtistForFolders, + usePrimaryArtistOnly: settings.usePrimaryArtistOnly, + filterContributingArtistsInAlbumArtist: + settings.filterContributingArtistsInAlbumArtist, + playlistName: item.playlistName, + ); + appOutputDir = fallbackDir; + final fallbackResult = await _invokeDownload( + useSaf: false, + outputDir: fallbackDir, + ); result = fallbackResult; + if (fallbackResult['success'] == true) { + effectiveSafMode = false; + effectiveOutputDir = fallbackDir; + finalSafFileName = null; + } + } catch (e) { + _log.e('App-folder storage fallback failed: $e'); } } return true; @@ -1672,6 +1673,28 @@ class _DownloadRun { String errorMsg = e.toString(); DownloadErrorType errorType = DownloadErrorType.unknown; + if (isStorageWriteFailure(errorMessage: errorMsg)) { + try { + await n._activateAppFolderStorageFallback( + failedOutputDir: n._isSafMode(settings) + ? null + : (effectiveOutputDir.isNotEmpty + ? effectiveOutputDir + : n.state.outputDir), + ); + n.updateItemStatus(item.id, DownloadStatus.queued, progress: 0.0); + _log.w( + 'Storage exception recovered; re-queued ${item.track.name} in the app folder', + ); + return; + } catch (fallbackError) { + _log.e( + 'Could not recover storage exception with app-folder fallback: ' + '$fallbackError', + ); + } + } + if (errorMsg.contains('could not find Deezer equivalent') || errorMsg.contains('track not found on Deezer')) { errorMsg = 'Track not found on Deezer (Metadata Unavailable)'; diff --git a/lib/providers/settings_provider.dart b/lib/providers/settings_provider.dart index dd086150..26ad7d0c 100644 --- a/lib/providers/settings_provider.dart +++ b/lib/providers/settings_provider.dart @@ -460,6 +460,21 @@ class SettingsNotifier extends Notifier { _saveSettings(); } + /// Atomically leaves SAF and persists a writable app-managed destination. + /// + /// Keeping these fields in one update avoids an intermediate saved state + /// where app-folder mode still points at the SAF display name, or where an + /// invalid tree URI can switch the mode back to SAF. + Future useAppFolderStorage(String directory) async { + state = state.copyWith( + storageMode: 'app', + downloadDirectory: directory, + downloadDirectoryBookmark: '', + downloadTreeUri: '', + ); + await _saveSettings(); + } + void setDownloadTreeUri(String uri, {String? displayName}) { final nextDisplay = displayName ?? state.downloadDirectory; state = state.copyWith( diff --git a/test/models_and_utils_test.dart b/test/models_and_utils_test.dart index 0511cc4e..59bdc97b 100644 --- a/test/models_and_utils_test.dart +++ b/test/models_and_utils_test.dart @@ -52,6 +52,53 @@ void main() { }); }); + group('storage write failure detection', () { + test('recognizes typed and common Android filesystem failures', () { + expect( + isStorageWriteFailure( + errorType: 'permission', + errorMessage: 'provider-specific text', + ), + isTrue, + ); + expect( + isStorageWriteFailure( + errorMessage: + 'failed to create file: open /storage/song.flac.partial: ' + 'operation not permitted', + ), + isTrue, + ); + expect( + isStorageWriteFailure( + errorMessage: + 'Native finalization failed: failed to publish deferred SAF output', + ), + isTrue, + ); + }); + + test( + 'does not mistake provider or network failures for storage errors', + () { + expect( + isStorageWriteFailure( + errorType: 'api_error', + errorMessage: 'HTTP 404 for /download', + ), + isFalse, + ); + expect( + isStorageWriteFailure( + errorType: 'network', + errorMessage: 'Connection reset by peer', + ), + isFalse, + ); + }, + ); + }); + group('local library incremental scan', () { test('rescans legacy metadata rows exactly once', () { expect(