mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-08 17:06:15 -08:00
fix(playback): avoid false codec diagnostics (#1314)
* docs(playback): design accurate native diagnostics * docs(playback): plan accurate native diagnostics * fix(playback): classify native source errors from evidence * fix(playback): preserve Video.js HTTP error context * fix(playback): show explicit HTTP playback errors * docs(playback): document native error evidence
This commit is contained in:
1 parent
32ba209b63
commit
bf13849d69
11 files changed
+1156
-6
No files matched your search
@@ -328,6 +328,13 @@ Supported diagnostic codes are:
|
||||
- `drm-or-encryption`
|
||||
- `unknown-playback-error`
|
||||
|
||||
Native `MediaError` code 4 alone is not codec evidence. A source with a known
|
||||
browser-incompatible container remains `unsupported-container`; otherwise, a
|
||||
code 4 error without stronger evidence is `unknown-playback-error`. An explicit
|
||||
Video.js HTTP error is `network-error` and shows its status. Because an HTTP
|
||||
status is server/network evidence rather than decoding evidence, external
|
||||
decoding is not presented as a likely fix.
|
||||
|
||||
`network-error` is reserved for provider/network loading failures. Browser security failures such as CORS, mixed content, Content Security Policy, and private-network-access blocks are classified as `browser-access-error` so the UI can explain that the browser player was blocked before playback reached decoding.
|
||||
|
||||
mpegts.js `Early-EOF` failures on MPEG-TS streams are classified as `media-decode-error` instead of generic `network-error`. These failures usually mean the fetch stream ended before mpegts.js expected a complete transport stream, and external players may still handle the same URL more tolerant of short reads or malformed TS boundaries.
|
||||
|
||||
@@ -0,0 +1,743 @@
|
||||
# Accurate Native Playback Diagnostics Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Fix issue #1159 so unavailable HLS sources are never misreported as unsupported codecs, while retaining an explicit HTTP error status exposed by Video.js.
|
||||
|
||||
**Architecture:** Extend the shared native-error input and normalized diagnostic with two safe structured fields: HTTP status and a bounded Video.js error type. Give explicit HTTP evidence precedence, keep independently known unsupported containers, and classify every other native code-4 failure as unknown; reuse the existing network/unknown UI copy and render `HTTP <status>` through existing diagnostic metadata and details surfaces.
|
||||
|
||||
**Tech Stack:** Angular 21, TypeScript, Video.js 8, Jest through Nx, ngx-translate JSON catalogs, Markdown release notes.
|
||||
|
||||
---
|
||||
|
||||
### Task 0: Bootstrap the Nx workspace
|
||||
|
||||
**Files:**
|
||||
- Verify only: `package.json`
|
||||
- Verify only: `pnpm-lock.yaml`
|
||||
|
||||
- [ ] **Step 1: Install the locked dependencies**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm install --frozen-lockfile
|
||||
```
|
||||
|
||||
Expected: exit 0 without changing `pnpm-lock.yaml`.
|
||||
|
||||
- [ ] **Step 2: Verify Nx project discovery**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm nx show projects
|
||||
```
|
||||
|
||||
Expected: exit 0 and output containing `ui-playback`, `web`, and `web-e2e`.
|
||||
|
||||
### Task 1: Make native classification evidence-based
|
||||
|
||||
**Files:**
|
||||
- Modify: `libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.model.ts`
|
||||
- Modify: `libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.ts`
|
||||
- Test: `libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts`
|
||||
|
||||
- [ ] **Step 1: Write the failing classifier regressions**
|
||||
|
||||
In `playback-diagnostics.util.spec.ts`, replace the MPEG-TS native code-4
|
||||
expectation with an unknown diagnosis and add the HTTP/opaque HLS cases:
|
||||
|
||||
```typescript
|
||||
it('keeps ambiguous MPEG-TS native source failures unknown', () => {
|
||||
const issue = classifyNativePlaybackIssue(
|
||||
{ code: 4, message: 'source not supported' },
|
||||
createPlaybackSourceMetadata({
|
||||
url: 'https://example.com/live/stream',
|
||||
mimeType: 'video/mp2t',
|
||||
player: 'videojs',
|
||||
})
|
||||
);
|
||||
|
||||
expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError);
|
||||
expect(issue.container).toBe('mp2t');
|
||||
expect(issue.externalFallbackRecommended).toBe(false);
|
||||
});
|
||||
|
||||
it('classifies an explicit Video.js HTTP failure as a network error', () => {
|
||||
const issue = classifyNativePlaybackIssue(
|
||||
{
|
||||
code: 4,
|
||||
message: 'The media could not be loaded',
|
||||
status: 404,
|
||||
metadata: { errorType: 'networkrequestfailed' },
|
||||
},
|
||||
createPlaybackSourceMetadata({
|
||||
url: 'https://example.com/missing/playlist.m3u8',
|
||||
mimeType: 'application/x-mpegURL',
|
||||
player: 'videojs',
|
||||
})
|
||||
);
|
||||
|
||||
expect(issue).toEqual(
|
||||
expect.objectContaining({
|
||||
code: PlaybackDiagnosticCode.NetworkError,
|
||||
httpStatus: 404,
|
||||
nativeErrorType: 'networkrequestfailed',
|
||||
externalFallbackRecommended: false,
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps native HLS code 4 unknown when no HTTP or codec evidence exists', () => {
|
||||
const issue = classifyNativePlaybackIssue(
|
||||
{
|
||||
code: 4,
|
||||
message:
|
||||
'The media could not be loaded, either because the server or network failed or because the format is not supported.',
|
||||
},
|
||||
createPlaybackSourceMetadata({
|
||||
url: 'https://example.com/live/playlist.m3u8',
|
||||
mimeType: 'application/x-mpegURL',
|
||||
player: 'videojs',
|
||||
})
|
||||
);
|
||||
|
||||
expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError);
|
||||
expect(issue.externalFallbackRecommended).toBe(false);
|
||||
expect(issue.httpStatus).toBeUndefined();
|
||||
});
|
||||
|
||||
it('does not treat opaque status zero or unsafe metadata as HTTP evidence', () => {
|
||||
const issue = classifyNativePlaybackIssue(
|
||||
{
|
||||
code: 4,
|
||||
status: 0,
|
||||
metadata: { errorType: 'request failed: token=secret value' },
|
||||
},
|
||||
createPlaybackSourceMetadata({
|
||||
url: 'https://example.com/live/playlist.m3u8',
|
||||
mimeType: 'application/x-mpegURL',
|
||||
player: 'videojs',
|
||||
})
|
||||
);
|
||||
|
||||
expect(issue.code).toBe(PlaybackDiagnosticCode.UnknownPlaybackError);
|
||||
expect(issue.httpStatus).toBeUndefined();
|
||||
expect(issue.nativeErrorType).toBeUndefined();
|
||||
});
|
||||
```
|
||||
|
||||
Keep the existing `.mkv` and `video/x-msvideo` tests unchanged so known
|
||||
unsupported containers remain covered.
|
||||
|
||||
- [ ] **Step 2: Run the focused classifier spec to verify RED**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
NODE_OPTIONS=--experimental-vm-modules \
|
||||
node node_modules/jest/bin/jest.js \
|
||||
--config jest.web-esm.workspace.ts \
|
||||
--runTestsByPath \
|
||||
libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts \
|
||||
--runInBand
|
||||
```
|
||||
|
||||
Expected: FAIL because code 4 still becomes `unsupported-codec` and the
|
||||
normalized diagnostic does not retain `httpStatus` or `nativeErrorType`.
|
||||
|
||||
- [ ] **Step 3: Extend the diagnostic input and output contracts**
|
||||
|
||||
In `playback-diagnostics.model.ts`, replace `NativePlaybackErrorInput` with:
|
||||
|
||||
```typescript
|
||||
export interface NativePlaybackErrorMetadataInput {
|
||||
readonly errorType?: unknown;
|
||||
}
|
||||
|
||||
export interface NativePlaybackErrorInput {
|
||||
readonly code?: number;
|
||||
readonly message?: string;
|
||||
readonly status?: number;
|
||||
readonly metadata?: NativePlaybackErrorMetadataInput;
|
||||
}
|
||||
```
|
||||
|
||||
Add these optional properties beside the existing native error properties in
|
||||
`PlaybackDiagnostic`:
|
||||
|
||||
```typescript
|
||||
readonly httpStatus?: number;
|
||||
readonly nativeErrorType?: string;
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Implement safe evidence extraction and classification**
|
||||
|
||||
In `playback-diagnostics.util.ts`, add:
|
||||
|
||||
```typescript
|
||||
const MIN_HTTP_ERROR_STATUS = 400;
|
||||
const MAX_HTTP_ERROR_STATUS = 599;
|
||||
const NATIVE_ERROR_TYPE_PATTERN = /^[A-Za-z0-9._:-]{1,128}$/;
|
||||
|
||||
function getHttpErrorStatus(
|
||||
error: NativePlaybackErrorInput | MediaError | null | undefined
|
||||
): number | undefined {
|
||||
if (!error || !('status' in error)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const status = error.status;
|
||||
return typeof status === 'number' &&
|
||||
Number.isInteger(status) &&
|
||||
status >= MIN_HTTP_ERROR_STATUS &&
|
||||
status <= MAX_HTTP_ERROR_STATUS
|
||||
? status
|
||||
: undefined;
|
||||
}
|
||||
|
||||
function getNativeErrorType(
|
||||
error: NativePlaybackErrorInput | MediaError | null | undefined
|
||||
): string | undefined {
|
||||
if (!error || !('metadata' in error)) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const errorType = error.metadata?.errorType;
|
||||
return typeof errorType === 'string' &&
|
||||
NATIVE_ERROR_TYPE_PATTERN.test(errorType)
|
||||
? errorType
|
||||
: undefined;
|
||||
}
|
||||
```
|
||||
|
||||
At the start of `classifyNativePlaybackIssue`, extract the two values:
|
||||
|
||||
```typescript
|
||||
const httpStatus = getHttpErrorStatus(error);
|
||||
const nativeErrorType = getNativeErrorType(error);
|
||||
```
|
||||
|
||||
Before the existing code-2 branch, add:
|
||||
|
||||
```typescript
|
||||
if (httpStatus !== undefined) {
|
||||
return createPlaybackDiagnostic({
|
||||
code: DiagnosticCode.NetworkError,
|
||||
source: DiagnosticSource.Native,
|
||||
metadata,
|
||||
httpStatus,
|
||||
nativeErrorCode,
|
||||
nativeErrorMessage,
|
||||
nativeErrorType,
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
Pass `nativeErrorType` through every remaining native diagnostic creation.
|
||||
Change the code-4 branch to:
|
||||
|
||||
```typescript
|
||||
if (nativeErrorCode === SOURCE_NOT_SUPPORTED_CODE) {
|
||||
return createPlaybackDiagnostic({
|
||||
code: isLikelyContainerIssue(metadata)
|
||||
? DiagnosticCode.UnsupportedContainer
|
||||
: DiagnosticCode.UnknownPlaybackError,
|
||||
source: DiagnosticSource.Native,
|
||||
metadata,
|
||||
nativeErrorCode,
|
||||
nativeErrorMessage,
|
||||
nativeErrorType,
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
Extend `createPlaybackDiagnostic` options and return value with:
|
||||
|
||||
```typescript
|
||||
readonly httpStatus?: number;
|
||||
readonly nativeErrorType?: string;
|
||||
```
|
||||
|
||||
and copy both values into the resulting `PlaybackDiagnostic`.
|
||||
|
||||
- [ ] **Step 5: Run the classifier spec to verify GREEN**
|
||||
|
||||
Run the focused command from step 2 again.
|
||||
|
||||
Expected: PASS, including the HLS 404, ambiguous HLS, status-zero, MPEG-TS,
|
||||
and known-container cases.
|
||||
|
||||
- [ ] **Step 6: Commit the classifier behavior**
|
||||
|
||||
```bash
|
||||
git add \
|
||||
libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.model.ts \
|
||||
libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.ts \
|
||||
libs/ui/playback/src/lib/playback-diagnostics/playback-diagnostics.util.spec.ts
|
||||
git commit -m "fix(playback): classify native source errors from evidence"
|
||||
```
|
||||
|
||||
Expected: one commit containing the normalized contract, classification rules,
|
||||
and their regression coverage.
|
||||
|
||||
### Task 2: Verify and align the Video.js HTTP boundary
|
||||
|
||||
**Files:**
|
||||
- Modify: `libs/ui/playback/src/lib/vjs-player/vjs-player.types.ts`
|
||||
- Test: `libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts`
|
||||
|
||||
- [ ] **Step 1: Add the Video.js boundary regression**
|
||||
|
||||
Import `NativePlaybackErrorInput` in `vjs-player.component.spec.ts`, change the
|
||||
harness `currentError` type to `NativePlaybackErrorInput | null`, and add:
|
||||
|
||||
```typescript
|
||||
it('preserves safe Video.js HTTP context in playback diagnostics', () => {
|
||||
const issues: Array<PlaybackDiagnostic | null> = [];
|
||||
component.playbackIssue.subscribe((issue) => issues.push(issue));
|
||||
render({
|
||||
sources: [
|
||||
{
|
||||
src: 'https://example.test/missing/playlist.m3u8',
|
||||
type: 'application/x-mpegURL',
|
||||
},
|
||||
],
|
||||
});
|
||||
harness.currentError = {
|
||||
code: 4,
|
||||
message: 'The media could not be loaded',
|
||||
status: 404,
|
||||
metadata: { errorType: 'networkrequestfailed' },
|
||||
};
|
||||
|
||||
harness.emit('error');
|
||||
|
||||
expect(issues.at(-1)).toEqual(
|
||||
expect.objectContaining({
|
||||
code: 'network-error',
|
||||
source: 'native',
|
||||
sourceUrl: 'https://example.test/missing/playlist.m3u8',
|
||||
httpStatus: 404,
|
||||
nativeErrorType: 'networkrequestfailed',
|
||||
externalFallbackRecommended: false,
|
||||
})
|
||||
);
|
||||
});
|
||||
```
|
||||
|
||||
Keep the existing MKV unsupported-container test.
|
||||
|
||||
- [ ] **Step 2: Run the focused component integration spec**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
NODE_OPTIONS=--experimental-vm-modules \
|
||||
node node_modules/jest/bin/jest.js \
|
||||
--config jest.web-esm.workspace.ts \
|
||||
--runTestsByPath \
|
||||
libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts \
|
||||
--runInBand
|
||||
```
|
||||
|
||||
Expected: PASS after Task 1. JavaScript already passes the complete error
|
||||
object at runtime; this regression proves that the component does not rebuild
|
||||
or strip it before classification.
|
||||
|
||||
- [ ] **Step 3: Reuse the shared native-error contract**
|
||||
|
||||
At the top of `vjs-player.types.ts`, add:
|
||||
|
||||
```typescript
|
||||
import type { NativePlaybackErrorInput } from '../playback-diagnostics/playback-diagnostics.model';
|
||||
```
|
||||
|
||||
Replace the `error` signature with:
|
||||
|
||||
```typescript
|
||||
error: () => NativePlaybackErrorInput | null;
|
||||
```
|
||||
|
||||
No component implementation change is required because
|
||||
`VjsPlayerComponent.handleVideoJsError` already passes the complete
|
||||
`player.error()` object to `classifyNativePlaybackIssue`.
|
||||
|
||||
- [ ] **Step 4: Verify the aligned Video.js type contract**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
NODE_OPTIONS=--experimental-vm-modules \
|
||||
node node_modules/jest/bin/jest.js \
|
||||
--config jest.web-esm.workspace.ts \
|
||||
--runTestsByPath \
|
||||
libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts \
|
||||
--runInBand
|
||||
pnpm exec tsc -p libs/ui/playback/tsconfig.lib.json --noEmit
|
||||
```
|
||||
|
||||
Expected: both commands pass for the HTTP integration case, existing
|
||||
source/reset behavior, and the production library type graph. The behavioral
|
||||
RED/GREEN proof lives in Task 1; this task aligns the TypeScript declaration
|
||||
with the already-tested runtime payload.
|
||||
|
||||
- [ ] **Step 5: Commit the Video.js boundary**
|
||||
|
||||
```bash
|
||||
git add \
|
||||
libs/ui/playback/src/lib/vjs-player/vjs-player.types.ts \
|
||||
libs/ui/playback/src/lib/vjs-player/vjs-player.component.spec.ts
|
||||
git commit -m "fix(playback): preserve Video.js HTTP error context"
|
||||
```
|
||||
|
||||
Expected: one commit containing only the Video.js type boundary and component
|
||||
regression.
|
||||
|
||||
### Task 3: Render explicit HTTP evidence
|
||||
|
||||
**Files:**
|
||||
- Modify: `libs/ui/playback/src/lib/web-player-view/web-player-view-diagnostics.utils.ts`
|
||||
- Test: `libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts`
|
||||
|
||||
- [ ] **Step 1: Write the failing rendered-diagnostic regression**
|
||||
|
||||
Add this helper in `web-player-view.component.spec.ts`:
|
||||
|
||||
```typescript
|
||||
function createHttpDiagnostic(): PlaybackDiagnostic {
|
||||
return {
|
||||
code: PlaybackDiagnosticCode.NetworkError,
|
||||
source: PlaybackDiagnosticSource.Native,
|
||||
sourceUrl: 'https://example.com/missing/playlist.m3u8',
|
||||
container: 'm3u8',
|
||||
mimeType: 'application/x-mpegURL',
|
||||
player: 'videojs',
|
||||
audioCodecs: [],
|
||||
videoCodecs: [],
|
||||
httpStatus: 404,
|
||||
nativeErrorCode: 4,
|
||||
nativeErrorMessage: 'The media could not be loaded',
|
||||
nativeErrorType: 'networkrequestfailed',
|
||||
externalFallbackRecommended: false,
|
||||
};
|
||||
}
|
||||
```
|
||||
|
||||
Add this component test:
|
||||
|
||||
```typescript
|
||||
it('renders explicit HTTP evidence without recommending an external player', () => {
|
||||
runtimeCapabilities.supportsManagedExternalPlayers = true;
|
||||
fixture.detectChanges();
|
||||
const issue = createHttpDiagnostic();
|
||||
|
||||
component.handlePlaybackIssue(issue);
|
||||
fixture.detectChanges();
|
||||
|
||||
const banner = fixture.debugElement.query(
|
||||
By.css('[data-test-id="playback-diagnostic-banner"]')
|
||||
);
|
||||
const mpvButton = fixture.debugElement.query(
|
||||
By.css('[data-test-id="playback-fallback-mpv"]')
|
||||
);
|
||||
|
||||
expect(banner.nativeElement.textContent).toContain('HTTP 404');
|
||||
expect(mpvButton).toBeNull();
|
||||
expect(component.getDiagnosticMeta(issue)).toBe('HTTP 404');
|
||||
expect(component.getDiagnosticDetails(issue)).toEqual(
|
||||
expect.arrayContaining([
|
||||
{
|
||||
labelKey: 'PLAYBACK_DIAGNOSTICS.DETAIL_ERROR_DETAILS',
|
||||
value: 'HTTP 404 · networkrequestfailed',
|
||||
},
|
||||
])
|
||||
);
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the focused view spec to verify RED**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
NODE_OPTIONS=--experimental-vm-modules \
|
||||
node node_modules/jest/bin/jest.js \
|
||||
--config jest.web-esm.workspace.ts \
|
||||
--runTestsByPath \
|
||||
libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts \
|
||||
--runInBand
|
||||
```
|
||||
|
||||
Expected: FAIL because the metadata still renders `m3u8` and the details helper
|
||||
does not include the structured HTTP context.
|
||||
|
||||
- [ ] **Step 3: Format HTTP metadata and safe error details**
|
||||
|
||||
At the start of `getDiagnosticMeta`, add:
|
||||
|
||||
```typescript
|
||||
if (issue.httpStatus !== undefined) {
|
||||
return `HTTP ${issue.httpStatus}`;
|
||||
}
|
||||
```
|
||||
|
||||
Replace the final error-details item in `getDiagnosticDetails` with:
|
||||
|
||||
```typescript
|
||||
{
|
||||
labelKey: 'PLAYBACK_DIAGNOSTICS.DETAIL_ERROR_DETAILS',
|
||||
value: formatDiagnosticErrorDetails(issue),
|
||||
},
|
||||
```
|
||||
|
||||
Add:
|
||||
|
||||
```typescript
|
||||
function formatDiagnosticErrorDetails(issue: PlaybackDiagnostic): string {
|
||||
return [
|
||||
issue.httpStatus === undefined ? '' : `HTTP ${issue.httpStatus}`,
|
||||
issue.nativeErrorType ?? '',
|
||||
issue.details ?? '',
|
||||
]
|
||||
.filter((value) => value.length > 0)
|
||||
.join(' · ');
|
||||
}
|
||||
```
|
||||
|
||||
Keep the existing translation keys and template unchanged.
|
||||
|
||||
- [ ] **Step 4: Run the view spec to verify GREEN**
|
||||
|
||||
Run the focused command from step 2 again.
|
||||
|
||||
Expected: PASS with visible `HTTP 404`, no MPV action, and the combined safe
|
||||
details row.
|
||||
|
||||
- [ ] **Step 5: Run all affected unit tests and lint**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback
|
||||
pnpm nx lint ui-playback
|
||||
```
|
||||
|
||||
Expected: both commands exit 0 with no failed tests or lint errors.
|
||||
|
||||
- [ ] **Step 6: Commit the rendered evidence**
|
||||
|
||||
```bash
|
||||
git add \
|
||||
libs/ui/playback/src/lib/web-player-view/web-player-view-diagnostics.utils.ts \
|
||||
libs/ui/playback/src/lib/web-player-view/web-player-view.component.spec.ts
|
||||
git commit -m "fix(playback): show explicit HTTP playback errors"
|
||||
```
|
||||
|
||||
Expected: one commit containing the diagnostic presentation and component
|
||||
coverage.
|
||||
|
||||
### Task 4: Document the behavior and add the release note
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/architecture/embedded-inline-playback.md`
|
||||
- Create: `.changes/playback-accurate-source-errors.md`
|
||||
|
||||
- [ ] **Step 1: Update the canonical playback documentation**
|
||||
|
||||
Add this paragraph after the supported diagnostic-code list in
|
||||
`docs/architecture/embedded-inline-playback.md`:
|
||||
|
||||
```markdown
|
||||
Native `MediaError` code 4 is not codec evidence by itself. A source already
|
||||
known to use a browser-incompatible container remains
|
||||
`unsupported-container`; otherwise the native failure stays
|
||||
`unknown-playback-error`. When Video.js exposes an explicit HTTP error status,
|
||||
the failure is classified as `network-error`, the status is shown in the
|
||||
diagnostic, and an external decoder is not presented as a likely fix for the
|
||||
same failed request.
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add the user-facing release note**
|
||||
|
||||
Create `.changes/playback-accurate-source-errors.md` with:
|
||||
|
||||
```markdown
|
||||
---
|
||||
type: fix
|
||||
area: playback
|
||||
issues: [1159]
|
||||
---
|
||||
|
||||
Unavailable streams no longer appear as unsupported codecs. When Video.js
|
||||
exposes a server error such as HTTP 404, the player shows that status; otherwise
|
||||
ambiguous source errors remain unidentified instead of guessing.
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Validate documentation and release metadata**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git diff --check
|
||||
pnpm run release:notes:validate
|
||||
pnpm run i18n:check
|
||||
```
|
||||
|
||||
Expected: all commands exit 0. The release note validates, and no translation
|
||||
catalog drift is introduced because the implementation reuses existing keys.
|
||||
|
||||
- [ ] **Step 4: Commit documentation and release note**
|
||||
|
||||
```bash
|
||||
git add \
|
||||
docs/architecture/embedded-inline-playback.md \
|
||||
.changes/playback-accurate-source-errors.md
|
||||
git commit -m "docs(playback): document native error evidence"
|
||||
```
|
||||
|
||||
Expected: one commit containing the canonical behavior contract and the
|
||||
issue-linked release note.
|
||||
|
||||
### Task 5: Complete verification and local Codex review
|
||||
|
||||
**Files:**
|
||||
- Verify: all files changed from `origin/master`
|
||||
|
||||
- [ ] **Step 1: Run the final affected validation ladder**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback
|
||||
pnpm nx lint ui-playback
|
||||
pnpm run typecheck:web
|
||||
pnpm run i18n:check
|
||||
pnpm run release:notes:validate
|
||||
git diff --check origin/master...HEAD
|
||||
git status --short
|
||||
```
|
||||
|
||||
Expected: every command exits 0, all `ui-playback` tests pass, and the worktree
|
||||
is clean. E2E is not required because no player selection, route, interaction,
|
||||
or overlay layout changes; classifier, boundary, and rendered output are
|
||||
covered by focused component tests.
|
||||
|
||||
- [ ] **Step 2: Dispatch a fresh local Codex reviewer**
|
||||
|
||||
Resolve the exact review range:
|
||||
|
||||
```bash
|
||||
git rev-parse origin/master
|
||||
git rev-parse HEAD
|
||||
```
|
||||
|
||||
Dispatch a fresh reviewer agent with no implementation-history context and
|
||||
this request:
|
||||
|
||||
```text
|
||||
Review origin/master...HEAD for issue #1159 against
|
||||
docs/superpowers/specs/2026-07-30-accurate-native-playback-diagnostics-design.md.
|
||||
Focus on correctness, regressions, unsafe disclosure, TypeScript/runtime
|
||||
contract mismatches, and missing tests. Report only actionable P0/P1/P2
|
||||
findings with exact file and line references; do not report style-only nits.
|
||||
Do not modify files.
|
||||
```
|
||||
|
||||
Expected: a local Codex review report before any push or PR creation.
|
||||
|
||||
- [ ] **Step 3: Verify and resolve every P1/P2 finding**
|
||||
|
||||
For each reported finding:
|
||||
|
||||
1. Reproduce or prove it from the code and tests.
|
||||
2. Add or update a regression test first when behavior changes.
|
||||
3. Run the focused test and observe the expected RED result.
|
||||
4. Apply the smallest valid fix.
|
||||
5. Run the focused test to GREEN.
|
||||
|
||||
If the review has confirmed P1/P2 findings, stage the tracked files changed by
|
||||
the verified fixes and commit them with:
|
||||
|
||||
```bash
|
||||
git add -u
|
||||
git commit -m "fix(playback): address local review findings"
|
||||
```
|
||||
|
||||
If a finding is invalid, record the concrete code/test evidence for rejecting
|
||||
it in the task summary instead of changing the implementation.
|
||||
|
||||
- [ ] **Step 4: Re-review after fixes**
|
||||
|
||||
If step 3 changed any file, send the same reviewer a follow-up request to
|
||||
re-check the new `origin/master...HEAD` diff for remaining P0/P1/P2 findings.
|
||||
Repeat steps 3 and 4 until no confirmed P1/P2 findings remain.
|
||||
|
||||
- [ ] **Step 5: Re-run final verification after review**
|
||||
|
||||
Run the complete validation command set from step 1 again.
|
||||
|
||||
Expected: all commands exit 0 after the final review changes, and
|
||||
`git status --short` is empty.
|
||||
|
||||
### Task 6: Push and create the focused pull request
|
||||
|
||||
**Files:**
|
||||
- Verify only: `.github/pull_request_template.md` when present
|
||||
|
||||
- [ ] **Step 1: Inspect the final commit range**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git log --oneline origin/master..HEAD
|
||||
git diff --stat origin/master...HEAD
|
||||
git status --short --branch
|
||||
```
|
||||
|
||||
Expected: only the design/plan, focused diagnostic implementation, regression
|
||||
tests, canonical documentation, and release note are present; the worktree is
|
||||
clean.
|
||||
|
||||
- [ ] **Step 2: Push the branch**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git push -u origin agent/fix-playback-diagnostic-1159
|
||||
```
|
||||
|
||||
Expected: push succeeds and configures the upstream branch.
|
||||
|
||||
- [ ] **Step 3: Create the ready pull request**
|
||||
|
||||
Create a non-draft PR with title:
|
||||
|
||||
```text
|
||||
fix(playback): avoid false codec diagnostics
|
||||
```
|
||||
|
||||
Use this body:
|
||||
|
||||
```markdown
|
||||
## Summary
|
||||
|
||||
- stop treating an ambiguous native `MediaError` code 4 as proof of an unsupported codec
|
||||
- preserve and show explicit Video.js HTTP error statuses such as 404
|
||||
- keep confirmed container/codec diagnostics and external fallback behavior evidence-based
|
||||
|
||||
## Testing
|
||||
|
||||
- `pnpm nx test ui-playback`
|
||||
- `pnpm nx lint ui-playback`
|
||||
- `pnpm run typecheck:web`
|
||||
- `pnpm run i18n:check`
|
||||
- `pnpm run release:notes:validate`
|
||||
- local Codex review completed with no unresolved P1/P2 findings
|
||||
|
||||
Closes #1159
|
||||
```
|
||||
|
||||
Expected: a ready PR targeting `master`, created only after local review and
|
||||
final validation succeed.
|
||||
@@ -0,0 +1,146 @@
|
||||
# Accurate Native Playback Diagnostics
|
||||
|
||||
## Context
|
||||
|
||||
Issue #1159 reports that an unavailable HLS URL is presented as an unsupported
|
||||
codec. Video.js can surface a failed request as `MediaError` code 4
|
||||
(`MEDIA_ERR_SRC_NOT_SUPPORTED`), but that code does not prove that the source's
|
||||
codec is incompatible. The current native classifier treats every code-4
|
||||
failure that is not a known unsupported container as `unsupported-codec`, so an
|
||||
`.m3u8` request with an HTTP failure receives misleading codec wording and a
|
||||
native-player recommendation.
|
||||
|
||||
The local Video.js boundary also narrows `player.error()` to `code` and
|
||||
`message`, even though Video.js errors may expose an HTTP `status` and
|
||||
structured `metadata`. That prevents the shared diagnostic from retaining an
|
||||
explicit response status such as 404.
|
||||
|
||||
## Goals
|
||||
|
||||
- Never claim that an ambiguous native code-4 failure proves an unsupported
|
||||
codec.
|
||||
- Preserve a valid HTTP error status reported by Video.js and classify it as a
|
||||
provider/network loading failure.
|
||||
- Show the HTTP status prominently and in technical details without adding a
|
||||
new top-level diagnostic code or new translated prose.
|
||||
- Keep confirmed unsupported-container and independently confirmed
|
||||
unsupported-codec diagnostics unchanged.
|
||||
- Add regression coverage for the issue's HLS/404 shape.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Redesign the complete playback diagnostic taxonomy.
|
||||
- Capture the richer hls.js, Shaka, mpegts.js, Embedded MPV, MPV, or VLC error
|
||||
models.
|
||||
- Add confidence levels, per-player fallback likelihood, recoverable warning
|
||||
history, stream probes, or automatic player failover.
|
||||
- Persist or correlate playback attempts across engines.
|
||||
- Infer an HTTP status, CORS failure, codec failure, or provider outage when the
|
||||
runtime does not expose structured evidence.
|
||||
|
||||
## Diagnostic Contract
|
||||
|
||||
Extend the native error input with the safe Video.js fields needed by the
|
||||
classifier:
|
||||
|
||||
- optional numeric `status`;
|
||||
- optional metadata containing a bounded vendor `errorType`.
|
||||
|
||||
Extend `PlaybackDiagnostic` with:
|
||||
|
||||
- optional `httpStatus`;
|
||||
- optional `nativeErrorType`.
|
||||
|
||||
Only integer HTTP error statuses from 400 through 599 are accepted as evidence.
|
||||
Status `0`, missing values, success statuses, redirects, strings, and
|
||||
out-of-range values remain unknown. Arbitrary metadata, response bodies,
|
||||
headers, and URLs are not copied into the diagnostic because they may contain
|
||||
credentials or provider data. A metadata error type is retained only when it
|
||||
matches the vendor-identifier form `[A-Za-z0-9._:-]{1,128}`; every other value
|
||||
is ignored rather than truncated or rendered.
|
||||
|
||||
## Classification Rules
|
||||
|
||||
Native classification uses the following precedence:
|
||||
|
||||
1. An explicit valid HTTP error status produces `network-error`, retains the
|
||||
status, and does not recommend an external player. The same URL and request
|
||||
context are expected to fail independently of the decoder.
|
||||
2. Native code 2 keeps the existing network/browser-access classification.
|
||||
3. Native code 3 keeps the existing media-decode classification.
|
||||
4. Native code 4 remains `unsupported-container` only when source metadata
|
||||
independently identifies a container already known to be unsuitable for the
|
||||
browser path.
|
||||
5. Every other native code-4 failure becomes `unknown-playback-error` and does
|
||||
not recommend an external player. The UI must not substitute a codec or
|
||||
network guess.
|
||||
6. Other native errors keep the existing unknown classification.
|
||||
|
||||
`unsupported-codec` remains available when codec incompatibility is supported
|
||||
by independent evidence, such as the HLS incompatible-codec error details or
|
||||
the existing manifest codec capability check.
|
||||
|
||||
## Video.js Boundary
|
||||
|
||||
The focused `VideoJsPlayer` error type retains `status` and the safe metadata
|
||||
error type instead of narrowing the error to `code` and `message`.
|
||||
`VjsPlayerComponent` continues to pass `player.error()` into the shared native
|
||||
classifier; no Video.js-specific classifier or duplicate UI path is added.
|
||||
|
||||
This PR does not subscribe to additional VHS request events. If Video.js does
|
||||
not expose an HTTP status on its terminal error, the result deliberately stays
|
||||
ambiguous.
|
||||
|
||||
## User Interface
|
||||
|
||||
An HTTP-backed `network-error` uses the existing translated network title and
|
||||
description. Its visible diagnostic metadata shows `HTTP <status>` before
|
||||
container or MIME information, so the issue's unavailable HLS source displays
|
||||
`HTTP 404` rather than `m3u8`. The existing translated “Error details” row
|
||||
combines the safe HTTP status and Video.js error type when present, avoiding
|
||||
new translation keys.
|
||||
|
||||
An ambiguous code-4 failure uses the existing unknown-playback title and
|
||||
description. Because no external fallback is recommended without codec,
|
||||
container, decode, or browser-access evidence, the surface retains retry/copy
|
||||
actions but does not present MPV or VLC as a likely fix.
|
||||
|
||||
No new translation keys or layout changes are required.
|
||||
|
||||
## Testing
|
||||
|
||||
Use test-driven development:
|
||||
|
||||
- Add a classifier regression proving that code 4 plus `.m3u8` and status 404
|
||||
becomes `network-error`, retains `httpStatus: 404`, and does not recommend an
|
||||
external player.
|
||||
- Add a classifier regression proving that code 4 plus `.m3u8` without a valid
|
||||
status becomes `unknown-playback-error`, not `unsupported-codec`.
|
||||
- Prove that status `0` is not presented as an HTTP response.
|
||||
- Keep the existing known-container code-4 case as
|
||||
`unsupported-container`.
|
||||
- Update the Video.js component test to prove that `status` and the safe
|
||||
metadata error type cross the component boundary.
|
||||
- Update focused diagnostic-view tests to prove that `HTTP 404` is visible and
|
||||
the new technical fields are rendered.
|
||||
|
||||
Run the `ui-playback` unit test and lint targets, the workspace typecheck
|
||||
target, i18n drift validation, and release-note validation. No new E2E flow is
|
||||
required because the diagnostic overlay layout and user interaction are
|
||||
unchanged; the classifier, component boundary, and rendered metadata are
|
||||
covered by focused unit/component tests.
|
||||
|
||||
## Documentation And Release Note
|
||||
|
||||
Update `docs/architecture/embedded-inline-playback.md` to document the
|
||||
evidence requirement for native code 4 and preservation of explicit HTTP
|
||||
statuses. Add one `fix` release note under `.changes/` for issue #1159.
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
- **Copy-only fix:** Mapping every code-4 failure to the existing unknown text
|
||||
would remove the false codec claim, but would continue discarding an explicit
|
||||
404 and provide less useful support information.
|
||||
- **Full diagnostic redesign:** Adding the complete HTTP/manifest/decode
|
||||
taxonomy and all player adapters would improve coverage, but it is too broad
|
||||
for the focused regression fix and will be handled in separate work.
|
||||
Reference in new issue
Block a user