fix(servers): stop rebinds from flashing an empty home screen after sign-in

Signing in triggered two back-to-back profile rebinds; the second re-added
the same Jellyfin connection, which tore down the live client and aborted
the home screen's in-flight fetches. The aborted pass was committed as
loaded-empty, flashing 'no content available' until the follow-up load
landed. Fix at the root instead of patching the sign-in window:

- addJellyfinConnection now reuses the live client when the connection is
  unchanged (token, deviceId, URL set), matching the existing Plex
  refreshTokensForProfile behavior; material changes still recreate it.
- Cancelled requests are classified end-to-end: the client's
  treat-as-empty helpers rethrow cancellations, and the aggregation
  fan-outs report cancelledServerIds alongside succeededServerIds.
- A fetch pass in which zero servers succeeded is never authoritative:
  it keeps existing content instead of wiping it (also fixes the
  pre-existing blanking of home/sidebar on a totally failed refresh),
  stays in loading while disrupted (cancellation or binding in flight),
  and only commits loaded-empty on a settled failure.
This commit is contained in:
edde746
2026-07-04 23:44:38 +02:00
parent c8ca8a7875
commit 87aea49ab6
12 changed files with 648 additions and 30 deletions
@@ -92,6 +92,12 @@ class MediaServerHttpException extends MediaServerException {
type == MediaServerHttpErrorType.connectionError ||
type == MediaServerHttpErrorType.receiveTimeout;
/// Whether the request was aborted client-side (client teardown or an
/// explicit abort), as opposed to failing against the server. A cancelled
/// fetch says nothing about the server's actual content — callers must not
/// treat it as an empty result.
bool get isCancellation => type == MediaServerHttpErrorType.cancelled;
@override
String toString() {
final parts = <String>[type.name];
+8 -4
View File
@@ -144,10 +144,14 @@ class _ProfileSessionScreenState extends State<ProfileSessionScreen> {
lazy: true,
),
ChangeNotifierProvider(
create: (context) => LibrariesProvider(
storageService: context.read<StorageService>(),
multiServer: context.read<MultiServerProvider>(),
),
create: (context) {
final activeProfile = context.read<ActiveProfileProvider>();
return LibrariesProvider(
storageService: context.read<StorageService>(),
multiServer: context.read<MultiServerProvider>(),
isProfileBinding: () => activeProfile.isBinding,
);
},
),
ChangeNotifierProvider(
create: (context) {
+44 -8
View File
@@ -61,8 +61,10 @@ class DiscoverProvider extends ChangeNotifier with DisposableChangeNotifierMixin
final LibrariesProvider _libraries;
/// Whether the profile binder is still wiring servers — a no-servers load
/// during binding stays in the loading state instead of flashing an error
/// (main_screen primes another load once binding settles).
/// during binding stays in the loading state instead of flashing an error,
/// and a zero-success pass during binding stays in the loading state
/// instead of flashing the empty placeholder (main_screen primes another
/// load once binding settles).
final bool Function() isProfileBinding;
StreamSubscription<WatchStateEvent>? _watchStateSubscription;
@@ -198,18 +200,52 @@ class DiscoverProvider extends ChangeNotifier with DisposableChangeNotifierMixin
includePlaybackHubs: false,
);
// A pass in which zero servers succeeded is never authoritative: it
// must not wipe existing content, and it may only commit "loaded,
// empty" when the failure is settled — not a client-side abort
// (teardown mid-fetch) and not mid-binding. In both of those cases a
// follow-up load is guaranteed (binding-settle prime, or
// syncToOnlineServers falling through to load() while not loaded).
final fetchedOnDeck = await onDeckFuture;
if (isDisposed) return;
_applyOnDeck(fetchedOnDeck.items);
_onDeckState = DiscoverLoadState.loaded;
_loadedOnDeckServerIds = fetchedOnDeck.succeededServerIds;
_loadGeneration++;
safeNotifyListeners();
unawaited(_syncSystemShelf(_onDeck));
if (fetchedOnDeck.succeededServerIds.isEmpty && _onDeck.isNotEmpty) {
// Keep the stale rows; the empty succeeded set makes the next status
// emission refetch every server.
appLogger.w('DiscoverProvider: on-deck pass failed on all servers; keeping previous items');
_onDeckState = DiscoverLoadState.loaded;
_loadedOnDeckServerIds = fetchedOnDeck.succeededServerIds;
safeNotifyListeners();
} else if (fetchedOnDeck.succeededServerIds.isEmpty &&
(fetchedOnDeck.cancelledServerIds.isNotEmpty || isProfileBinding())) {
// Disrupted with nothing to show yet: stay in loading so the screen
// keeps its skeleton instead of flashing the empty placeholder.
// Don't return — the hubs fetch is still in flight below.
appLogger.d('DiscoverProvider: on-deck pass disrupted with no prior content; keeping loading state');
} else {
_applyOnDeck(fetchedOnDeck.items);
_onDeckState = DiscoverLoadState.loaded;
_loadedOnDeckServerIds = fetchedOnDeck.succeededServerIds;
_loadGeneration++;
safeNotifyListeners();
unawaited(_syncSystemShelf(_onDeck));
}
final fetchedHubs = await hubsFuture;
if (isDisposed) return;
if (fetchedHubs.succeededServerIds.isEmpty && _hubs.isNotEmpty) {
appLogger.w('DiscoverProvider: hub pass failed on all servers; keeping previous hubs');
_hubsState = DiscoverLoadState.loaded;
_loadedHubServerIds = fetchedHubs.succeededServerIds;
safeNotifyListeners();
return;
}
if (fetchedHubs.succeededServerIds.isEmpty &&
(fetchedHubs.cancelledServerIds.isNotEmpty || isProfileBinding())) {
appLogger.d('DiscoverProvider: hub pass disrupted with no prior content; keeping loading state');
return;
}
final filteredHubs = _filterDiscoverHubs(fetchedHubs.hubs);
sortMediaHubsByLibraryOrder(filteredHubs, _libraries.libraries);
+38 -3
View File
@@ -15,9 +15,13 @@ enum LibrariesLoadState { initial, loading, loaded, error }
/// Both SideNavigationRail and LibrariesScreen consume this provider
/// instead of independently fetching library data.
class LibrariesProvider extends ChangeNotifier with DisposableChangeNotifierMixin {
LibrariesProvider({StorageService? storageService, MultiServerProvider? multiServer})
: _storageService = storageService,
_multiServer = multiServer {
LibrariesProvider({
StorageService? storageService,
MultiServerProvider? multiServer,
bool Function()? isProfileBinding,
}) : _storageService = storageService,
_multiServer = multiServer,
_isProfileBinding = isProfileBinding ?? _neverBinding {
// Reload libraries when a new server comes online. Servers bind in waves
// on sign-in / profile switch and slow ones reconnect after the initial
// load; without this they stay missing from the sidebar until a re-switch
@@ -26,7 +30,16 @@ class LibrariesProvider extends ChangeNotifier with DisposableChangeNotifierMixi
_multiServer?.addOnlineServersListener(syncToOnlineServers);
}
static bool _neverBinding() => false;
final MultiServerProvider? _multiServer;
/// Whether the profile binder is still wiring servers — a zero-success
/// first load during binding stays in the loading state instead of
/// flashing "no libraries" (main_screen primes another load once binding
/// settles).
final bool Function() _isProfileBinding;
StorageService? _storageService;
DataAggregationService? _aggregationService;
List<MediaLibrary> _libraries = [];
@@ -194,6 +207,28 @@ class LibrariesProvider extends ChangeNotifier with DisposableChangeNotifierMixi
// internally; Jellyfin clients return MediaLibrary natively.
final result = await _aggregationService!.getMediaLibrariesFromAllServers();
// A pass in which zero servers succeeded is never authoritative — it
// must not replace existing data, and it may only commit "loaded,
// empty" when the failure is settled (not a client-side abort, not
// mid-binding). Recovery is guaranteed: the binding-settle prime and
// the next status emission both re-drive a load while state isn't a
// fully-covered `loaded`.
if (result.succeededServerIds.isEmpty) {
if (reloadInPlace) {
// A totally-failed silent refresh keeps the last good list instead
// of wiping the sidebar. Clear the succeeded set so the next
// status emission refetches rather than treating the stale list as
// covering those servers.
appLogger.w('LibrariesProvider: refresh failed on all servers; keeping previous libraries');
_loadedServerIds = result.succeededServerIds;
return false;
}
if (result.cancelledServerIds.isNotEmpty || _isProfileBinding()) {
appLogger.d('LibrariesProvider: first load disrupted (zero successful servers); staying in loading state');
return false;
}
}
// Filter out music libraries (not supported)
final filteredLibraries = result.libraries.where((lib) => !ContentTypeHelper.isMusicLibrary(lib)).toList();
+30 -12
View File
@@ -6,14 +6,22 @@ import '../media/media_item.dart';
import '../media/media_kind.dart';
import '../media/media_library.dart';
import '../media/media_server_client.dart';
import '../exceptions/media_server_exceptions.dart';
import '../utils/app_logger.dart';
import '../utils/external_ids.dart';
import '../utils/global_key_utils.dart';
import '../utils/search_relevance.dart';
import 'multi_server_manager.dart';
typedef OnDeckAggregationResult = ({List<MediaItem> items, Set<String> succeededServerIds});
typedef HubAggregationResult = ({List<MediaHub> hubs, Set<String> succeededServerIds});
typedef OnDeckAggregationResult = ({List<MediaItem> items, Set<String> succeededServerIds, Set<String> cancelledServerIds});
typedef HubAggregationResult = ({List<MediaHub> hubs, Set<String> succeededServerIds, Set<String> cancelledServerIds});
typedef LibraryAggregationResult = ({List<MediaLibrary> libraries, Set<String> succeededServerIds, Set<String> cancelledServerIds});
/// Whether [error] is a client-side abort (client teardown mid-request)
/// rather than a genuine server failure. Aggregation reports these servers
/// in `cancelledServerIds` so callers can tell a *disrupted* pass — whose
/// results say nothing about actual content — from a settled failure.
bool _isCancellation(Object error) => error is MediaServerHttpException && error.isCancellation;
/// Cross-server aggregation: fans calls out to every online client and
/// merges the results. Single-server operations now go through the
@@ -47,28 +55,34 @@ class DataAggregationService {
/// list. [succeededServerIds] lets callers tell a *failed* fetch apart from a
/// server that genuinely has no libraries — both contribute nothing, so
/// conflating them would let a transient failure be cached as "loaded" and
/// never retried.
Future<({List<MediaLibrary> libraries, Set<String> succeededServerIds})> getMediaLibrariesFromAllServers({
Set<String>? serverIds,
}) async {
/// never retried. Servers whose fetch was aborted client-side land in
/// `cancelledServerIds` — a disrupted pass, unlike a settled failure, must
/// never be committed as authoritative.
Future<LibraryAggregationResult> getMediaLibrariesFromAllServers({Set<String>? serverIds}) async {
final clients = _clientsFor(serverIds);
if (clients.isEmpty) {
appLogger.w('No online servers available for fetching libraries (neutral)');
return (libraries: const <MediaLibrary>[], succeededServerIds: const <String>{});
return (libraries: const <MediaLibrary>[], succeededServerIds: const <String>{}, cancelledServerIds: const <String>{});
}
final succeededServerIds = <String>{};
final cancelledServerIds = <String>{};
final futures = clients.entries.map((entry) async {
try {
final libraries = await entry.value.fetchLibraries();
succeededServerIds.add(entry.key);
return libraries;
} catch (e, stackTrace) {
if (_isCancellation(e)) cancelledServerIds.add(entry.key);
appLogger.e('Failed neutral library fetch from ${entry.key}', error: e, stackTrace: stackTrace);
return <MediaLibrary>[];
}
});
final results = await Future.wait(futures);
return (libraries: [for (final list in results) ...list], succeededServerIds: succeededServerIds);
return (
libraries: [for (final list in results) ...list],
succeededServerIds: succeededServerIds,
cancelledServerIds: cancelledServerIds,
);
}
/// Fetch "On Deck" (Continue Watching) from all servers and merge by recency.
@@ -83,15 +97,17 @@ class DataAggregationService {
final clients = _clientsFor(serverIds);
if (clients.isEmpty) {
appLogger.w('No online servers available for fetching on deck');
return (items: const <MediaItem>[], succeededServerIds: const <String>{});
return (items: const <MediaItem>[], succeededServerIds: const <String>{}, cancelledServerIds: const <String>{});
}
final cancelledServerIds = <String>{};
final futures = clients.entries.map((entry) async {
final client = entry.value;
try {
final items = await client.fetchContinueWatching(count: limit);
return (serverId: entry.key, items: items);
} catch (e, st) {
if (_isCancellation(e)) cancelledServerIds.add(entry.key);
appLogger.e('Failed on-deck fetch from ${entry.key}', error: e, stackTrace: st);
return (serverId: null, items: <MediaItem>[]);
}
@@ -125,7 +141,7 @@ class DataAggregationService {
appLogger.i('Fetched ${items.length} on deck items from all servers');
return (items: items, succeededServerIds: succeededServerIds);
return (items: items, succeededServerIds: succeededServerIds, cancelledServerIds: cancelledServerIds);
}
/// Merge an [existing] Continue Watching list with [fresh] rows from
@@ -289,7 +305,7 @@ class DataAggregationService {
final clients = _clientsFor(serverIds);
if (clients.isEmpty) {
appLogger.w('No online servers available for fetching hubs');
return (hubs: const <MediaHub>[], succeededServerIds: const <String>{});
return (hubs: const <MediaHub>[], succeededServerIds: const <String>{}, cancelledServerIds: const <String>{});
}
// Only fallback clients need a library prefetch when home layout is on;
@@ -299,6 +315,7 @@ class DataAggregationService {
? _groupLibrariesByServer((await getMediaLibrariesFromAllServers(serverIds: serverIds)).libraries)
: null;
final cancelledServerIds = <String>{};
final futures = clients.entries.map((entry) async {
final serverId = entry.key;
final client = entry.value;
@@ -320,6 +337,7 @@ class DataAggregationService {
hubs: _postProcessHubs(hubs, serverId: ServerId(serverId), hiddenLibraryKeys: hiddenLibraryKeys),
);
} catch (e, stackTrace) {
if (_isCancellation(e)) cancelledServerIds.add(serverId);
appLogger.e('Failed to fetch hubs from server $serverId', error: e, stackTrace: stackTrace);
return (serverId: null, hubs: <MediaHub>[]);
}
@@ -335,7 +353,7 @@ class DataAggregationService {
all.addAll(result.hubs);
}
final hubs = limit != null && limit < all.length ? all.sublist(0, limit) : all;
return (hubs: hubs, succeededServerIds: succeededServerIds);
return (hubs: hubs, succeededServerIds: succeededServerIds, cancelledServerIds: cancelledServerIds);
}
/// Per-library hub fetch for a single client. Filters to visible
@@ -1279,6 +1279,9 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin {
final page = await fetchMoreHubItemsPage(hubId, start: 0, size: limit ?? 50);
return page.items;
} catch (e, st) {
// A cancelled request says nothing about the hub's contents — let it
// propagate so the caller classifies the fetch as disrupted, not empty.
if (e is MediaServerHttpException && e.isCancellation) rethrow;
appLogger.w('JellyfinClient: failed to fetch hub items for $hubId (treating as empty)', error: e, stackTrace: st);
return const [];
}
@@ -1598,6 +1601,10 @@ mixin _JellyfinBrowseMethods on MediaServerCacheMixin {
}
return _itemsArray(data);
} catch (e, st) {
// A cancelled request says nothing about the endpoint's contents — let
// it propagate so the caller classifies the fetch as disrupted, not
// empty.
if (e is MediaServerHttpException && e.isCancellation) rethrow;
appLogger.w('JellyfinClient: $path failed (treating as empty)', error: e, stackTrace: st);
return const [];
}
+79 -2
View File
@@ -547,6 +547,15 @@ class MultiServerManager {
/// Add a Jellyfin server backed by an authenticated [JellyfinConnection].
/// Returns true on success.
///
/// When a live client already exists for the same compound id and the
/// connection is equivalent (see [canReuseJellyfinClient]), that client is
/// reused instead of recreated — profile rebinds re-add unchanged
/// connections routinely, and tearing the client down would abort its
/// in-flight requests. A material change (token, deviceId, URL set) still
/// replaces the client. This mirrors the Plex rebind path, where
/// [refreshTokensForProfile] reuses the online client via an in-place
/// token update.
///
/// Jellyfin clients use the shared endpoint-racing flow when multiple URLs
/// are configured, then instantiate the client against the lowest-latency
/// reachable URL.
@@ -557,6 +566,13 @@ class MultiServerManager {
/// client (preserves any in-flight operations on the prior profile).
Future<bool> addJellyfinConnection(JellyfinConnection connection) async {
try {
// Every close path detaches the client from [_jellyfinByCompoundId]
// before closing it, so a client found here is never mid-close.
final existing = _jellyfinByCompoundId[connection.id];
if (existing != null && canReuseJellyfinClient(live: existing.connection, incoming: connection)) {
return _reuseJellyfinClient(existing);
}
var resolvedConnection = connection;
if (connection.baseUrls.length > 1) {
try {
@@ -591,8 +607,9 @@ class MultiServerManager {
final compoundId = resolvedConnection.id;
final machineId = resolvedConnection.serverMachineId;
// Replace any prior client for this exact compound id (re-add of the
// same user — e.g., token refresh or settings re-add).
// Replace the prior client for this compound id — reaching here means
// the connection materially changed (token refresh, URL-set edit); an
// unchanged re-add was already handled by the reuse branch above.
final oldClient = _jellyfinByCompoundId[compoundId];
if (oldClient != null) _closeClient(oldClient);
_jellyfinByCompoundId[compoundId] = client;
@@ -619,6 +636,66 @@ class MultiServerManager {
}
}
/// Whether the live client bound to [live] can serve [incoming] without
/// being recreated. Recreation is required when a field baked into the
/// client at construction time changes:
/// - `accessToken` / `deviceId` are embedded in the auth headers when the
/// HTTP client is built;
/// - `baseUrls` fixes the failover candidate set. Compared as a set: both
/// the client and the add-path endpoint race reorder the list as
/// endpoints are promoted, so ordering drifts on an unchanged server.
///
/// Everything else is deliberately ignored: the active `baseUrl` drifts as
/// the client rotates endpoints, `isAdministrator` self-refreshes on health
/// checks, and the remaining fields are display metadata. `userId` and
/// `serverMachineId` equality is implied by the compound-id lookup that
/// precedes this check.
@visibleForTesting
static bool canReuseJellyfinClient({required JellyfinConnection live, required JellyfinConnection incoming}) {
return live.accessToken == incoming.accessToken &&
live.deviceId == incoming.deviceId &&
setEquals(live.baseUrls.toSet(), incoming.baseUrls.toSet());
}
/// Re-add of an unchanged connection: keep the live client (preserving its
/// in-flight requests and settled endpoint choice), re-bind it as the
/// machine's active user, and run a fresh health probe so callers still
/// get a current result. Skips the endpoint race ([JellyfinClient] has
/// per-request failover plus exhaustion-triggered reconnect), the
/// connection-update wiring (already attached when the client was first
/// added), and the connection persist (the client persists its own
/// endpoint rotations).
Future<bool> _reuseJellyfinClient(JellyfinClient client) async {
final compoundId = client.connection.id;
final machineId = client.connection.serverMachineId;
final rebound = _activeJellyfinMachine[machineId] != compoundId;
_clients[machineId] = client;
_activeJellyfinMachine[machineId] = compoundId;
final health = await client.checkHealth();
_jellyfinHealthByCompoundId[compoundId] = health;
if (_activeJellyfinMachine[machineId] != compoundId) {
// A concurrent remove/re-add won while the probe was in flight.
appLogger.d('Ignoring stale Jellyfin reuse result for ${client.connection.serverName}');
return health == HealthStatus.online;
}
_applyHealth(ServerId(machineId), health);
if (rebound) {
// The machine's active user changed even if its online status didn't;
// client-map consumers need to observe the swap.
_statusController.add(Map.from(_serverStatus));
}
final healthy = health == HealthStatus.online;
appLogger.i(
'Reusing existing Jellyfin client for ${client.connection.serverName}'
'${healthy ? '' : ' (unhealthy)'} (connection unchanged)',
);
if (_connectivitySubscription == null && healthy) {
_startNetworkMonitoring();
}
return healthy;
}
void _wireJellyfinConnectionUpdates(JellyfinClient client) {
client.onConnectionUpdated = (updated) async {
if (_jellyfinByCompoundId[updated.id] != client) {
+114 -1
View File
@@ -57,6 +57,8 @@ class _FakeAggregationService extends DataAggregationService {
Set<String>? lastHubsServerIds;
Set<String>? onDeckSucceededServerIds;
Set<String>? hubSucceededServerIds;
Set<String> onDeckCancelledServerIds = const {};
Set<String> hubCancelledServerIds = const {};
List<MediaItem> Function() onDeckResult = () => const [];
List<MediaHub> Function() hubsResult = () => const [];
@@ -72,6 +74,7 @@ class _FakeAggregationService extends DataAggregationService {
return (
items: limit != null && items.length > limit ? items.sublist(0, limit) : items,
succeededServerIds: onDeckSucceededServerIds ?? serverIds ?? const {'server_1'},
cancelledServerIds: onDeckCancelledServerIds,
);
}
@@ -85,7 +88,11 @@ class _FakeAggregationService extends DataAggregationService {
}) async {
hubCalls++;
lastHubsServerIds = serverIds;
return (hubs: hubsResult(), succeededServerIds: hubSucceededServerIds ?? serverIds ?? const {'server_1'});
return (
hubs: hubsResult(),
succeededServerIds: hubSucceededServerIds ?? serverIds ?? const {'server_1'},
cancelledServerIds: hubCancelledServerIds,
);
}
}
@@ -288,6 +295,112 @@ void main() {
expect(binderProvider.errorMessage, isNotNull);
});
// A pass in which zero servers succeeded is never authoritative: it must
// not wipe existing content, and it may only commit "loaded, empty" when
// the failure is settled (no cancellations, binder not running). The
// sign-in empty-flash regression: a rebind tore down the client mid-fetch,
// the aborted pass committed loaded-empty, and the screen flashed
// "no content available" until the follow-up load landed.
test('zero-success pass with cancellations stays loading instead of committing empty', () async {
aggregation.onDeckSucceededServerIds = const {};
aggregation.hubSucceededServerIds = const {};
aggregation.onDeckCancelledServerIds = const {'server_1'};
aggregation.hubCancelledServerIds = const {'server_1'};
await provider.load();
expect(provider.isLoading, isTrue);
expect(provider.areHubsLoading, isTrue);
expect(provider.errorMessage, isNull);
expect(provider.loadGeneration, 0);
// The guaranteed follow-up load lands the real content.
aggregation.onDeckSucceededServerIds = null;
aggregation.hubSucceededServerIds = null;
aggregation.onDeckCancelledServerIds = const {};
aggregation.hubCancelledServerIds = const {};
aggregation.onDeckResult = () => [_item('a')];
aggregation.hubsResult = () => [_hub('hub-1')];
await provider.load();
expect(provider.onDeck.map((i) => i.id), ['a']);
expect(provider.hubs.map((h) => h.id), ['hub-1']);
expect(provider.isLoading, isFalse);
expect(provider.areHubsLoading, isFalse);
});
test('zero-success pass during profile binding stays loading (no cancellations)', () async {
// Covers the timeout-during-bind window: every fetch failed while the
// binder was still wiring servers, with no cancellation marker.
isBinding = true;
aggregation.onDeckSucceededServerIds = const {};
aggregation.hubSucceededServerIds = const {};
await provider.load();
expect(provider.isLoading, isTrue);
expect(provider.areHubsLoading, isTrue);
expect(provider.errorMessage, isNull);
});
test('settled zero-success pass with no prior content commits loaded-empty', () async {
// Locks the no-eternal-spinner constraint: a genuinely dead server
// outside any disruption window keeps today's empty state.
aggregation.onDeckSucceededServerIds = const {};
aggregation.hubSucceededServerIds = const {};
await provider.load();
expect(provider.isLoading, isFalse);
expect(provider.areHubsLoading, isFalse);
expect(provider.onDeck, isEmpty);
expect(provider.hubs, isEmpty);
expect(provider.errorMessage, isNull);
});
test('totally failed refresh keeps previous content instead of wiping it', () async {
aggregation.onDeckResult = () => [_item('a')];
aggregation.hubsResult = () => [_hub('hub-1')];
await provider.load();
final generationBefore = provider.loadGeneration;
aggregation.onDeckSucceededServerIds = const {};
aggregation.hubSucceededServerIds = const {};
aggregation.onDeckResult = () => const [];
aggregation.hubsResult = () => const [];
await provider.load();
expect(provider.onDeck.map((i) => i.id), ['a']);
expect(provider.hubs.map((h) => h.id), ['hub-1']);
expect(provider.isLoading, isFalse);
expect(provider.areHubsLoading, isFalse);
// No new data: a failed pass must not reset the hero carousel.
expect(provider.loadGeneration, generationBefore);
// The kept content does not count as covering the failed servers — the
// next status emission refetches them.
aggregation.onDeckSucceededServerIds = null;
aggregation.hubSucceededServerIds = null;
aggregation.onDeckResult = () => [_item('b')];
final callsBefore = aggregation.onDeckCalls;
await provider.syncToOnlineServers({'server_1'});
expect(aggregation.onDeckCalls, greaterThan(callsBefore));
});
test('a disrupted half is independent: on-deck commits while hubs stay loading', () async {
aggregation.onDeckResult = () => [_item('a')];
aggregation.hubSucceededServerIds = const {};
aggregation.hubCancelledServerIds = const {'server_1'};
await provider.load();
expect(provider.isLoading, isFalse);
expect(provider.onDeck.map((i) => i.id), ['a']);
expect(provider.areHubsLoading, isTrue);
expect(provider.hubs, isEmpty);
});
test('updateItem refetches one item and swaps it in place', () async {
aggregation.onDeckResult = () => [_item('ep-1')];
aggregation.hubsResult = () => [
@@ -2,6 +2,7 @@ import 'dart:async';
import 'package:plezy/media/ids.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:plezy/exceptions/media_server_exceptions.dart';
import 'package:plezy/media/media_backend.dart';
import 'package:plezy/media/media_kind.dart';
import 'package:plezy/media/media_library.dart';
@@ -364,6 +365,103 @@ void main() {
manager.dispose();
});
test('a first load disrupted by cancellations stays loading instead of flashing empty', () async {
// The sign-in empty-flash regression: a rebind tore the client down
// mid-fetch, the aborted pass used to commit loaded-empty, and the
// sidebar flashed "no libraries found" until the follow-up load landed.
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: ServerId('A'), libraries: [_serverLib(ServerId('A'), '1', 'Movies A')])
..error = MediaServerHttpException(type: MediaServerHttpErrorType.cancelled, message: 'HTTP client is closing');
manager.debugRegisterClientForTesting(clientA);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.loadLibraries();
expect(p.isLoading, isTrue);
expect(p.hasLoaded, isFalse);
expect(p.errorMessage, isNull);
// The guaranteed follow-up load (binding-settle prime / next status
// emission) lands the real list.
clientA.error = null;
await p.loadLibraries();
expect(p.hasLoaded, isTrue);
expect(p.libraries.map((l) => l.title), ['Movies A']);
p.dispose();
manager.dispose();
});
test('a zero-success first load during profile binding stays loading', () async {
// The timeout-during-bind window: every fetch failed while the binder
// was still wiring servers, with no cancellation marker.
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: ServerId('A'), libraries: [_serverLib(ServerId('A'), '1', 'Movies A')])
..error = Exception('probe timed out');
manager.debugRegisterClientForTesting(clientA);
var binding = true;
final p = LibrariesProvider(isProfileBinding: () => binding)..initialize(DataAggregationService(manager));
await p.loadLibraries();
expect(p.isLoading, isTrue);
expect(p.hasLoaded, isFalse);
binding = false;
clientA.error = null;
await p.loadLibraries();
expect(p.hasLoaded, isTrue);
expect(p.libraries.map((l) => l.title), ['Movies A']);
p.dispose();
manager.dispose();
});
test('a settled zero-success first load still commits loaded-empty', () async {
// Locks the no-eternal-spinner constraint: a genuinely dead server
// outside any disruption window keeps today's empty state.
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: ServerId('A'))..error = Exception('connection refused');
manager.debugRegisterClientForTesting(clientA);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.loadLibraries();
expect(p.hasLoaded, isTrue);
expect(p.libraries, isEmpty);
expect(p.errorMessage, isNull);
p.dispose();
manager.dispose();
});
test('a totally failed silent refresh keeps the last good list', () async {
// Regression (pre-existing wipe bug): a reload-in-place where every
// server fails without throwing used to replace the list with [] —
// blanking the sidebar on a transient outage.
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: ServerId('A'), libraries: [_serverLib(ServerId('A'), '1', 'Movies A')]);
manager.debugRegisterClientForTesting(clientA);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.loadLibraries();
expect(p.libraries.map((l) => l.title), ['Movies A']);
clientA.error = Exception('offline');
await p.loadLibraries();
expect(p.hasLoaded, isTrue);
expect(p.libraries.map((l) => l.title), ['Movies A']);
// The kept list does not count as covering the failed server — the
// next sync refetches it.
clientA.error = null;
final callsBefore = clientA.fetchLibrariesCalls;
await p.syncToOnlineServers({'A'});
expect(clientA.fetchLibrariesCalls, callsBefore + 1);
p.dispose();
manager.dispose();
});
test('online-servers listener is removed on dispose', () {
final manager = MultiServerManager();
final multiServer = MultiServerProvider(manager, DataAggregationService(manager));
@@ -7,6 +7,9 @@ import 'package:http/http.dart' as http;
import 'package:http/testing.dart';
import 'package:plezy/connection/connection.dart';
import 'package:plezy/database/app_database.dart';
import 'package:plezy/exceptions/media_server_exceptions.dart';
import 'package:plezy/media/media_backend.dart';
import 'package:plezy/media/media_library.dart';
import 'package:plezy/media/media_server_client.dart';
import 'package:plezy/models/plex/plex_config.dart';
import 'package:plezy/services/data_aggregation_service.dart';
@@ -29,6 +32,34 @@ JellyfinConnection _conn() => JellyfinConnection(
http.Response _json(Object body) => http.Response(jsonEncode(body), 200, headers: {'content-type': 'application/json'});
/// Minimal client whose `fetchLibraries` either returns canned libraries or
/// throws [error] — enough surface to exercise the fan-out's per-server
/// failure classification without a real backend.
class _LibrariesClient implements MediaServerClient {
_LibrariesClient(this.serverId, {this.error, this.libraries = const []});
@override
final ServerId serverId;
@override
final String serverName = 'Server';
final Object? error;
final List<MediaLibrary> libraries;
@override
Future<List<MediaLibrary>> fetchLibraries() async {
if (error != null) throw error!;
return libraries;
}
@override
void close() {}
@override
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}
/// Smoke tests for the surviving cross-server aggregation surface on
/// [DataAggregationService]. Single-server passthroughs were removed in
/// favour of `context.tryGetMediaClientForServer(...).<method>()`; what's
@@ -65,6 +96,37 @@ void main() {
expect(onDeck.succeededServerIds, isEmpty);
});
test('classifies cancelled per-server failures apart from settled ones', () async {
// A cancelled fetch (our own client torn down mid-request) says nothing
// about the server's content; consumers use cancelledServerIds to keep
// a disrupted pass from being committed as authoritative. A settled
// failure (server down) lands in neither set.
manager.debugRegisterClientForTesting(
_LibrariesClient(
ServerId('ok'),
libraries: [MediaLibrary(id: '1', backend: MediaBackend.plex, title: 'Movies', serverId: ServerId('ok'))],
),
);
manager.debugRegisterClientForTesting(
_LibrariesClient(
ServerId('torn-down'),
error: MediaServerHttpException(type: MediaServerHttpErrorType.cancelled, message: 'HTTP client is closing'),
),
);
manager.debugRegisterClientForTesting(
_LibrariesClient(
ServerId('down'),
error: MediaServerHttpException(type: MediaServerHttpErrorType.connectionError, message: 'refused'),
),
);
final result = await service.getMediaLibrariesFromAllServers();
expect(result.libraries.map((l) => l.title), ['Movies']);
expect(result.succeededServerIds, {'ok'});
expect(result.cancelledServerIds, {'torn-down'});
});
test('searchAcrossServers overfetches and ranks before trimming across backends', () async {
final plexRequests = <Uri>[];
final jellyfinRequests = <Uri>[];
@@ -231,4 +231,65 @@ void main() {
expect(requests.map((uri) => uri.host), ['primary.example.com', 'fallback.example.com', 'primary.example.com']);
});
});
group('cancellation vs treat-as-empty', () {
// The hub/next-up fetch helpers swallow per-endpoint failures into empty
// lists so one broken endpoint doesn't sink a whole row. A *cancelled*
// request is different: it means our own client was torn down mid-fetch
// and says nothing about the server's content, so it must propagate —
// otherwise a disrupted server counts as "succeeded with partial data"
// and aborted sign-in fetches flash an empty home screen.
MediaServerHttpException cancelled() =>
MediaServerHttpException(type: MediaServerHttpErrorType.cancelled, message: 'HTTP client is closing');
test('fetchContinueWatching propagates a cancelled NextUp sub-fetch', () async {
final client = _withMock(
MockClient((req) async {
if (req.url.path == '/Shows/NextUp') throw cancelled();
return http.Response(jsonEncode({'Items': []}), 200, headers: {'content-type': 'application/json'});
}),
);
addTearDown(client.close);
await expectLater(
client.fetchContinueWatching(),
throwsA(isA<MediaServerHttpException>().having((e) => e.isCancellation, 'isCancellation', isTrue)),
);
});
test('fetchContinueWatching still treats a NextUp server error as empty', () async {
final client = _withMock(
MockClient((req) async {
if (req.url.path == '/Shows/NextUp') return http.Response('Internal error', 500);
return http.Response(
jsonEncode({
'Items': [
{'Id': 'ep-1', 'Type': 'Episode', 'Name': 'Resume Me'},
],
}),
200,
headers: {'content-type': 'application/json'},
);
}),
);
addTearDown(client.close);
final items = await client.fetchContinueWatching();
expect(items.map((i) => i.id), ['ep-1']);
});
test('fetchMoreHubItems propagates a cancellation and swallows server errors', () async {
final cancelledClient = _withMock(MockClient((_) async => throw cancelled()));
addTearDown(cancelledClient.close);
await expectLater(
cancelledClient.fetchMoreHubItems('home.nextup'),
throwsA(isA<MediaServerHttpException>().having((e) => e.isCancellation, 'isCancellation', isTrue)),
);
final failingClient = _withMock(MockClient((_) async => http.Response('Internal error', 500)));
addTearDown(failingClient.close);
expect(await failingClient.fetchMoreHubItems('home.nextup'), isEmpty);
});
});
}
@@ -322,6 +322,107 @@ void main() {
});
});
// ============================================================
// addJellyfinConnection reuse
// ============================================================
group('addJellyfinConnection reuse', () {
// The reuse branch is what keeps a passive rebind (re-adding the same
// persisted connection) from tearing down a live client and aborting its
// in-flight requests. The identity assertions are load-bearing: any
// recreation implies the prior client was closed.
test('re-adding an identical connection reuses the live client', () async {
var probes = 0;
final client = JellyfinClient.forTesting(
connection: _jellyfinConnection('user-a'),
httpClient: MockClient((request) async {
probes++;
expect(request.url.path, '/Users/Me');
return http.Response('{}', 200, headers: {'content-type': 'application/json'});
}),
);
addTearDown(client.close);
final m = MultiServerManager();
addTearDown(m.dispose);
m.debugRegisterJellyfinClientForTesting(client);
final healthy = await m.addJellyfinConnection(_jellyfinConnection('user-a'));
expect(healthy, isTrue);
expect(m.getJellyfinClientByCompoundId('jf-machine/user-a'), same(client));
expect(m.getClient(ServerId('jf-machine')), same(client));
// One fresh health probe on the existing client; no recreation.
expect(probes, 1);
});
test('reuse rebinds an inactive compound client without closing the active one', () async {
JellyfinClient clientFor(String userId) => JellyfinClient.forTesting(
connection: _jellyfinConnection(userId),
httpClient: MockClient((_) async => http.Response('{}', 200, headers: {'content-type': 'application/json'})),
);
final userA = clientFor('user-a');
final userB = clientFor('user-b');
addTearDown(userA.close);
addTearDown(userB.close);
final m = MultiServerManager();
addTearDown(m.dispose);
m.debugRegisterJellyfinClientForTesting(userA);
m.debugRegisterJellyfinClientForTesting(userB); // takes the machine slot
await m.addJellyfinConnection(_jellyfinConnection('user-a'));
expect(m.getClient(ServerId('jf-machine')), same(userA));
expect(m.getJellyfinClientByCompoundId('jf-machine/user-a'), same(userA));
// The other user's client stays registered for a future switch back.
expect(m.getJellyfinClientByCompoundId('jf-machine/user-b'), same(userB));
});
});
group('canReuseJellyfinClient', () {
// A false verdict routes addJellyfinConnection to the pre-existing
// replace path (covered by 'ignores stale admin-status persistence from
// a replaced Jellyfin client' above).
final base = _jellyfinConnection('user-a');
test('identical connection is reusable', () {
expect(MultiServerManager.canReuseJellyfinClient(live: base, incoming: _jellyfinConnection('user-a')), isTrue);
});
test('changed access token requires recreation', () {
expect(
MultiServerManager.canReuseJellyfinClient(live: base, incoming: base.copyWith(accessToken: 'rotated')),
isFalse,
);
});
test('changed device id requires recreation', () {
expect(
MultiServerManager.canReuseJellyfinClient(live: base, incoming: base.copyWith(deviceId: 'other-device')),
isFalse,
);
});
test('same URL set with a different active URL is reusable', () {
// The live client rotates its active endpoint on its own; the add-path
// race reorders the candidate list. Neither warrants a teardown.
final live = base.copyWith(
baseUrl: 'https://a.example.com',
baseUrls: ['https://a.example.com', 'https://b.example.com'],
);
final incoming = base.copyWith(
baseUrl: 'https://b.example.com',
baseUrls: ['https://b.example.com', 'https://a.example.com'],
);
expect(MultiServerManager.canReuseJellyfinClient(live: live, incoming: incoming), isTrue);
});
test('an added or removed URL requires recreation', () {
final twoUrls = base.copyWith(baseUrls: ['https://jf.example.com', 'https://alt.example.com']);
expect(MultiServerManager.canReuseJellyfinClient(live: twoUrls, incoming: base), isFalse);
expect(MultiServerManager.canReuseJellyfinClient(live: base, incoming: twoUrls), isFalse);
});
});
// ============================================================
// removeServer
// ============================================================