fix(player): tell the user when the server cannot read the media file
A 404 on the media stream means the server resolved the item but could not open the file behind it — moved, deleted, or on storage that went away. Jellyfin maps the resulting FileNotFoundException to 404, and PlaybackInfo never stats the file, so negotiation succeeds and only the stream request fails. Playback then died with a snackbar reading "Failed to open [REDACTED_URL]" before popping the route, which tells the user nothing and leaves nothing useful in a bug report. Generalize the HTTP-500 log probe into PlayerError.httpStatusFromLog and latch every status in fatalPlaybackHttpStatuses. Each latches on its own so the 503 that stream-lavf-o deliberately retries cannot mask the fatal status behind it. A 404 now raises a dedicated modal naming the cause and the fix. On Android a 404 previously failed the "Response code: 500" string test and fell through to the ExoPlayer→MPV fallback, showing "switching to compatible player" before failing again on the same request. Read the real status off HttpDataSource.InvalidResponseCodeException instead and skip the fallback: an HTTP status is not a codec problem.
This commit is contained in:
@@ -0,0 +1,56 @@
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:plezy/mpv/models.dart';
|
||||
|
||||
void main() {
|
||||
group('PlayerError.httpStatusFromLog', () {
|
||||
test('reads the status out of the ffmpeg warn line that precedes a failed open', () {
|
||||
// Verbatim from the #1750 report: the only line in the whole failure that
|
||||
// names the status. The error-level "Failed to open ..." that follows
|
||||
// omits it.
|
||||
expect(PlayerError.httpStatusFromLog('http: HTTP error 404 Not Found'), 404);
|
||||
expect(PlayerError.httpStatusFromLog('http: HTTP error 500 Internal Server Error'), 500);
|
||||
expect(PlayerError.httpStatusFromLog('http: HTTP error 503 Service Unavailable'), 503);
|
||||
});
|
||||
|
||||
test('reads the status media3 stringifies into its exception chain', () {
|
||||
expect(
|
||||
PlayerError.httpStatusFromLog(
|
||||
'androidx.media3.datasource.HttpDataSource\$InvalidResponseCodeException: Response code: 404',
|
||||
),
|
||||
404,
|
||||
);
|
||||
expect(PlayerError.httpStatusFromLog('Response code: 500'), 500);
|
||||
});
|
||||
|
||||
test('returns null for player logs that name no status', () {
|
||||
expect(PlayerError.httpStatusFromLog('Failed to open https://jf.example.com/Videos/item-1/stream.'), isNull);
|
||||
expect(PlayerError.httpStatusFromLog('loading failed'), isNull);
|
||||
expect(PlayerError.httpStatusFromLog(''), isNull);
|
||||
});
|
||||
|
||||
test('does not mistake an unrelated number for a status', () {
|
||||
// A bare code with no HTTP context must not reach the fatal dialogs.
|
||||
expect(PlayerError.httpStatusFromLog('Set property: stream-buffer-size="404"'), isNull);
|
||||
expect(PlayerError.httpStatusFromLog('audio/aac 500 kbps'), isNull);
|
||||
// Adjacent digits are not a 3-digit status.
|
||||
expect(PlayerError.httpStatusFromLog('http: HTTP error 4040 Nope'), isNull);
|
||||
});
|
||||
|
||||
test('reports the first status when a line names several', () {
|
||||
expect(PlayerError.httpStatusFromLog('HTTP error 404 after HTTP error 500'), 404);
|
||||
});
|
||||
});
|
||||
|
||||
group('fatalPlaybackHttpStatuses', () {
|
||||
test('covers exactly the statuses no client-side retry can recover', () {
|
||||
expect(fatalPlaybackHttpStatuses, {404, 500});
|
||||
});
|
||||
|
||||
test('excludes the 503 the reconnect path deliberately retries', () {
|
||||
// stream-lavf-o sets reconnect_on_http_error=503, so a 503 is expected
|
||||
// mid-playback and must not latch as fatal.
|
||||
expect(fatalPlaybackHttpStatuses.contains(503), isFalse);
|
||||
expect(PlayerError.httpStatusFromLog('http: HTTP error 503 Service Unavailable'), 503);
|
||||
});
|
||||
});
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter/services.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:plezy/i18n/strings.g.dart';
|
||||
import 'package:plezy/utils/dialogs.dart';
|
||||
import 'package:plezy/utils/platform_detector.dart';
|
||||
|
||||
@@ -111,6 +112,44 @@ void main() {
|
||||
await expectLater(result, completion(isNull));
|
||||
expect(find.byType(AlertDialog), findsNothing);
|
||||
});
|
||||
|
||||
testWidgets('media-unreadable dialog names the server-side cause and cannot be dismissed by the barrier', (
|
||||
tester,
|
||||
) async {
|
||||
final hostContext = await _pumpHost(tester);
|
||||
final result = showMediaUnreadableDialog(hostContext);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
expect(find.text(t.messages.mediaUnreadableTitle), findsOneWidget);
|
||||
// The body has to say what a 404 on the stream actually means, because the
|
||||
// only recovery is on the server (#1750).
|
||||
expect(find.textContaining('HTTP 404'), findsOneWidget);
|
||||
expect(find.textContaining('could not read'), findsOneWidget);
|
||||
|
||||
// Barrier taps must not strand the caller's future.
|
||||
await tester.tapAt(const Offset(10, 10));
|
||||
await tester.pumpAndSettle();
|
||||
expect(find.byType(AlertDialog), findsOneWidget);
|
||||
|
||||
await tester.tap(find.text(t.common.close));
|
||||
await tester.pumpAndSettle();
|
||||
await expectLater(result, completes);
|
||||
expect(find.byType(AlertDialog), findsNothing);
|
||||
});
|
||||
|
||||
testWidgets('server-limit dialog stays distinct from the media-unreadable one', (tester) async {
|
||||
final hostContext = await _pumpHost(tester);
|
||||
final result = showServerLimitDialog(hostContext);
|
||||
await tester.pumpAndSettle();
|
||||
|
||||
expect(find.text(t.messages.serverLimitTitle), findsOneWidget);
|
||||
expect(find.textContaining('HTTP 500'), findsOneWidget);
|
||||
expect(find.text(t.messages.mediaUnreadableTitle), findsNothing);
|
||||
|
||||
await tester.tap(find.text(t.common.close));
|
||||
await tester.pumpAndSettle();
|
||||
await expectLater(result, completes);
|
||||
});
|
||||
}
|
||||
|
||||
Future<BuildContext> _pumpHost(WidgetTester tester) async {
|
||||
|
||||
Reference in New Issue
Block a user