perf(home): load the home screen once instead of twice per cold start

The Discover tab fanned out its whole request set twice on every cold
start and replayed slow rows on a shrinking timeout ladder, so a healthy
remote server produced anywhere from 4s to 15s of loading.

Measured against a remote Jellyfin server with four libraries, 24
interleaved cold-start samples per side:

  requests  19 -> 9      payload  219 KB -> 94 KB
  settled   5231ms -> 2502ms median, 13222ms -> 5927ms p95

Four independent causes:

- Retry policy. `Client.send` resolves on response headers, so the
  connect budget covers the server's think time and a slow-but-alive
  query raises `connectionTimeout`. Replaying it made the server re-run
  the query with a shorter budget than the one it just missed; the
  `[10s, 8s, 5s]` ladder turned an 11s answer into an empty row after
  23s. Hub surfaces now get one whole-request deadline, retry only
  immediate connection errors, and the deadline bounds the whole call
  including the request still in flight.

- Request shape. `/Items/Latest` groups a TV library by series, so its
  rows are Series folder dtos and `RecursiveItemCount`/`ChildCount` cost
  a DB count each, per row. Hub rows now ask for `Overview` only; watch
  state survives because Jellyfin derives `UserData.Played` from
  `UnplayedItemCount` when the count fields are absent. `/Shows/NextUp`
  sends `NextUpDateCutoff` to bound the server's series-key scan, and
  `Thumb` leaves `EnableImageTypes` since nothing reads it. `UserData`
  and `PremiereDate` leave the browse set: neither is an `ItemFields`
  member, so the server dropped them anyway.

- Fan-out. Per-library hubs ran in batches of three separated by a
  barrier, so one slow library stalled every library behind it. A
  sliding window keeps the same peak concurrency without head-of-line
  blocking. Concurrent `fetchLibraries` calls now share one `/Views`
  instead of racing two identical round trips, Plex's global and music
  hub legs start together, and Jellyfin gets Plex's pool tuning.

- Duplicate pass. `DiscoverScreen.initState` starts a load and the
  online-entry hook asked for a full refresh on top of it, which
  `CoalescedLoadCoordinator` correctly queued as a trailing pass. The
  hook now calls `primeRefresh`, which rides along with a load already
  in flight; profile switches still go through `fullRefresh`.

