From 665603b314e3705e9f5644250942ca595c0a2bbd Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Fri, 12 Jun 2026 15:33:58 +0200 Subject: [PATCH] fix(jellyfin): bounded transient retry on hub surfaces --- lib/services/jellyfin_client.dart | 2 + .../jellyfin_client/parts/browse.dart | 77 +++++++++++++++---- .../jellyfin_client/parts/collections.dart | 2 +- .../jellyfin_client/parts/live_tv.dart | 9 ++- .../jellyfin_client/parts/metadata_edit.dart | 2 +- .../jellyfin_client/parts/playback.dart | 2 +- .../jellyfin_client/parts/playlists.dart | 2 +- .../jellyfin_client/parts/watch_state.dart | 2 +- .../jellyfin_client_failures_test.dart | 23 ++++++ 9 files changed, 101 insertions(+), 20 deletions(-) diff --git a/lib/services/jellyfin_client.dart b/lib/services/jellyfin_client.dart index 4c0e98a3..6b731853 100644 --- a/lib/services/jellyfin_client.dart +++ b/lib/services/jellyfin_client.dart @@ -39,6 +39,8 @@ import '../media/media_source_info.dart'; import '../media/media_sort.dart'; import '../utils/app_logger.dart'; import '../utils/failover_http_client.dart'; +import '../utils/media_server_retry.dart'; +import '../utils/media_server_timeouts.dart'; import '../utils/log_redaction_manager.dart'; import '../utils/external_ids.dart'; import '../utils/media_server_http_client.dart'; diff --git a/lib/services/jellyfin_client/parts/browse.dart b/lib/services/jellyfin_client/parts/browse.dart index 537b763a..e199baf3 100644 --- a/lib/services/jellyfin_client/parts/browse.dart +++ b/lib/services/jellyfin_client/parts/browse.dart @@ -2,6 +2,25 @@ part of '../../jellyfin_client.dart'; String _segment(String value) => Uri.encodeComponent(value); +/// Transport policy for a hub surface: bounded transient retries, no +/// endpoint failover. See `_getItemsResponse`. +typedef _HubRetryPolicy = ({String operation, List attemptTimeouts}); + +const _HubRetryPolicy _homeHubRetry = ( + operation: 'Jellyfin home hubs', + attemptTimeouts: MediaServerTimeouts.homeHubAttemptTimeouts, +); + +const _HubRetryPolicy _libraryHubRetry = ( + operation: 'Jellyfin library hubs', + attemptTimeouts: MediaServerTimeouts.libraryHubAttemptTimeouts, +); + +const _HubRetryPolicy _continueWatchingRetry = ( + operation: 'Jellyfin continue watching', + attemptTimeouts: MediaServerTimeouts.homeHubAttemptTimeouts, +); + List> _itemsArray(Object? data) { if (data is Map) { final items = data['Items']; @@ -111,7 +130,7 @@ const _detailFields = mixin _JellyfinBrowseMethods on MediaServerCacheMixin { JellyfinConnection get connection; - MediaServerHttpClient get _http; + FailoverHttpClient get _http; MediaItem? _mapItem(Map json); List _mapItems(Iterable> items); @@ -963,7 +982,7 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin { 'Recursive': 'true', 'EnableTotalRecordCount': 'false', ...jellyfinImageQueryParameters, - }), + }, retry: _continueWatchingRetry), _safeFetchItemsArray('/Shows/NextUp', { 'userId': connection.userId, 'Limit': ?count?.toString(), @@ -971,7 +990,7 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin { 'EnableResumable': 'false', 'EnableTotalRecordCount': 'false', ...jellyfinImageQueryParameters, - }), + }, retry: _continueWatchingRetry), ]); return _mergeContinueWatchingAndNextUp( @@ -991,7 +1010,7 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin { 'Fields': _browseFields, 'IncludeItemTypes': 'Movie,Series,Episode', ...jellyfinImageQueryParameters, - }); + }, retry: _homeHubRetry); if (!includePlaybackHubs) { final latest = await latestFuture; @@ -1019,7 +1038,7 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin { 'Recursive': 'true', 'EnableTotalRecordCount': 'false', ...jellyfinImageQueryParameters, - }), + }, retry: _homeHubRetry), _safeFetchItemsArray('/Shows/NextUp', { 'userId': connection.userId, 'Limit': limit.toString(), @@ -1027,7 +1046,7 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin { 'EnableResumable': 'false', 'EnableTotalRecordCount': 'false', ...jellyfinImageQueryParameters, - }), + }, retry: _homeHubRetry), ]); return [ @@ -1083,7 +1102,7 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin { 'ParentId': libraryId, 'Fields': _browseFields, ...jellyfinImageQueryParameters, - }); + }, retry: _libraryHubRetry); if (!includePlaybackHubs) { final latest = await latestFuture; @@ -1113,7 +1132,7 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin { 'Recursive': 'true', 'EnableTotalRecordCount': 'false', ...jellyfinImageQueryParameters, - }), + }, retry: _libraryHubRetry), includeNextUp ? _safeFetchItemsArray('/Shows/NextUp', { 'userId': connection.userId, @@ -1123,7 +1142,7 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin { 'EnableResumable': 'false', 'EnableTotalRecordCount': 'false', ...jellyfinImageQueryParameters, - }) + }, retry: _libraryHubRetry) : Future.value(const >[]), ]); @@ -1443,15 +1462,47 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin { return result; } - Future>> _fetchItemsArray(String path, Map queryParameters) async { - final response = await _http.get(path, queryParameters: queryParameters); + /// GET [path], optionally under a hub-surface transport policy ([retry]): + /// bounded transient retries with per-attempt timeouts and **no endpoint + /// failover** — a slow hub row must not move the whole client off an + /// otherwise working endpoint (same policy as Plex's three hub fetches; + /// see [retryTransientMediaServerCall] / [FailoverHttpClient]). + Future _getItemsResponse( + String path, + Map queryParameters, + _HubRetryPolicy? retry, + ) { + if (retry == null) return _http.get(path, queryParameters: queryParameters); + return retryTransientMediaServerCall( + operation: retry.operation, + attemptTimeouts: retry.attemptTimeouts, + call: (timeout, abort) => _http.get( + path, + queryParameters: queryParameters, + timeout: timeout, + abort: abort, + allowEndpointFailover: false, + ), + ); + } + + Future>> _fetchItemsArray( + String path, + Map queryParameters, { + _HubRetryPolicy? retry, + }) async { + final response = await _getItemsResponse(path, queryParameters, retry); throwIfHttpError(response); return _itemsArray(response.data); } - Future>> _safeFetchItemsArray(String path, Map queryParameters) async { + Future>> _safeFetchItemsArray( + String path, + Map queryParameters, { + _HubRetryPolicy? retry, + }) async { try { - final response = await _http.get(path, queryParameters: queryParameters); + final response = await _getItemsResponse(path, queryParameters, retry); throwIfHttpError(response); final data = response.data; if (data is List) { diff --git a/lib/services/jellyfin_client/parts/collections.dart b/lib/services/jellyfin_client/parts/collections.dart index 9e89bc26..9270e464 100644 --- a/lib/services/jellyfin_client/parts/collections.dart +++ b/lib/services/jellyfin_client/parts/collections.dart @@ -2,7 +2,7 @@ part of '../../jellyfin_client.dart'; mixin _JellyfinCollectionMethods on MediaServerCacheMixin { JellyfinConnection get connection; - MediaServerHttpClient get _http; + FailoverHttpClient get _http; List _mapItems(Iterable> items); static const int _collectionsPageSize = 36; diff --git a/lib/services/jellyfin_client/parts/live_tv.dart b/lib/services/jellyfin_client/parts/live_tv.dart index 7ade0796..c8c54e17 100644 --- a/lib/services/jellyfin_client/parts/live_tv.dart +++ b/lib/services/jellyfin_client/parts/live_tv.dart @@ -2,9 +2,14 @@ part of '../../jellyfin_client.dart'; mixin _JellyfinLiveTvMethods on MediaServerCacheMixin { JellyfinConnection get connection; - MediaServerHttpClient get _http; + FailoverHttpClient get _http; String? _absolutizeImagePath(String? path); - Future>> _safeFetchItemsArray(String path, Map queryParameters); + Future>> _safeFetchItemsArray( + String path, + Map queryParameters, { + // ignore: unused_element_parameter + _HubRetryPolicy? retry, + }); /// Returns `true` when this server has Live TV configured (channels /// available). Probes `/LiveTv/Channels?limit=1`. Used by [MultiServerProvider] diff --git a/lib/services/jellyfin_client/parts/metadata_edit.dart b/lib/services/jellyfin_client/parts/metadata_edit.dart index 2d092236..5afd2356 100644 --- a/lib/services/jellyfin_client/parts/metadata_edit.dart +++ b/lib/services/jellyfin_client/parts/metadata_edit.dart @@ -2,7 +2,7 @@ part of '../../jellyfin_client.dart'; mixin _JellyfinMetadataEditMethods on MediaServerCacheMixin { JellyfinConnection get connection; - MediaServerHttpClient get _http; + FailoverHttpClient get _http; Future?> fetchEditableMetadataItem(String itemId) async { if (isOfflineMode) return null; diff --git a/lib/services/jellyfin_client/parts/playback.dart b/lib/services/jellyfin_client/parts/playback.dart index 43a2714a..bab1cb6c 100644 --- a/lib/services/jellyfin_client/parts/playback.dart +++ b/lib/services/jellyfin_client/parts/playback.dart @@ -2,7 +2,7 @@ part of '../../jellyfin_client.dart'; mixin _JellyfinPlaybackMethods on MediaServerCacheMixin { JellyfinConnection get connection; - MediaServerHttpClient get _http; + FailoverHttpClient get _http; /// Backend-neutral [PlaybackExtras] for [itemId]. Jellyfin exposes chapters /// at the item level (`raw['Chapters']`) and native skip segments through a diff --git a/lib/services/jellyfin_client/parts/playlists.dart b/lib/services/jellyfin_client/parts/playlists.dart index 19b483a5..2d218ad0 100644 --- a/lib/services/jellyfin_client/parts/playlists.dart +++ b/lib/services/jellyfin_client/parts/playlists.dart @@ -2,7 +2,7 @@ part of '../../jellyfin_client.dart'; mixin _JellyfinPlaylistMethods on MediaServerCacheMixin { JellyfinConnection get connection; - MediaServerHttpClient get _http; + FailoverHttpClient get _http; String? _absolutizeImagePath(String? path); List _mapItems(Iterable> items); diff --git a/lib/services/jellyfin_client/parts/watch_state.dart b/lib/services/jellyfin_client/parts/watch_state.dart index 94d5f46d..df28069d 100644 --- a/lib/services/jellyfin_client/parts/watch_state.dart +++ b/lib/services/jellyfin_client/parts/watch_state.dart @@ -2,7 +2,7 @@ part of '../../jellyfin_client.dart'; mixin _JellyfinWatchStateMethods on MediaServerCacheMixin { JellyfinConnection get connection; - MediaServerHttpClient get _http; + FailoverHttpClient get _http; @override Future markWatched(MediaItem item) async { diff --git a/test/services/jellyfin_client_failures_test.dart b/test/services/jellyfin_client_failures_test.dart index d81a6bb6..e7bf3532 100644 --- a/test/services/jellyfin_client_failures_test.dart +++ b/test/services/jellyfin_client_failures_test.dart @@ -165,6 +165,29 @@ void main() { expect(client.connection.baseUrls, ['https://fallback.example.com', 'https://primary.example.com']); }); + test('hub surfaces retry transient failures without hopping endpoints', () async { + final attemptsByPath = {}; + final client = JellyfinClient.forTesting( + connection: _conn( + baseUrl: 'https://primary.example.com', + baseUrls: const ['https://primary.example.com', 'https://fallback.example.com'], + ), + httpClient: MockClient((req) async { + expect(req.url.host, 'primary.example.com', reason: 'retry-wrapped hub fetches must not fail over'); + final attempt = attemptsByPath.update(req.url.path, (n) => n + 1, ifAbsent: () => 1); + if (attempt == 1) throw TimeoutException('slow row'); + return http.Response(jsonEncode({'Items': []}), 200, headers: {'content-type': 'application/json'}); + }), + ); + addTearDown(client.close); + + final items = await client.fetchContinueWatching(); + + expect(items, isEmpty); + expect(attemptsByPath.values, everyElement(2)); + expect(client.connection.baseUrl, 'https://primary.example.com'); + }); + test('exhausting every endpoint fires onAllEndpointsExhausted', () async { var exhausted = 0; final client = JellyfinClient.forTesting(