Refs #1784
This commit is contained in:
edde746
2026-08-04 04:35:06 +02:00
parent 8624c37041
commit 74d3af3ae1
19 changed files with 831 additions and 181 deletions
+10
View File
@@ -4,6 +4,16 @@ mixin Refreshable {
mixin FullRefreshable { mixin FullRefreshable {
void fullRefresh(); void fullRefresh();
/// Online-entry variant, used by `main_screen._primeOnlineServices` on cold
/// start and on reconnect-from-offline. Its job is to guarantee the tab
/// loads once servers are up — not to force a refetch.
///
/// Screens that already kick off their own load in `initState` override this
/// to skip while that pass is still in flight; otherwise the prime queues an
/// identical trailing pass and the tab fetches everything twice (#1784).
/// Profile switches go through [fullRefresh] instead, which always refetches.
void primeRefresh() => fullRefresh();
} }
mixin FocusableTab { mixin FocusableTab {
+5
View File
@@ -167,6 +167,11 @@ class DiscoverProvider extends ChangeNotifier with DisposableChangeNotifierMixin
return _loadCoordinator.requestFull(); return _loadCoordinator.requestFull();
} }
/// Whether a [load] pass is already running. The startup online-entry hook
/// uses this to skip a prime that would only duplicate the load the screen
/// started in `initState`.
bool get isLoadInFlight => _loadCoordinator.isBusy;
Future<void> _loadOnce() async { Future<void> _loadOnce() async {
// Yield to the microtask queue before the first notify so a load() // Yield to the microtask queue before the first notify so a load()
// kicked off during build (the screen's initState) doesn't mark // kicked off during build (the screen's initState) doesn't mark
+11
View File
@@ -600,6 +600,17 @@ class _DiscoverScreenState extends State<DiscoverScreen>
unawaited(_discover.load()); unawaited(_discover.load());
} }
@override
void primeRefresh() {
// `initState` already fired `load()`. On cold start that pass is still
// running when the online-entry hook primes the tab, and asking again only
// queues an identical trailing pass — the whole home fan-out twice (#1784).
// When nothing is in flight (reconnect-from-offline, or a first pass that
// gave up because no server was online yet) a real refresh is still owed.
if (_discover.isLoadInFlight) return;
fullRefresh();
}
/// Whether the loaded hubs span more than one connected server. /// Whether the loaded hubs span more than one connected server.
bool _hubsSpanMultipleServers() { bool _hubsSpanMultipleServers() {
final serverIds = _hubs.where((hub) => hub.serverId != null).map((hub) => hub.serverId).toSet(); final serverIds = _hubs.where((hub) => hub.serverId != null).map((hub) => hub.serverId).toSet();
+16 -5
View File
@@ -520,7 +520,7 @@ class _MainScreenState extends State<MainScreen>
} }
if (!mounted) return; if (!mounted) return;
_fullRefreshContentTabs(); _primeContentTabs();
} }
/// Single-shot "resume queued downloads once any client is online" rule, /// Single-shot "resume queued downloads once any client is online" rule,
@@ -1634,15 +1634,26 @@ class _MainScreenState extends State<MainScreen>
if (_screenKeys[tab]?.currentState case final T state) fn(state); if (_screenKeys[tab]?.currentState case final T state) fn(state);
} }
/// Full-refresh the primary content tabs. Shared by the online-entry hook /// Full-refresh the primary content tabs. Used by the profile-switch
/// ([_primeOnlineServices]) and the profile-switch invalidation /// invalidation ([_invalidateAllScreens]), which must refetch everything for
/// ([_invalidateAllScreens]), which refresh the same set. /// the new identity.
void _fullRefreshContentTabs() { void _fullRefreshContentTabs() {
for (final tab in const [NavigationTabId.discover, NavigationTabId.libraries, NavigationTabId.search]) { for (final tab in _contentTabs) {
_onScreen<FullRefreshable>(tab, (screen) => screen.fullRefresh()); _onScreen<FullRefreshable>(tab, (screen) => screen.fullRefresh());
} }
} }
/// Online-entry variant used by [_primeOnlineServices] on cold start and on
/// reconnect-from-offline. Screens that already started their own load skip
/// it; see [FullRefreshable.primeRefresh].
void _primeContentTabs() {
for (final tab in _contentTabs) {
_onScreen<FullRefreshable>(tab, (screen) => screen.primeRefresh());
}
}
static const _contentTabs = [NavigationTabId.discover, NavigationTabId.libraries, NavigationTabId.search];
Widget _buildBottomNavigationBar(BuildContext context, {required bool hideLabels}) { Widget _buildBottomNavigationBar(BuildContext context, {required bool hideLabels}) {
final tabs = _getBottomNavigationTabs(context); final tabs = _getBottomNavigationTabs(context);
final selectedIndex = tabs.indexWhere((tab) => tab.id == _currentTab); final selectedIndex = tabs.indexWhere((tab) => tab.id == _currentTab);
+38 -24
View File
@@ -453,29 +453,35 @@ class DataAggregationService {
final serverLibraries = libraries?[serverId]; final serverLibraries = libraries?[serverId];
final shouldUseGlobalHubs = useGlobalHubs && client.capabilities.richHubs; final shouldUseGlobalHubs = useGlobalHubs && client.capabilities.richHubs;
final hubItemLimit = limit ?? defaultHubPreviewLimit; final hubItemLimit = limit ?? defaultHubPreviewLimit;
final hubs = shouldUseGlobalHubs List<MediaHub> hubs;
? [ if (shouldUseGlobalHubs) {
...await client.fetchGlobalHubs(limit: hubItemLimit, includePlaybackHubs: includePlaybackHubs), // Both legs are independent, so start them before awaiting either.
// Spreading `...await a, ...await b` into one list literal evaluates
// them in order, which serialised the music rows behind the global
// hub round trip.
final globalFuture = client.fetchGlobalHubs(limit: hubItemLimit, includePlaybackHubs: includePlaybackHubs);
// Plex's promoted/global hub endpoint never includes music // Plex's promoted/global hub endpoint never includes music
// libraries — append their per-library hubs so music rows // libraries — append their per-library hubs so music rows
// reach home. No-op (zero extra calls) without a visible // reach home. No-op (zero extra calls) without a visible
// music library. // music library.
...await _fetchLibraryHubsForClient( final musicFuture = _fetchLibraryHubsForClient(
client, client,
limit: hubItemLimit, limit: hubItemLimit,
hiddenLibraryKeys: hiddenLibraryKeys, hiddenLibraryKeys: hiddenLibraryKeys,
includePlaybackHubs: includePlaybackHubs, includePlaybackHubs: includePlaybackHubs,
libraries: serverLibraries ?? const [], libraries: serverLibraries ?? const [],
kinds: const {MediaKind.artist}, kinds: const {MediaKind.artist},
), );
] hubs = [...await globalFuture, ...await musicFuture];
: await _fetchLibraryHubsForClient( } else {
hubs = await _fetchLibraryHubsForClient(
client, client,
limit: hubItemLimit, limit: hubItemLimit,
hiddenLibraryKeys: hiddenLibraryKeys, hiddenLibraryKeys: hiddenLibraryKeys,
includePlaybackHubs: includePlaybackHubs, includePlaybackHubs: includePlaybackHubs,
libraries: useGlobalHubs ? serverLibraries : null, libraries: useGlobalHubs ? serverLibraries : null,
); );
}
return _postProcessHubs(hubs, serverId: ServerId(serverId), hiddenLibraryKeys: hiddenLibraryKeys); return _postProcessHubs(hubs, serverId: ServerId(serverId), hiddenLibraryKeys: hiddenLibraryKeys);
}, },
); );
@@ -506,31 +512,39 @@ class DataAggregationService {
return true; return true;
}).toList(); }).toList();
// Sliding window rather than batches of three separated by a barrier: a
// batch waits for its slowest member before the next one starts, so wall
// time was the sum of per-batch maxima and one slow library stalled every
// library queued behind it (#1784). Starting the next request the moment
// any slot frees keeps the same peak concurrency with no head-of-line
// blocking. Results are written back by index so hub order stays the
// library order regardless of completion order.
const concurrency = 3; const concurrency = 3;
final all = <MediaHub>[]; final results = List<List<MediaHub>>.filled(visible.length, const []);
for (var start = 0; start < visible.length; start += concurrency) { var next = 0;
final batch = visible.skip(start).take(concurrency);
final results = await Future.wait( Future<void> worker() async {
batch.map((l) async { while (true) {
final index = next++;
if (index >= visible.length) return;
final library = visible[index];
try { try {
return await client.fetchLibraryHubs( results[index] = await client.fetchLibraryHubs(
l.id, library.id,
libraryName: l.title, libraryName: library.title,
limit: limit, limit: limit,
includePlaybackHubs: includePlaybackHubs, includePlaybackHubs: includePlaybackHubs,
libraryKind: l.kind, libraryKind: library.kind,
); );
} catch (e, st) { } catch (e, st) {
appLogger.e('Failed to fetch library hubs for ${l.globalKey}', error: e, stackTrace: st); appLogger.e('Failed to fetch library hubs for ${library.globalKey}', error: e, stackTrace: st);
return <MediaHub>[];
}
}),
);
for (final list in results) {
all.addAll(list);
} }
} }
return all; }
await Future.wait([for (var i = 0; i < concurrency && i < visible.length; i++) worker()]);
return [for (final list in results) ...list];
} }
/// Filter hidden-library items and drop empty hubs. /// Filter hidden-library items and drop empty hubs.
+5
View File
@@ -177,6 +177,11 @@ class JellyfinClient
baseUrl: connection.baseUrl, baseUrl: connection.baseUrl,
defaultHeaders: headers, defaultHeaders: headers,
logLabel: 'Jellyfin', logLabel: 'Jellyfin',
// Same pool tuning Plex uses: the home fan-out issues several concurrent
// requests per pass, and the untuned dart:io default drops idle
// connections after 15s — a fresh TLS handshake per request on a
// high-RTT/CDN link.
usePlexApiClient: true,
prioritizedEndpoints: connection.baseUrls, prioritizedEndpoints: connection.baseUrls,
onEndpointSwitch: (newBaseUrl, {required persist}) => client._handleEndpointSwitch(newBaseUrl, persist: persist), onEndpointSwitch: (newBaseUrl, {required persist}) => client._handleEndpointSwitch(newBaseUrl, persist: persist),
onAllEndpointsExhausted: onAllEndpointsExhausted, onAllEndpointsExhausted: onAllEndpointsExhausted,
+82 -29
View File
@@ -2,23 +2,21 @@ part of '../../jellyfin_client.dart';
String _segment(String value) => Uri.encodeComponent(value); String _segment(String value) => Uri.encodeComponent(value);
/// Transport policy for a hub surface: bounded transient retries, no /// Transport policy for a hub surface: one whole-request deadline, retries
/// endpoint failover. See `_getItemsResponse`. /// only on immediate connection errors, no endpoint failover. See
typedef _HubRetryPolicy = ({String operation, List<Duration> attemptTimeouts}); /// `_getItemsResponse` and [retryTransientMediaServerCall].
typedef _HubRetryPolicy = ({String operation, Duration deadline});
const _HubRetryPolicy _homeHubRetry = ( const _HubRetryPolicy _homeHubRetry = (operation: 'Jellyfin home hubs', deadline: MediaServerTimeouts.homeHubDeadline);
operation: 'Jellyfin home hubs',
attemptTimeouts: MediaServerTimeouts.homeHubAttemptTimeouts,
);
const _HubRetryPolicy _libraryHubRetry = ( const _HubRetryPolicy _libraryHubRetry = (
operation: 'Jellyfin library hubs', operation: 'Jellyfin library hubs',
attemptTimeouts: MediaServerTimeouts.libraryHubAttemptTimeouts, deadline: MediaServerTimeouts.libraryHubDeadline,
); );
const _HubRetryPolicy _continueWatchingRetry = ( const _HubRetryPolicy _continueWatchingRetry = (
operation: 'Jellyfin continue watching', operation: 'Jellyfin continue watching',
attemptTimeouts: MediaServerTimeouts.homeHubAttemptTimeouts, deadline: MediaServerTimeouts.homeHubDeadline,
); );
List<Map<String, dynamic>> _itemsArray(Object? data) { List<Map<String, dynamic>> _itemsArray(Object? data) {
@@ -58,15 +56,47 @@ LibraryPage<T> _pagedItems<T>(
/// list calls; we ask for the minimum extras needed to drive the /// list calls; we ask for the minimum extras needed to drive the
/// MediaItem mapper: /// MediaItem mapper:
/// - `RecursiveItemCount`/`ChildCount` for series leaf count /// - `RecursiveItemCount`/`ChildCount` for series leaf count
/// - `UserData` is included in defaults but pinned for safety
/// - `PremiereDate` for sort-by-release-date and episode metadata
/// - `OriginalTitle`/`SortName` for sort + alphabetised display /// - `OriginalTitle`/`SortName` for sort + alphabetised display
/// - `Overview` so list rows can show their description /// - `Overview` so list rows can show their description
/// ///
/// Heavier fields (`MediaSources`, `People`, `Genres`, `Tags`, `Studios`, /// Heavier fields (`MediaSources`, `People`, `Genres`, `Tags`, `Studios`,
/// `Taglines`, `ProviderIds`, `Chapters`) stay in [_detailFields] — together /// `Taglines`, `ProviderIds`, `Chapters`) stay in [_detailFields] — together
/// they added seconds to large-library pages on small home servers. /// they added seconds to large-library pages on small home servers.
const _browseFields = 'RecursiveItemCount,ChildCount,UserData,PremiereDate,OriginalTitle,SortName,Overview'; ///
/// `UserData` and `PremiereDate` are deliberately absent: neither is a member
/// of Jellyfin's `ItemFields` enum, so the server's
/// `CommaDelimitedCollectionModelBinder` drops them element-by-element and
/// they never did anything. `UserData` is governed by `EnableUserData`
/// (default true) and `dto.PremiereDate` is set unconditionally.
const _browseFields = 'RecursiveItemCount,ChildCount,OriginalTitle,SortName,Overview';
/// Field set for the home / per-library hub rows (Recently Added, Continue
/// Watching, Next Up). Poster cards render artwork, title, year and the
/// watch badge; only `Overview` needs asking for, to feed the mobile hero
/// (`discover_screen`) and the TV spotlight blurb.
///
/// Crucially this drops `RecursiveItemCount`/`ChildCount`. `/Items/Latest`
/// groups a TV library by series, so those rows are Series FOLDER dtos and
/// each count field costs a per-row DB query server-side
/// (`Folder.GetRecursiveChildCount` / `Folder.GetChildCount`) — the same cost
/// already documented on [_folderBrowseFields] and [_musicAlbumRowFields],
/// which the home rows never got (#1784).
///
/// Watch state survives intact: Jellyfin computes `UserData.Played` from
/// `UnplayedItemCount` when `RecursiveItemCount` is absent
/// (`Folder.FillUserDataDtoValues`), and [MediaItem.unwatchedCount] falls back
/// to `UserData.UnplayedItemCount`. Only the season progress bar needs real
/// leaf totals, and seasons never appear on a hub row.
const _hubRowFields = 'Overview';
/// How far back `/Shows/NextUp` looks for a series to resume, mirroring
/// Jellyfin web's `maxDaysForNextUp` default. Without it the server's
/// `GetNextUpSeriesKeys` scan is unbounded over every series the user has ever
/// played an episode of.
const _nextUpDateCutoffDays = 365;
String _nextUpDateCutoff() =>
DateTime.now().toUtc().subtract(const Duration(days: _nextUpDateCutoffDays)).toIso8601String();
/// Existing episode-row requests can show Plex-style quality labels when the /// Existing episode-row requests can show Plex-style quality labels when the
/// response includes `MediaSources`. Keep this off broad library/search/latest /// response includes `MediaSources`. Keep this off broad library/search/latest
@@ -213,11 +243,32 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
/// `_providerLibraries`. /// `_providerLibraries`.
List<MediaLibrary>? _loadedLibraryViews; List<MediaLibrary>? _loadedLibraryViews;
/// In-flight `/Views` request, shared by concurrent callers.
///
/// At cold start `LibrariesProvider.loadLibraries()` and
/// `DataAggregationService.getHubsFromAllServers` both ask for libraries at
/// the same time, and the hub fan-out sits *serially* behind its copy. Two
/// identical uncached round trips is a full RTT of pure cold-start latency on
/// a remote server (#1784). Plex has never paid it — its library list comes
/// from the `/media/providers` response cached at client creation.
///
/// Single-flight only: once the request settles the next caller re-fetches,
/// so a library added server-side still shows up on the next refresh.
Future<List<MediaLibrary>>? _inFlightLibraries;
@override @override
Future<List<MediaLibrary>> fetchLibraries() async { Future<List<MediaLibrary>> fetchLibraries() {
final libraries = await _fetchLibraries(); final inFlight = _inFlightLibraries;
if (inFlight != null) return inFlight;
final request = _fetchLibraries().then((libraries) {
_loadedLibraryViews = libraries; _loadedLibraryViews = libraries;
return libraries; return libraries;
});
_inFlightLibraries = request;
return request.whenComplete(() {
if (identical(_inFlightLibraries, request)) _inFlightLibraries = null;
});
} }
/// [abort] tears the view fetch down with the pass that owns it — a /// [abort] tears the view fetch down with the pass that owns it — a
@@ -1414,7 +1465,7 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
_fetchItemsArray('/UserItems/Resume', { _fetchItemsArray('/UserItems/Resume', {
'userId': connection.userId, 'userId': connection.userId,
'Limit': ?count?.toString(), 'Limit': ?count?.toString(),
'Fields': _browseFields, 'Fields': _hubRowFields,
'MediaTypes': 'Video', 'MediaTypes': 'Video',
'Recursive': 'true', 'Recursive': 'true',
'EnableTotalRecordCount': 'false', 'EnableTotalRecordCount': 'false',
@@ -1423,8 +1474,9 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
_safeFetchItemsArray('/Shows/NextUp', { _safeFetchItemsArray('/Shows/NextUp', {
'userId': connection.userId, 'userId': connection.userId,
'Limit': ?count?.toString(), 'Limit': ?count?.toString(),
'Fields': _browseFields, 'Fields': _hubRowFields,
'EnableResumable': 'false', 'EnableResumable': 'false',
'NextUpDateCutoff': _nextUpDateCutoff(),
'EnableTotalRecordCount': 'false', 'EnableTotalRecordCount': 'false',
...jellyfinImageQueryParameters, ...jellyfinImageQueryParameters,
}, retry: _continueWatchingRetry), }, retry: _continueWatchingRetry),
@@ -1519,7 +1571,7 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
final latestFuture = _safeFetchItemsArray('/Users/${_segment(connection.userId)}/Items/Latest', { final latestFuture = _safeFetchItemsArray('/Users/${_segment(connection.userId)}/Items/Latest', {
'Limit': limit.toString(), 'Limit': limit.toString(),
'ParentId': ?parentId, 'ParentId': ?parentId,
'Fields': _browseFields, 'Fields': _hubRowFields,
'IncludeItemTypes': ?latestItemTypes, 'IncludeItemTypes': ?latestItemTypes,
...jellyfinImageQueryParameters, ...jellyfinImageQueryParameters,
}, retry: retry); }, retry: retry);
@@ -1547,7 +1599,7 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
'userId': connection.userId, 'userId': connection.userId,
'ParentId': ?parentId, 'ParentId': ?parentId,
'Limit': limit.toString(), 'Limit': limit.toString(),
'Fields': _browseFields, 'Fields': _hubRowFields,
'MediaTypes': 'Video', 'MediaTypes': 'Video',
'Recursive': 'true', 'Recursive': 'true',
'EnableTotalRecordCount': 'false', 'EnableTotalRecordCount': 'false',
@@ -1558,8 +1610,9 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
'userId': connection.userId, 'userId': connection.userId,
'ParentId': ?parentId, 'ParentId': ?parentId,
'Limit': limit.toString(), 'Limit': limit.toString(),
'Fields': _browseFields, 'Fields': _hubRowFields,
'EnableResumable': 'false', 'EnableResumable': 'false',
'NextUpDateCutoff': _nextUpDateCutoff(),
'EnableTotalRecordCount': 'false', 'EnableTotalRecordCount': 'false',
...jellyfinImageQueryParameters, ...jellyfinImageQueryParameters,
}, retry: retry) }, retry: retry)
@@ -1701,7 +1754,7 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
'IncludeItemTypes': 'Movie,Series,Episode,Video,MusicVideo,Photo', 'IncludeItemTypes': 'Movie,Series,Episode,Video,MusicVideo,Photo',
'SortBy': 'DateCreated,SortName,ProductionYear', 'SortBy': 'DateCreated,SortName,ProductionYear',
'SortOrder': 'Descending,Descending,Descending', 'SortOrder': 'Descending,Descending,Descending',
'Fields': _browseFields, 'Fields': _hubRowFields,
...jellyfinImageQueryParameters, ...jellyfinImageQueryParameters,
}, },
offset: offset, offset: offset,
@@ -1732,7 +1785,7 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
'userId': connection.userId, 'userId': connection.userId,
'StartIndex': offset.toString(), 'StartIndex': offset.toString(),
'Limit': effectiveLimit, 'Limit': effectiveLimit,
'Fields': _browseFields, 'Fields': _hubRowFields,
'Recursive': 'true', 'Recursive': 'true',
'EnableTotalRecordCount': 'true', 'EnableTotalRecordCount': 'true',
if (parentId != null) 'ParentId': parentId else 'MediaTypes': 'Video', if (parentId != null) 'ParentId': parentId else 'MediaTypes': 'Video',
@@ -1749,9 +1802,10 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
'userId': connection.userId, 'userId': connection.userId,
'StartIndex': offset.toString(), 'StartIndex': offset.toString(),
'Limit': effectiveLimit, 'Limit': effectiveLimit,
'Fields': _browseFields, 'Fields': _hubRowFields,
'ParentId': ?parentId, 'ParentId': ?parentId,
'EnableResumable': 'false', 'EnableResumable': 'false',
'NextUpDateCutoff': _nextUpDateCutoff(),
'EnableTotalRecordCount': 'true', 'EnableTotalRecordCount': 'true',
...jellyfinImageQueryParameters, ...jellyfinImageQueryParameters,
}, },
@@ -2036,14 +2090,13 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
} }
/// GET [path], optionally under a hub-surface transport policy ([retry]): /// GET [path], optionally under a hub-surface transport policy ([retry]):
/// bounded transient retries with per-attempt timeouts and **no endpoint /// one whole-request deadline, retries only on immediate connection errors,
/// failover** — a slow hub row must not move the whole client off an /// and **no endpoint failover** — a slow hub row must not move the whole
/// otherwise working endpoint (same policy as Plex's three hub fetches; /// client off an otherwise working endpoint (same policy as Plex's three hub
/// see [retryTransientMediaServerCall] / [FailoverHttpClient]). /// fetches; see [retryTransientMediaServerCall] / [FailoverHttpClient]).
/// ///
/// [timeout] and [allowEndpointFailover] configure the un-retried path only; a /// [timeout] and [allowEndpointFailover] configure the un-retried path only; a
/// [retry] policy carries its own per-attempt timeouts and always disables /// [retry] policy carries its own deadline and always disables failover.
/// failover.
Future<MediaServerResponse> _getItemsResponse( Future<MediaServerResponse> _getItemsResponse(
String path, String path,
Map<String, dynamic> queryParameters, Map<String, dynamic> queryParameters,
@@ -2064,7 +2117,7 @@ mixin _JellyfinBrowseMethods on _JellyfinClientInternals {
abort?.throwIfAborted(); abort?.throwIfAborted();
return retryTransientMediaServerCall( return retryTransientMediaServerCall(
operation: retry.operation, operation: retry.operation,
attemptTimeouts: retry.attemptTimeouts, deadline: retry.deadline,
call: (timeout, attemptAbort) => _http.get( call: (timeout, attemptAbort) => _http.get(
path, path,
queryParameters: queryParameters, queryParameters: queryParameters,
+6 -1
View File
@@ -5,8 +5,13 @@ import 'plex_constants.dart';
/// Browse responses retain up to three backdrops so hero surfaces can rotate /// Browse responses retain up to three backdrops so hero surfaces can rotate
/// artwork without allowing image-tag payloads to grow without bound. /// artwork without allowing image-tag payloads to grow without bound.
const jellyfinBackdropImageLimit = 3; const jellyfinBackdropImageLimit = 3;
/// `Thumb` is deliberately absent: `JellyfinMappers` never reads
/// `ImageTags['Thumb']`, and `parentThumbPath`/`grandparentThumbPath` are built
/// from the season/series *Primary* tags. Asking for it added a dead image type
/// to ~40 requests and widened the server's inherited-image parent walk.
const jellyfinImageQueryParameters = <String, String>{ const jellyfinImageQueryParameters = <String, String>{
'EnableImageTypes': 'Primary,Backdrop,Thumb,Logo', 'EnableImageTypes': 'Primary,Backdrop,Logo',
'ImageTypeLimit': '$jellyfinBackdropImageLimit', 'ImageTypeLimit': '$jellyfinBackdropImageLimit',
}; };
+6 -6
View File
@@ -1564,7 +1564,7 @@ class PlexClient
final response = await retryTransientMediaServerCall( final response = await retryTransientMediaServerCall(
operation: 'Plex continue watching hubs', operation: 'Plex continue watching hubs',
attemptTimeouts: MediaServerTimeouts.homeHubAttemptTimeouts, deadline: MediaServerTimeouts.homeHubDeadline,
call: (timeout, abort) => _getWithFailover( call: (timeout, abort) => _getWithFailover(
continueWatchingHubKey ?? '/hubs', continueWatchingHubKey ?? '/hubs',
queryParameters: queryParameters, queryParameters: queryParameters,
@@ -2244,7 +2244,7 @@ class PlexClient
required String path, required String path,
required Map<String, dynamic> queryParameters, required Map<String, dynamic> queryParameters,
required String operation, required String operation,
required List<Duration> attemptTimeouts, required Duration deadline,
required String failureLabel, required String failureLabel,
int? librarySectionID, int? librarySectionID,
String? librarySectionTitle, String? librarySectionTitle,
@@ -2253,7 +2253,7 @@ class PlexClient
try { try {
final response = await retryTransientMediaServerCall( final response = await retryTransientMediaServerCall(
operation: operation, operation: operation,
attemptTimeouts: attemptTimeouts, deadline: deadline,
call: (timeout, abort) => _getWithFailover( call: (timeout, abort) => _getWithFailover(
path, path,
queryParameters: queryParameters, queryParameters: queryParameters,
@@ -2291,7 +2291,7 @@ class PlexClient
path: '/hubs/sections/$sectionId', path: '/hubs/sections/$sectionId',
queryParameters: {'count': limit, 'includeGuids': 1}, queryParameters: {'count': limit, 'includeGuids': 1},
operation: 'Plex library hubs', operation: 'Plex library hubs',
attemptTimeouts: MediaServerTimeouts.libraryHubAttemptTimeouts, deadline: MediaServerTimeouts.libraryHubDeadline,
failureLabel: 'library hubs', failureLabel: 'library hubs',
librarySectionID: _librarySectionIdFromString(sectionId), librarySectionID: _librarySectionIdFromString(sectionId),
librarySectionTitle: libraryName, librarySectionTitle: libraryName,
@@ -2305,7 +2305,7 @@ class PlexClient
path: _providerPromotedHubKey ?? _providerHomeHubKey ?? '/hubs', path: _providerPromotedHubKey ?? _providerHomeHubKey ?? '/hubs',
queryParameters: {'count': limit, 'includeGuids': 1}, queryParameters: {'count': limit, 'includeGuids': 1},
operation: 'Plex global hubs', operation: 'Plex global hubs',
attemptTimeouts: MediaServerTimeouts.homeHubAttemptTimeouts, deadline: MediaServerTimeouts.homeHubDeadline,
failureLabel: 'global hubs', failureLabel: 'global hubs',
); );
@@ -2314,7 +2314,7 @@ class PlexClient
path: '/hubs/metadata/$ratingKey/related', path: '/hubs/metadata/$ratingKey/related',
queryParameters: {'count': count}, queryParameters: {'count': count},
operation: 'Plex related hubs', operation: 'Plex related hubs',
attemptTimeouts: MediaServerTimeouts.libraryHubAttemptTimeouts, deadline: MediaServerTimeouts.libraryHubDeadline,
failureLabel: 'related hubs', failureLabel: 'related hubs',
filter: _videoOrCollectionHubItem, filter: _videoOrCollectionHubItem,
); );
@@ -21,6 +21,11 @@ final class CoalescedLoadCoordinator<T> {
bool _pendingFull = false; bool _pendingFull = false;
bool _disposed = false; bool _disposed = false;
/// Whether a pass is running (or queued behind the running one). Lets a
/// caller tell "this surface is already loading" from "nothing has started",
/// without changing the drain's trailing-replay contract.
bool get isBusy => _inFlight != null;
Future<void> requestFull() { Future<void> requestFull() {
if (_disposed) return Future<void>.value(); if (_disposed) return Future<void>.value();
_pendingFull = true; _pendingFull = true;
+54 -21
View File
@@ -4,50 +4,83 @@ import 'media_server_http_client.dart';
typedef MediaServerRetryCall<T> = Future<T> Function(Duration timeout, AbortController abort); typedef MediaServerRetryCall<T> = Future<T> Function(Duration timeout, AbortController abort);
/// Retries media-server calls only when the failure is transient transport /// Runs a media-server call under a single whole-request [deadline], retrying
/// noise. Callers pass per-attempt timeouts so cold-start surfaces can use a /// only failures that cost nothing to retry.
/// bounded retry budget without changing global HTTP defaults.
/// ///
/// Retry vs failover (see `FailoverHttpClient` for the other half): retry is /// Retry vs failover (see `FailoverHttpClient` for the other half): retry is
/// for a *slow-but-working* endpoint, failover is for a *dead* one. Surfaces /// for a *slow-but-working* endpoint, failover is for a *dead* one. Surfaces
/// wrapped in this helper should pass `allowEndpointFailover: false` on the /// wrapped in this helper should pass `allowEndpointFailover: false` on the
/// inner GET so a slow row doesn't move the whole client off an otherwise /// inner GET so a slow row doesn't move the whole client off an otherwise
/// working endpoint — every existing combined call site does. /// working endpoint — every existing combined call site does.
///
/// ## Why timeouts are not retried
///
/// `http.Client.send` resolves when response *headers* arrive, so
/// [MediaServerHttpClient]'s connect budget covers DNS + TCP + TLS + request +
/// **the server's think time**. A Jellyfin `/Items/Latest` that needs 11s on a
/// large library therefore raises a `TimeoutException`, which
/// [MediaServerHttpException.from] types as
/// [MediaServerHttpErrorType.connectionTimeout] — indistinguishable from a
/// failed socket connect.
///
/// Replaying it makes the server re-run the same expensive query from scratch,
/// with a *shorter* budget than the one it just missed. The old
/// `[10s, 8s, 5s]` ladder turned an 11s answer into an empty row after 23s
/// (#1784). A timeout means "this endpoint is slower than we can wait", and the
/// answer to that is to give up, not to ask three times.
///
/// [MediaServerHttpErrorType.connectionError] is different: a refused
/// connection, DNS failure or reset socket fails immediately and costs the
/// server nothing, and is frequently a one-off on a mobile link. Those are
/// retried up to [maxConnectionRetries] times.
///
/// [deadline] bounds the whole call — every attempt, plus whatever the last one
/// is still waiting on — and aborts the in-flight request when it expires. Each
/// attempt is *also* handed [deadline] as its own budget so the HTTP layer
/// fails on its own terms first; the outer bound is the backstop that makes the
/// guarantee unconditional.
Future<T> retryTransientMediaServerCall<T>({ Future<T> retryTransientMediaServerCall<T>({
required String operation, required String operation,
required List<Duration> attemptTimeouts, required Duration deadline,
required MediaServerRetryCall<T> call, required MediaServerRetryCall<T> call,
}) async { int maxConnectionRetries = 2,
if (attemptTimeouts.isEmpty) { }) {
throw ArgumentError.value(attemptTimeouts, 'attemptTimeouts', 'must contain at least one timeout'); if (deadline <= Duration.zero) {
throw ArgumentError.value(deadline, 'deadline', 'must be positive');
} }
for (var attempt = 0; attempt < attemptTimeouts.length; attempt++) { AbortController? inFlight;
final timeout = attemptTimeouts[attempt];
Future<T> attempts() async {
for (var attempt = 0; ; attempt++) {
final abort = AbortController(); final abort = AbortController();
inFlight = abort;
try { try {
return await call(timeout, abort); return await call(deadline, abort);
} on MediaServerHttpException catch (e, st) { } on MediaServerHttpException catch (e, st) {
abort.abort(); abort.abort();
final isLastAttempt = attempt == attemptTimeouts.length - 1; if (e.type != MediaServerHttpErrorType.connectionError || attempt >= maxConnectionRetries) {
if (!e.isTransient || isLastAttempt) {
Error.throwWithStackTrace(e, st); Error.throwWithStackTrace(e, st);
} }
appLogger.w( appLogger.w(
'Retrying $operation after transient media-server failure', 'Retrying $operation after a connection error',
error: { error: {'attempt': attempt + 1, 'maxAttempts': maxConnectionRetries + 1, 'type': e.type.name},
'attempt': attempt + 1,
'maxAttempts': attemptTimeouts.length,
'nextTimeoutMs': attemptTimeouts[attempt + 1].inMilliseconds,
'type': e.type.name,
},
); );
} catch (e, st) { } catch (e, st) {
abort.abort(); abort.abort();
Error.throwWithStackTrace(e, st); Error.throwWithStackTrace(e, st);
} }
} }
}
throw StateError('unreachable retry state'); return attempts().timeout(
deadline,
onTimeout: () {
inFlight?.abort();
throw MediaServerHttpException(
type: MediaServerHttpErrorType.connectionTimeout,
message: '$operation exceeded its ${deadline.inSeconds}s budget',
);
},
);
} }
+17 -6
View File
@@ -7,13 +7,24 @@ class MediaServerTimeouts {
static const receive = Duration(seconds: 120); static const receive = Duration(seconds: 120);
/// Retry budget for home `/hubs` startup calls. These endpoints can be slow /// Whole-request deadline for home `/hubs` startup calls. These endpoints can
/// while Plex wakes idle disks, but should not block forever. /// be slow while Plex wakes idle disks or a CDN-fronted Jellyfin runs a cold
static const homeHubAttemptTimeouts = [Duration(seconds: 10), Duration(seconds: 5), Duration(milliseconds: 2500)]; /// query, but should not block forever.
///
/// Deliberately a *single* budget rather than a retry ladder. `Client.send`
/// resolves when response headers arrive, so this budget covers the server's
/// think time, not just the socket connect — a slow-but-alive query trips it.
/// Replaying that request makes the server re-run the same expensive query
/// from scratch, so the old `[10s, 5s, 2.5s]` ladder turned an 11s answer
/// into a 17.5s empty row (#1784). See [retryTransientMediaServerCall].
static const homeHubDeadline = Duration(seconds: 15);
/// Retry budget for per-library home hub rows (`/hubs/sections/{id}`). These /// Whole-request deadline for per-library home hub rows
/// can be slower than the top-level home hub call on remote Plex servers. /// (`/hubs/sections/{id}`, Jellyfin `/Items/Latest`). These can be slower
static const libraryHubAttemptTimeouts = [Duration(seconds: 10), Duration(seconds: 8), Duration(seconds: 5)]; /// than the top-level home hub call on remote servers. Same single-budget
/// rationale as [homeHubDeadline] — it replaced `[10s, 8s, 5s]`, whose 23s
/// worst case was the dominant cold-start stall in #1784.
static const libraryHubDeadline = Duration(seconds: 20);
/// Timeout for probing a cached/preferred endpoint (used in /// Timeout for probing a cached/preferred endpoint (used in
/// [PlexServer.findBestWorkingConnection]). /// [PlexServer.findBestWorkingConnection]).
+168
View File
@@ -1,3 +1,4 @@
import 'dart:async';
import 'package:drift/native.dart'; import 'package:drift/native.dart';
import 'package:material_symbols_icons/symbols.dart'; import 'package:material_symbols_icons/symbols.dart';
import 'package:plezy/media/ids.dart'; import 'package:plezy/media/ids.dart';
@@ -14,6 +15,7 @@ import 'package:plezy/media/media_backend.dart';
import 'package:plezy/media/media_hub.dart'; import 'package:plezy/media/media_hub.dart';
import 'package:plezy/media/media_item.dart'; import 'package:plezy/media/media_item.dart';
import 'package:plezy/media/media_kind.dart'; import 'package:plezy/media/media_kind.dart';
import 'package:plezy/media/media_library.dart';
import 'package:plezy/media/media_server_client.dart'; import 'package:plezy/media/media_server_client.dart';
import 'package:plezy/media/server_capabilities.dart'; import 'package:plezy/media/server_capabilities.dart';
import 'package:plezy/mixins/refreshable.dart'; import 'package:plezy/mixins/refreshable.dart';
@@ -240,6 +242,122 @@ void main() {
expect(FocusManager.instance.primaryFocus?.debugLabel, 'tv_browse_rail'); expect(FocusManager.instance.primaryFocus?.debugLabel, 'tv_browse_rail');
}); });
testWidgets('startup prime does not duplicate the load DiscoverScreen started in initState', (tester) async {
// DiscoverScreen.initState fires load(); main_screen._primeOnlineServices
// then primes the content tabs once libraries land. Asking for a full
// refresh there queued a trailing pass through CoalescedLoadCoordinator and
// ran the whole home fan-out twice on every cold start (#1784).
await SettingsService.getInstance();
tester.view.devicePixelRatio = 1.0;
tester.view.physicalSize = const Size(1280, 720);
addTearDown(() {
tester.view.resetDevicePixelRatio();
tester.view.resetPhysicalSize();
});
final hub = MediaHub(id: 'hub_1', title: 'Recommended', type: 'movie', items: const [], size: 0);
final client = _GatedHubsFakeClient(hubs: [hub]);
// Disposes both the provider and the manager it wraps; MultiServerProvider
// does not own the manager, and manager.dispose() is what closes its
// status/progress controllers and the registered clients.
final multiServerProvider = testMultiServer(clients: [client]).provider;
final hiddenLibrariesProvider = HiddenLibrariesProvider();
final librariesProvider = LibrariesProvider();
final watchTogetherProvider = WatchTogetherProvider();
final companionRemoteProvider = CompanionRemoteProvider();
final db = AppDatabase.forTesting(NativeDatabase.memory());
final connectionRegistry = _FakeConnectionRegistry(db);
final profileConnectionRegistry = _FakeProfileConnectionRegistry(db);
final storage = await StorageService.getInstance();
final plexHome = PlexHomeService(
connections: connectionRegistry,
profileConnections: profileConnectionRegistry,
storage: storage,
plexHomeUserFetcher: (_) async => const [],
);
final activeProfileProvider = ActiveProfileProvider(
registry: _FakeProfileRegistry(db),
plexHome: plexHome,
connections: connectionRegistry,
profileConnections: profileConnectionRegistry,
storage: storage,
);
final discoverProvider = DiscoverProvider(
multiServerProvider,
hiddenLibrariesProvider,
librariesProvider,
profileId: null,
isProfileBinding: () => false,
);
addTearDown(() async {
discoverProvider.dispose();
activeProfileProvider.dispose();
companionRemoteProvider.dispose();
watchTogetherProvider.dispose();
librariesProvider.dispose();
hiddenLibrariesProvider.dispose();
// multiServerProvider + its manager are torn down by testMultiServer.
await plexHome.dispose();
await db.close();
});
await tester.pumpWidget(
TranslationProvider(
child: MultiProvider(
providers: [
ChangeNotifierProvider<MultiServerProvider>.value(value: multiServerProvider),
ChangeNotifierProvider<HiddenLibrariesProvider>.value(value: hiddenLibrariesProvider),
ChangeNotifierProvider<LibrariesProvider>.value(value: librariesProvider),
ChangeNotifierProvider<DiscoverProvider>.value(value: discoverProvider),
ChangeNotifierProvider<WatchTogetherProvider>.value(value: watchTogetherProvider),
ChangeNotifierProvider<CompanionRemoteProvider>.value(value: companionRemoteProvider),
ChangeNotifierProvider<ActiveProfileProvider>.value(value: activeProfileProvider),
],
child: MaterialApp(
theme: monoTheme(dark: true),
home: MainScreenFocusScope(
focusSidebar: () {},
focusContent: () {},
isSidebarFocused: false,
sideNavigationWidth: SideNavigationRailState.expandedWidth,
reservedSideNavigationWidth: SideNavigationRailState.tvCollapsedWidth,
foregroundLeft: 0,
foregroundWidth: 1280,
viewportWidth: 1280,
child: const SizedBox(width: 1280, height: 720, child: DiscoverScreen()),
),
),
),
),
);
await tester.pump();
// The initState pass is in flight, waiting on the server.
expect(discoverProvider.isLoadInFlight, isTrue);
expect(client.hubCalls, 1);
final screen = tester.state(find.byType(DiscoverScreen)) as FullRefreshable;
screen.primeRefresh();
client.release();
// Bounded pumps, not pumpAndSettle: the hero carousel runs a repeating
// auto-scroll timer, so the frame loop never goes quiet.
await tester.pump();
await tester.pump();
await tester.pump();
expect(client.hubCalls, 1, reason: 'the prime rode along with the load already running');
expect(discoverProvider.isLoadInFlight, isFalse);
// A prime with nothing in flight (reconnect-from-offline) must still refetch.
screen.primeRefresh();
await tester.pump();
client.release();
await tester.pump();
await tester.pump();
expect(client.hubCalls, 2);
});
testWidgets('TV selects Continue Watching when it arrives after recommendation hubs', (tester) async { testWidgets('TV selects Continue Watching when it arrives after recommendation hubs', (tester) async {
await SettingsService.getInstance(); await SettingsService.getInstance();
@@ -540,6 +658,56 @@ class _FakeMediaServerClient implements MediaServerClient {
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
} }
/// Hub client that holds each fetch open until [release], so a test can act
/// while a Discover load pass is genuinely in flight.
class _GatedHubsFakeClient implements MediaServerClient {
_GatedHubsFakeClient({required this.hubs});
final List<MediaHub> hubs;
int hubCalls = 0;
final _gates = <Completer<List<MediaHub>>>[];
void release() {
for (final gate in _gates.where((g) => !g.isCompleted)) {
gate.complete(hubs);
}
}
@override
ServerId get serverId => ServerId('server_1');
@override
String? get serverName => 'Server';
@override
MediaBackend get backend => MediaBackend.plex;
@override
ServerCapabilities get capabilities => ServerCapabilities.plex;
@override
Future<List<MediaItem>> fetchContinueWatching({int? count = 20}) async => const [];
@override
Future<List<MediaLibrary>> fetchLibraries() async => const [];
@override
Future<List<MediaHub>> fetchGlobalHubs({int limit = defaultHubPreviewLimit, bool includePlaybackHubs = true}) {
hubCalls++;
final gate = Completer<List<MediaHub>>();
_gates.add(gate);
return gate.future;
}
/// Reached by `MultiServerManager.dispose()` via `_closeClientGracefully`.
/// Releases any gate still open so teardown cannot leave a fetch hanging.
@override
void close() => release();
@override
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}
class _FakeProfileRegistry extends ProfileRegistry { class _FakeProfileRegistry extends ProfileRegistry {
_FakeProfileRegistry(super.db); _FakeProfileRegistry(super.db);
@@ -1,3 +1,4 @@
import 'dart:async';
import 'dart:convert'; import 'dart:convert';
import 'package:plezy/media/ids.dart'; import 'package:plezy/media/ids.dart';
@@ -11,6 +12,8 @@ import 'package:plezy/exceptions/media_server_exceptions.dart';
import 'package:plezy/media/media_backend.dart'; import 'package:plezy/media/media_backend.dart';
import 'package:plezy/media/media_kind.dart'; import 'package:plezy/media/media_kind.dart';
import 'package:plezy/media/media_library.dart'; import 'package:plezy/media/media_library.dart';
import 'package:plezy/media/media_hub.dart';
import 'package:plezy/media/server_capabilities.dart';
import 'package:plezy/media/media_server_client.dart'; import 'package:plezy/media/media_server_client.dart';
import 'package:plezy/media/media_item.dart'; import 'package:plezy/media/media_item.dart';
import 'package:plezy/models/plex/plex_config.dart'; import 'package:plezy/models/plex/plex_config.dart';
@@ -82,6 +85,59 @@ class _LibrariesClient implements MediaServerClient {
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
} }
/// Per-library hub client whose fetches are held open individually, so a test
/// can observe exactly when the fan-out starts each library.
class _GatedHubsClient implements MediaServerClient {
_GatedHubsClient(this.libraries);
final List<MediaLibrary> libraries;
final started = <String>[];
final _gates = <String, Completer<List<MediaHub>>>{};
void complete(String libraryId) {
_gates[libraryId]!.complete([
MediaHub(
id: '$libraryId.recent',
title: libraryId,
type: 'mixed',
items: const [],
size: 0,
libraryId: libraryId,
),
]);
}
@override
ServerId get serverId => ServerId('server-1');
@override
String get serverName => 'Server';
@override
ServerCapabilities get capabilities => ServerCapabilities.jellyfin;
@override
Future<List<MediaLibrary>> fetchLibraries() async => libraries;
@override
Future<List<MediaHub>> fetchLibraryHubs(
String libraryId, {
String? libraryName,
int limit = defaultHubPreviewLimit,
bool includePlaybackHubs = true,
MediaKind? libraryKind,
}) {
started.add(libraryId);
return (_gates[libraryId] = Completer<List<MediaHub>>()).future;
}
@override
void close() {}
@override
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}
/// Smoke tests for the surviving cross-server aggregation surface on /// Smoke tests for the surviving cross-server aggregation surface on
/// [DataAggregationService]. Single-server passthroughs were removed in /// [DataAggregationService]. Single-server passthroughs were removed in
/// favour of `context.tryGetMediaClientForServer(...).<method>()`; what's /// favour of `context.tryGetMediaClientForServer(...).<method>()`; what's
@@ -112,6 +168,43 @@ void main() {
expect(result.libraries, isEmpty); expect(result.libraries, isEmpty);
expect(result.succeededServerIds, isEmpty); expect(result.succeededServerIds, isEmpty);
}); });
test('per-library hub fan-out refills a free slot instead of waiting for the batch', () async {
// Old shape: batches of three separated by `Future.wait`, so the 4th
// library could not start until ALL of the first three had answered and
// one slow row stalled every row queued behind it (#1784). A sliding
// window keeps the same peak concurrency with no head-of-line blocking.
final client = _GatedHubsClient([
for (var i = 1; i <= 4; i++)
MediaLibrary(
id: 'lib-$i',
backend: MediaBackend.jellyfin,
title: 'Library $i',
kind: MediaKind.movie,
serverId: ServerId('server-1'),
),
]);
manager.debugRegisterClientForTesting(client);
final pending = service.getHubsFromAllServers(useGlobalHubs: false, includePlaybackHubs: false);
await pumpEventQueue();
expect(client.started, ['lib-1', 'lib-2', 'lib-3'], reason: 'concurrency stays at 3');
// Free exactly one slot. The 4th library must take it immediately, while
// its two batch-mates are still in flight.
client.complete('lib-1');
await pumpEventQueue();
expect(client.started, ['lib-1', 'lib-2', 'lib-3', 'lib-4']);
// Finish out of order — hub order must still follow library order.
client.complete('lib-4');
client.complete('lib-3');
client.complete('lib-2');
final result = await pending;
expect(result.hubs.map((hub) => hub.libraryId), ['lib-1', 'lib-2', 'lib-3', 'lib-4']);
});
test('searchAcrossServers and getOnDeckFromAllServers return empty when no clients', () async { test('searchAcrossServers and getOnDeckFromAllServers return empty when no clients', () async {
final search = await service.searchAcrossServers('hello'); final search = await service.searchAcrossServers('hello');
@@ -944,17 +1037,20 @@ void main() {
reason: 'the home screen excludes playback-derived music rows', reason: 'the home screen excludes playback-derived music rows',
); );
// Music Latest returns album FOLDER dtos — count/user-data fields would // Music Latest returns album FOLDER dtos — count/user-data fields would
// each cost a recursive per-album COUNT query (#1552); video libraries // each cost a recursive per-album COUNT query (#1552).
// keep the full browse fields (series leaf counts).
final musicLatest = captured.singleWhere( final musicLatest = captured.singleWhere(
(uri) => uri.path == '/Users/user-1/Items/Latest' && uri.queryParameters['ParentId'] == 'music', (uri) => uri.path == '/Users/user-1/Items/Latest' && uri.queryParameters['ParentId'] == 'music',
); );
expect(musicLatest.queryParameters['Fields'], 'PremiereDate,OriginalTitle,SortName'); expect(musicLatest.queryParameters['Fields'], 'PremiereDate,OriginalTitle,SortName');
expect(musicLatest.queryParameters['EnableUserData'], 'false'); expect(musicLatest.queryParameters['EnableUserData'], 'false');
// Video Latest rows carry the same risk: `/Items/Latest` groups a TV
// library by series, so the rows are Series FOLDER dtos and the count
// fields cost a per-row COUNT each (#1784). Watch state survives via
// UserData.UnplayedItemCount, so the hub row set asks for neither.
final movieLatest = captured.singleWhere( final movieLatest = captured.singleWhere(
(uri) => uri.path == '/Users/user-1/Items/Latest' && uri.queryParameters['ParentId'] == 'movies', (uri) => uri.path == '/Users/user-1/Items/Latest' && uri.queryParameters['ParentId'] == 'movies',
); );
expect(movieLatest.queryParameters['Fields'], contains('RecursiveItemCount')); expect(movieLatest.queryParameters['Fields'], 'Overview');
expect(movieLatest.queryParameters.containsKey('EnableUserData'), isFalse); expect(movieLatest.queryParameters.containsKey('EnableUserData'), isFalse);
}); });
@@ -311,7 +311,33 @@ void main() {
expect(client.connection.baseUrl, 'https://primary.example.com'); expect(client.connection.baseUrl, 'https://primary.example.com');
}); });
test('hub surfaces retry transient failures without hopping endpoints', () async { test('hub surfaces do not replay a timeout, and do not hop endpoints', () async {
// `Client.send` resolves on response headers, so a hub row that times out
// is usually a server still working on an expensive query. Replaying it
// makes the server start over — the 23s cold-start stall in #1784.
final attemptsByPath = <String, int>{};
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');
attemptsByPath.update(req.url.path, (n) => n + 1, ifAbsent: () => 1);
throw TimeoutException('slow row');
}),
);
addTearDown(client.close);
await expectLater(client.fetchContinueWatching(), throwsA(isA<MediaServerHttpException>()));
expect(attemptsByPath.values, everyElement(1));
expect(client.connection.baseUrl, 'https://primary.example.com');
});
test('hub surfaces retry an immediate connection error without hopping endpoints', () async {
// A refused/reset socket costs the server nothing and is often a one-off,
// so unlike a timeout it is worth asking again on the same endpoint.
final attemptsByPath = <String, int>{}; final attemptsByPath = <String, int>{};
final client = JellyfinClient.forTesting( final client = JellyfinClient.forTesting(
connection: _conn( connection: _conn(
@@ -321,7 +347,7 @@ void main() {
httpClient: MockClient((req) async { httpClient: MockClient((req) async {
expect(req.url.host, 'primary.example.com', reason: 'retry-wrapped hub fetches must not fail over'); 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); final attempt = attemptsByPath.update(req.url.path, (n) => n + 1, ifAbsent: () => 1);
if (attempt == 1) throw TimeoutException('slow row'); if (attempt == 1) throw http.ClientException('connection reset', req.url);
return http.Response(jsonEncode({'Items': []}), 200, headers: {'content-type': 'application/json'}); return http.Response(jsonEncode({'Items': []}), 200, headers: {'content-type': 'application/json'});
}), }),
); );
+60 -26
View File
@@ -102,6 +102,38 @@ void main() {
tearDown(() { tearDown(() {
client.close(); client.close();
}); });
test('concurrent fetchLibraries calls share one /Views request', () async {
// At cold start LibrariesProvider.loadLibraries() and
// DataAggregationService.getHubsFromAllServers ask for libraries at the
// same time, and the hub fan-out waits serially behind its copy. Two
// identical round trips was a full RTT of dead cold-start latency (#1784).
var views = 0;
final scoped = JellyfinClient.forTesting(
connection: _conn(),
httpClient: MockClient((req) async {
if (req.url.path == '/Users/user-1/Views') views++;
return http.Response(
jsonEncode({
'Items': [
{'Id': 'lib-1', 'Name': 'Movies', 'CollectionType': 'movies'},
],
}),
200,
headers: {'content-type': 'application/json'},
);
}),
);
addTearDown(scoped.close);
final results = await Future.wait([scoped.fetchLibraries(), scoped.fetchLibraries()]);
expect(views, 1);
expect(results.map((libs) => libs.single.id), ['lib-1', 'lib-1']);
// Single-flight, not a cache: a later pass still sees server-side changes.
await scoped.fetchLibraries();
expect(views, 2);
});
test('buildDirectStreamUrl includes static flag, api_key, and device id', () { test('buildDirectStreamUrl includes static flag, api_key, and device id', () {
final url = client.buildDirectStreamUrl('item-99'); final url = client.buildDirectStreamUrl('item-99');
@@ -264,7 +296,7 @@ void main() {
'/Items/$encodedItemId/SpecialFeatures', '/Items/$encodedItemId/SpecialFeatures',
}); });
expect(requests.every((uri) => uri.queryParameters['userId'] == 'user-1'), isTrue); expect(requests.every((uri) => uri.queryParameters['userId'] == 'user-1'), isTrue);
expect(requests.every((uri) => uri.queryParameters['EnableImageTypes'] == 'Primary,Backdrop,Thumb,Logo'), isTrue); expect(requests.every((uri) => uri.queryParameters['EnableImageTypes'] == 'Primary,Backdrop,Logo'), isTrue);
expect(requests.every((uri) => uri.queryParameters['ImageTypeLimit'] == '3'), isTrue); expect(requests.every((uri) => uri.queryParameters['ImageTypeLimit'] == '3'), isTrue);
expect(extras.map((item) => item.id).toList(), ['trailer-1', 'featurette-1']); expect(extras.map((item) => item.id).toList(), ['trailer-1', 'featurette-1']);
expect(extras.every((item) => item.kind.isVideo), isTrue); expect(extras.every((item) => item.kind.isVideo), isTrue);
@@ -2291,7 +2323,7 @@ void main() {
expect(captured!.queryParameters['EnableTotalRecordCount'], 'true'); expect(captured!.queryParameters['EnableTotalRecordCount'], 'true');
expect(captured!.queryParameters['IncludeItemTypes'], 'Movie'); expect(captured!.queryParameters['IncludeItemTypes'], 'Movie');
expect(captured!.queryParameters['Fields'], isNot(contains('MediaSources'))); expect(captured!.queryParameters['Fields'], isNot(contains('MediaSources')));
expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(captured!.queryParameters['ImageTypeLimit'], '3'); expect(captured!.queryParameters['ImageTypeLimit'], '3');
}); });
@@ -2617,7 +2649,7 @@ void main() {
expect(captured!.queryParameters['SortBy'], 'PremiereDate,ProductionYear,SortName'); expect(captured!.queryParameters['SortBy'], 'PremiereDate,ProductionYear,SortName');
expect(captured!.queryParameters['SortOrder'], 'Descending,Descending,Ascending'); expect(captured!.queryParameters['SortOrder'], 'Descending,Descending,Ascending');
expect(captured!.queryParameters['CollapseBoxSetItems'], 'false'); expect(captured!.queryParameters['CollapseBoxSetItems'], 'false');
expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(captured!.queryParameters['ImageTypeLimit'], '3'); expect(captured!.queryParameters['ImageTypeLimit'], '3');
}); });
@@ -2643,7 +2675,7 @@ void main() {
expect(capturedNextUp, isNotNull); expect(capturedNextUp, isNotNull);
expect(capturedNextUp!.queryParameters['seriesId'], 'show-1'); expect(capturedNextUp!.queryParameters['seriesId'], 'show-1');
expect(capturedNextUp!.queryParameters['Limit'], '1'); expect(capturedNextUp!.queryParameters['Limit'], '1');
expect(capturedNextUp!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(capturedNextUp!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(capturedNextUp!.queryParameters['ImageTypeLimit'], '3'); expect(capturedNextUp!.queryParameters['ImageTypeLimit'], '3');
expect(capturedNextUp!.queryParameters.containsKey('EnableResumable'), isFalse); expect(capturedNextUp!.queryParameters.containsKey('EnableResumable'), isFalse);
expect(capturedNextUp!.queryParameters.containsKey('NextUpDateCutoff'), isFalse); expect(capturedNextUp!.queryParameters.containsKey('NextUpDateCutoff'), isFalse);
@@ -2747,16 +2779,21 @@ void main() {
expect(resume.queryParameters['MediaTypes'], 'Video'); expect(resume.queryParameters['MediaTypes'], 'Video');
expect(resume.queryParameters['Recursive'], 'true'); expect(resume.queryParameters['Recursive'], 'true');
expect(resume.queryParameters['EnableTotalRecordCount'], 'false'); expect(resume.queryParameters['EnableTotalRecordCount'], 'false');
expect(resume.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(resume.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(resume.queryParameters['ImageTypeLimit'], '3'); expect(resume.queryParameters['ImageTypeLimit'], '3');
final nextUp = requests.singleWhere((uri) => uri.path == '/Shows/NextUp'); final nextUp = requests.singleWhere((uri) => uri.path == '/Shows/NextUp');
expect(nextUp.queryParameters['userId'], 'user-1'); expect(nextUp.queryParameters['userId'], 'user-1');
expect(nextUp.queryParameters['Limit'], '3'); expect(nextUp.queryParameters['Limit'], '3');
expect(nextUp.queryParameters['EnableResumable'], 'false'); expect(nextUp.queryParameters['EnableResumable'], 'false');
expect(nextUp.queryParameters['EnableTotalRecordCount'], 'false'); expect(nextUp.queryParameters['EnableTotalRecordCount'], 'false');
expect(nextUp.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(nextUp.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(nextUp.queryParameters['ImageTypeLimit'], '3'); expect(nextUp.queryParameters['ImageTypeLimit'], '3');
expect(nextUp.queryParameters.containsKey('NextUpDateCutoff'), isFalse); // Bounds the server's unbounded GetNextUpSeriesKeys scan (#1784).
expect(
DateTime.parse(nextUp.queryParameters['NextUpDateCutoff']!),
isNot(null),
reason: 'must be a parseable ISO-8601 instant',
);
}); });
test('fetchContinueWatching orders a recently watched series Next Up above an older resume item', () async { test('fetchContinueWatching orders a recently watched series Next Up above an older resume item', () async {
@@ -3226,7 +3263,7 @@ void main() {
expect(hubs.single.more, isTrue); expect(hubs.single.more, isTrue);
}); });
test('global Next Up excludes resumable episodes without date cutoff', () async { test('global Next Up excludes resumable episodes and bounds the server scan with a date cutoff', () async {
final client = buildClient(); final client = buildClient();
addTearDown(client.close); addTearDown(client.close);
@@ -3240,9 +3277,9 @@ void main() {
expect(nextUp.queryParameters['Limit'], '12'); expect(nextUp.queryParameters['Limit'], '12');
expect(nextUp.queryParameters['EnableResumable'], 'false'); expect(nextUp.queryParameters['EnableResumable'], 'false');
expect(nextUp.queryParameters['EnableTotalRecordCount'], 'false'); expect(nextUp.queryParameters['EnableTotalRecordCount'], 'false');
expect(nextUp.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(nextUp.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(nextUp.queryParameters['ImageTypeLimit'], '3'); expect(nextUp.queryParameters['ImageTypeLimit'], '3');
expect(nextUp.queryParameters.containsKey('NextUpDateCutoff'), isFalse); expect(nextUp.queryParameters['NextUpDateCutoff'], isNotNull, reason: 'bounds the server-side scan (#1784)');
}); });
test('can skip global playback hubs', () async { test('can skip global playback hubs', () async {
@@ -3269,7 +3306,7 @@ void main() {
return JellyfinClient.forTesting(connection: _conn(), httpClient: mock); return JellyfinClient.forTesting(connection: _conn(), httpClient: mock);
} }
test('show library Next Up excludes resumable episodes without date cutoff', () async { test('show library Next Up excludes resumable episodes and bounds the server scan with a date cutoff', () async {
final client = buildClient(); final client = buildClient();
addTearDown(client.close); addTearDown(client.close);
@@ -3281,9 +3318,9 @@ void main() {
expect(nextUp.queryParameters['Limit'], '12'); expect(nextUp.queryParameters['Limit'], '12');
expect(nextUp.queryParameters['EnableResumable'], 'false'); expect(nextUp.queryParameters['EnableResumable'], 'false');
expect(nextUp.queryParameters['EnableTotalRecordCount'], 'false'); expect(nextUp.queryParameters['EnableTotalRecordCount'], 'false');
expect(nextUp.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(nextUp.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(nextUp.queryParameters['ImageTypeLimit'], '3'); expect(nextUp.queryParameters['ImageTypeLimit'], '3');
expect(nextUp.queryParameters.containsKey('NextUpDateCutoff'), isFalse); expect(nextUp.queryParameters['NextUpDateCutoff'], isNotNull, reason: 'bounds the server-side scan (#1784)');
}); });
test('movie library skips Next Up and disables resume total count', () async { test('movie library skips Next Up and disables resume total count', () async {
@@ -3337,7 +3374,7 @@ void main() {
expect(captured!.queryParameters['IncludeItemTypes'], 'Movie,Series,Episode,Video,MusicVideo,Photo'); expect(captured!.queryParameters['IncludeItemTypes'], 'Movie,Series,Episode,Video,MusicVideo,Photo');
expect(captured!.queryParameters['SortBy'], 'DateCreated,SortName,ProductionYear'); expect(captured!.queryParameters['SortBy'], 'DateCreated,SortName,ProductionYear');
expect(captured!.queryParameters['SortOrder'], 'Descending,Descending,Descending'); expect(captured!.queryParameters['SortOrder'], 'Descending,Descending,Descending');
expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(captured!.queryParameters['ImageTypeLimit'], '3'); expect(captured!.queryParameters['ImageTypeLimit'], '3');
expect(captured!.queryParameters.containsKey('ParentId'), isFalse); expect(captured!.queryParameters.containsKey('ParentId'), isFalse);
client.close(); client.close();
@@ -3355,7 +3392,7 @@ void main() {
expect(captured!.queryParameters['MediaTypes'], 'Video'); expect(captured!.queryParameters['MediaTypes'], 'Video');
expect(captured!.queryParameters['Recursive'], 'true'); expect(captured!.queryParameters['Recursive'], 'true');
expect(captured!.queryParameters['EnableTotalRecordCount'], 'true'); expect(captured!.queryParameters['EnableTotalRecordCount'], 'true');
expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(captured!.queryParameters['ImageTypeLimit'], '3'); expect(captured!.queryParameters['ImageTypeLimit'], '3');
expect(captured!.queryParameters.containsKey('ParentId'), isFalse); expect(captured!.queryParameters.containsKey('ParentId'), isFalse);
client.close(); client.close();
@@ -3373,9 +3410,9 @@ void main() {
expect(captured!.queryParameters.containsKey('ParentId'), isFalse); expect(captured!.queryParameters.containsKey('ParentId'), isFalse);
expect(captured!.queryParameters['EnableResumable'], 'false'); expect(captured!.queryParameters['EnableResumable'], 'false');
expect(captured!.queryParameters['EnableTotalRecordCount'], 'true'); expect(captured!.queryParameters['EnableTotalRecordCount'], 'true');
expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(captured!.queryParameters['ImageTypeLimit'], '3'); expect(captured!.queryParameters['ImageTypeLimit'], '3');
expect(captured!.queryParameters.containsKey('NextUpDateCutoff'), isFalse); expect(captured!.queryParameters['NextUpDateCutoff'], isNotNull, reason: 'bounds the server-side scan (#1784)');
client.close(); client.close();
}); });
@@ -3392,7 +3429,7 @@ void main() {
expect(captured!.queryParameters['Recursive'], 'true'); expect(captured!.queryParameters['Recursive'], 'true');
expect(captured!.queryParameters['EnableTotalRecordCount'], 'true'); expect(captured!.queryParameters['EnableTotalRecordCount'], 'true');
expect(captured!.queryParameters['IncludeItemTypes'], 'Movie,Series,Episode,Video,MusicVideo,Photo'); expect(captured!.queryParameters['IncludeItemTypes'], 'Movie,Series,Episode,Video,MusicVideo,Photo');
expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(captured!.queryParameters['ImageTypeLimit'], '3'); expect(captured!.queryParameters['ImageTypeLimit'], '3');
client.close(); client.close();
}); });
@@ -3408,7 +3445,7 @@ void main() {
expect(captured!.queryParameters['StartIndex'], '0'); expect(captured!.queryParameters['StartIndex'], '0');
expect(captured!.queryParameters['Recursive'], 'true'); expect(captured!.queryParameters['Recursive'], 'true');
expect(captured!.queryParameters['EnableTotalRecordCount'], 'true'); expect(captured!.queryParameters['EnableTotalRecordCount'], 'true');
expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(captured!.queryParameters['ImageTypeLimit'], '3'); expect(captured!.queryParameters['ImageTypeLimit'], '3');
client.close(); client.close();
}); });
@@ -3424,9 +3461,9 @@ void main() {
expect(captured!.queryParameters['StartIndex'], '0'); expect(captured!.queryParameters['StartIndex'], '0');
expect(captured!.queryParameters['EnableResumable'], 'false'); expect(captured!.queryParameters['EnableResumable'], 'false');
expect(captured!.queryParameters['EnableTotalRecordCount'], 'true'); expect(captured!.queryParameters['EnableTotalRecordCount'], 'true');
expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(captured!.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(captured!.queryParameters['ImageTypeLimit'], '3'); expect(captured!.queryParameters['ImageTypeLimit'], '3');
expect(captured!.queryParameters.containsKey('NextUpDateCutoff'), isFalse); expect(captured!.queryParameters['NextUpDateCutoff'], isNotNull, reason: 'bounds the server-side scan (#1784)');
client.close(); client.close();
}); });
@@ -3602,12 +3639,9 @@ void main() {
expect(itemsRequest.queryParameters['Limit'], '36'); expect(itemsRequest.queryParameters['Limit'], '36');
expect(itemsRequest.queryParameters['SortBy'], 'SortName'); expect(itemsRequest.queryParameters['SortBy'], 'SortName');
expect(itemsRequest.queryParameters['SortOrder'], 'Ascending'); expect(itemsRequest.queryParameters['SortOrder'], 'Ascending');
expect( expect(itemsRequest.queryParameters['Fields'], 'RecursiveItemCount,ChildCount,OriginalTitle,SortName,Overview');
itemsRequest.queryParameters['Fields'],
'RecursiveItemCount,ChildCount,UserData,PremiereDate,OriginalTitle,SortName,Overview',
);
expect(itemsRequest.queryParameters.containsKey('EnableTotalRecordCount'), isFalse); expect(itemsRequest.queryParameters.containsKey('EnableTotalRecordCount'), isFalse);
expect(itemsRequest.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(itemsRequest.queryParameters['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(itemsRequest.queryParameters['ImageTypeLimit'], '3'); expect(itemsRequest.queryParameters['ImageTypeLimit'], '3');
}); });
@@ -89,7 +89,7 @@ void main() {
expect(params['Fields'], 'UserData'); expect(params['Fields'], 'UserData');
expect(params['IncludeItemTypes'], isNotEmpty); expect(params['IncludeItemTypes'], isNotEmpty);
expect(params['EnableTotalRecordCount'], 'true'); expect(params['EnableTotalRecordCount'], 'true');
expect(params['EnableImageTypes'], 'Primary,Backdrop,Thumb,Logo'); expect(params['EnableImageTypes'], 'Primary,Backdrop,Logo');
expect(params['ImageTypeLimit'], '3'); expect(params['ImageTypeLimit'], '3');
}); });
+38 -6
View File
@@ -62,13 +62,13 @@ void main() {
}); });
group('PlexClient home hub retries', () { group('PlexClient home hub retries', () {
test('fetchGlobalHubs retries a transient first failure', () async { test('fetchGlobalHubs retries a first-attempt connection error', () async {
final db = AppDatabase.forTesting(NativeDatabase.memory()); final db = AppDatabase.forTesting(NativeDatabase.memory());
PlexApiCache.initialize(db); PlexApiCache.initialize(db);
addTearDown(db.close); addTearDown(db.close);
final httpClient = _SequenceClient([ final httpClient = _SequenceClient([
(_) async => throw TimeoutException('cold Plex start'), (_) async => throw http.ClientException('connection reset on cold Plex start'),
(_) async => _jsonResponse(_globalHubsPayload()), (_) async => _jsonResponse(_globalHubsPayload()),
]); ]);
final client = PlexClient.forTesting( final client = PlexClient.forTesting(
@@ -96,6 +96,38 @@ void main() {
expect(httpClient.requests.map((r) => r.url.queryParameters['count']), everyElement('12')); expect(httpClient.requests.map((r) => r.url.queryParameters['count']), everyElement('12'));
}); });
test('fetchGlobalHubs does not replay a hub row that timed out', () async {
// `Client.send` resolves on response headers, so a hub timeout usually
// means the server is still working on the query. Replaying it made the
// server start over on a shorter budget — the #1784 cold-start stall.
final db = AppDatabase.forTesting(NativeDatabase.memory());
PlexApiCache.initialize(db);
addTearDown(db.close);
final httpClient = _SequenceClient([
(_) async => throw TimeoutException('server still building the hub'),
(_) async => _jsonResponse(_globalHubsPayload()),
]);
final client = PlexClient.forTesting(
config: PlexConfig(
baseUrl: 'http://server:32400',
token: 'token',
clientIdentifier: 'client-id',
product: 'Plezy',
version: 'test',
),
serverId: ServerId('server-id'),
profileScopeId: buildPlexProfileScopeId(serverId: ServerId('server-id'), profileId: 'test-profile'),
serverName: 'Server',
httpClient: httpClient,
);
addTearDown(client.close);
// `_fetchHubs` degrades a failed row to empty rather than sinking home.
expect(await client.fetchGlobalHubs(limit: 12), isEmpty);
expect(httpClient.requests, hasLength(1));
});
test('fetchGlobalHubs sends configured Plex language headers', () async { test('fetchGlobalHubs sends configured Plex language headers', () async {
final db = AppDatabase.forTesting(NativeDatabase.memory()); final db = AppDatabase.forTesting(NativeDatabase.memory());
PlexApiCache.initialize(db); PlexApiCache.initialize(db);
@@ -159,7 +191,7 @@ void main() {
expect(httpClient.requests[1].headers['X-Plex-Language'], 'fr'); expect(httpClient.requests[1].headers['X-Plex-Language'], 'fr');
}); });
test('fetchGlobalHubs retries transient failures without switching Plex endpoints', () async { test('fetchGlobalHubs retries a connection error without switching Plex endpoints', () async {
final db = AppDatabase.forTesting(NativeDatabase.memory()); final db = AppDatabase.forTesting(NativeDatabase.memory());
PlexApiCache.initialize(db); PlexApiCache.initialize(db);
addTearDown(db.close); addTearDown(db.close);
@@ -167,7 +199,7 @@ void main() {
const primary = 'http://primary:32400'; const primary = 'http://primary:32400';
const fallback = 'http://fallback:32400'; const fallback = 'http://fallback:32400';
final httpClient = _SequenceClient([ final httpClient = _SequenceClient([
(_) async => throw TimeoutException('queued behind cold handshakes'), (_) async => throw http.ClientException('connection reset'),
(_) async => _jsonResponse(_globalHubsPayload()), (_) async => _jsonResponse(_globalHubsPayload()),
]); ]);
final client = PlexClient.forTesting( final client = PlexClient.forTesting(
@@ -327,7 +359,7 @@ void main() {
expect(httpClient.requests.single.url.queryParameters['includeGuids'], '1'); expect(httpClient.requests.single.url.queryParameters['includeGuids'], '1');
}); });
test('fetchLibraryHubs retries transient failures without switching Plex endpoints', () async { test('fetchLibraryHubs retries a connection error without switching Plex endpoints', () async {
final db = AppDatabase.forTesting(NativeDatabase.memory()); final db = AppDatabase.forTesting(NativeDatabase.memory());
PlexApiCache.initialize(db); PlexApiCache.initialize(db);
addTearDown(db.close); addTearDown(db.close);
@@ -335,7 +367,7 @@ void main() {
const primary = 'http://primary:32400'; const primary = 'http://primary:32400';
const fallback = 'http://fallback:32400'; const fallback = 'http://fallback:32400';
final httpClient = _SequenceClient([ final httpClient = _SequenceClient([
(_) async => throw TimeoutException('queued behind image downloads'), (_) async => throw http.ClientException('connection reset'),
(_) async => _jsonResponse(_globalHubsPayload()), (_) async => _jsonResponse(_globalHubsPayload()),
]); ]);
final client = PlexClient.forTesting( final client = PlexClient.forTesting(
+147 -16
View File
@@ -1,34 +1,165 @@
import 'package:fake_async/fake_async.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:plezy/exceptions/media_server_exceptions.dart'; import 'package:plezy/exceptions/media_server_exceptions.dart';
import 'package:plezy/utils/media_server_http_client.dart'; import 'package:plezy/utils/media_server_http_client.dart';
import 'package:plezy/utils/media_server_retry.dart'; import 'package:plezy/utils/media_server_retry.dart';
MediaServerHttpException _error(MediaServerHttpErrorType type) =>
MediaServerHttpException(type: type, message: type.name);
void main() { void main() {
group('retryTransientMediaServerCall', () { group('retryTransientMediaServerCall', () {
test('retries transient failures in timeout order', () async { test('runs the call once with the whole deadline as its budget', () async {
const timeouts = [Duration(seconds: 10), Duration(seconds: 5), Duration(milliseconds: 2500)];
final seenTimeouts = <Duration>[]; final seenTimeouts = <Duration>[];
final aborts = <AbortController>[];
final result = await retryTransientMediaServerCall<String>( final result = await retryTransientMediaServerCall<String>(
operation: 'test operation', operation: 'test operation',
attemptTimeouts: timeouts, deadline: const Duration(seconds: 20),
call: (timeout, abort) async { call: (timeout, _) async {
seenTimeouts.add(timeout); seenTimeouts.add(timeout);
aborts.add(abort);
if (seenTimeouts.length < 3) {
throw MediaServerHttpException(type: MediaServerHttpErrorType.connectionTimeout, message: 'timed out');
}
return 'ok'; return 'ok';
}, },
); );
expect(result, 'ok'); expect(result, 'ok');
expect(seenTimeouts, timeouts); expect(seenTimeouts, [const Duration(seconds: 20)]);
});
// The regression this whole policy exists for: `http.Client.send` resolves
// on response HEADERS, so a slow-but-alive server surfaces as
// connectionTimeout. Replaying it made the server re-run the same query and
// turned an 11s answer into an empty row after 23s (#1784).
test('does not replay a timeout — the server is working, just slow', () async {
var attempts = 0;
await expectLater(
retryTransientMediaServerCall<void>(
operation: 'test operation',
deadline: const Duration(seconds: 20),
call: (_, _) async {
attempts++;
throw _error(MediaServerHttpErrorType.connectionTimeout);
},
),
throwsA(
isA<MediaServerHttpException>().having((e) => e.type, 'type', MediaServerHttpErrorType.connectionTimeout),
),
);
expect(attempts, 1);
});
test('a slow response that lands inside the deadline resolves, and is requested once', () {
fakeAsync((async) {
var attempts = 0;
Object? result;
// Answers at T+11s, well past the old 10s first-attempt budget.
retryTransientMediaServerCall<String>(
operation: 'test operation',
deadline: const Duration(seconds: 20),
call: (_, _) async {
attempts++;
await Future<void>.delayed(const Duration(seconds: 11));
return 'late but fine';
},
).then((value) => result = value);
async.elapse(const Duration(seconds: 10, milliseconds: 900));
expect(result, isNull, reason: 'still in flight');
expect(attempts, 1);
async.elapse(const Duration(milliseconds: 200));
expect(result, 'late but fine');
expect(attempts, 1, reason: 'never replayed');
});
});
test('retries an immediate connection error', () {
fakeAsync((async) {
final aborts = <AbortController>[];
Object? result;
retryTransientMediaServerCall<String>(
operation: 'test operation',
deadline: const Duration(seconds: 20),
call: (_, abort) async {
aborts.add(abort);
await Future<void>.delayed(const Duration(seconds: 1));
if (aborts.length < 3) throw _error(MediaServerHttpErrorType.connectionError);
return 'ok';
},
).then((value) => result = value);
async.elapse(const Duration(seconds: 5));
expect(result, 'ok');
expect(aborts, hasLength(3));
expect(aborts[0].isAborted, isTrue); expect(aborts[0].isAborted, isTrue);
expect(aborts[1].isAborted, isTrue); expect(aborts[1].isAborted, isTrue);
expect(aborts[2].isAborted, isFalse); expect(aborts[2].isAborted, isFalse);
}); });
});
test('gives up at the deadline and aborts the in-flight request', () {
fakeAsync((async) {
var attempts = 0;
AbortController? last;
Duration? settledAt;
Object? error;
retryTransientMediaServerCall<void>(
operation: 'test operation',
deadline: const Duration(seconds: 10),
call: (_, abort) async {
attempts++;
last = abort;
// Never answers: the request the deadline has to cut off.
await Future<void>.delayed(const Duration(days: 1));
},
).catchError((Object e) {
error = e;
settledAt = async.elapsed;
});
async.elapse(const Duration(seconds: 30));
expect(attempts, 1, reason: 'a timeout is never replayed');
expect(settledAt, const Duration(seconds: 10));
expect(last?.isAborted, isTrue, reason: 'the in-flight request is torn down');
expect(
error,
isA<MediaServerHttpException>().having((e) => e.type, 'type', MediaServerHttpErrorType.connectionTimeout),
);
});
});
test('bounds total wall time by the deadline, not by attempts × timeout', () {
fakeAsync((async) {
Duration? settledAt;
Object? error;
retryTransientMediaServerCall<void>(
operation: 'test operation',
deadline: const Duration(seconds: 15),
call: (_, _) async {
// Every attempt burns most of the budget before failing in a
// retryable way — the worst case for a retry loop.
await Future<void>.delayed(const Duration(seconds: 6));
throw _error(MediaServerHttpErrorType.connectionError);
},
).catchError((Object e) {
error = e;
settledAt = async.elapsed;
});
async.elapse(const Duration(seconds: 120));
expect(error, isA<MediaServerHttpException>());
expect(settledAt, isNotNull);
expect(settledAt!, lessThanOrEqualTo(const Duration(seconds: 15)));
});
});
test('does not retry non-transient failures', () async { test('does not retry non-transient failures', () async {
var attempts = 0; var attempts = 0;
@@ -36,7 +167,7 @@ void main() {
await expectLater( await expectLater(
retryTransientMediaServerCall<void>( retryTransientMediaServerCall<void>(
operation: 'test operation', operation: 'test operation',
attemptTimeouts: const [Duration(seconds: 10), Duration(seconds: 5)], deadline: const Duration(seconds: 20),
call: (_, _) async { call: (_, _) async {
attempts++; attempts++;
throw MediaServerHttpException( throw MediaServerHttpException(
@@ -52,22 +183,22 @@ void main() {
expect(attempts, 1); expect(attempts, 1);
}); });
test('rethrows final transient failure after exhausting attempts', () async { test('does not swallow a cancellation as a retryable failure', () async {
var attempts = 0; var attempts = 0;
await expectLater( await expectLater(
retryTransientMediaServerCall<void>( retryTransientMediaServerCall<void>(
operation: 'test operation', operation: 'test operation',
attemptTimeouts: const [Duration(seconds: 10), Duration(seconds: 5), Duration(milliseconds: 2500)], deadline: const Duration(seconds: 20),
call: (_, _) async { call: (_, _) async {
attempts++; attempts++;
throw MediaServerHttpException(type: MediaServerHttpErrorType.receiveTimeout, message: 'receive timed out'); throw _error(MediaServerHttpErrorType.cancelled);
}, },
), ),
throwsA(isA<MediaServerHttpException>().having((e) => e.type, 'type', MediaServerHttpErrorType.receiveTimeout)), throwsA(isA<MediaServerHttpException>().having((e) => e.isCancellation, 'isCancellation', isTrue)),
); );
expect(attempts, 3); expect(attempts, 1);
}); });
}); });
} }