feat: error screen with demo handling assets#148
Conversation
📝 WalkthroughWalkthroughThis PR adds an ErrorScreen UI with normalized error details, refactors stream presets to a features/groups model with client-side DRM/codec support probing, integrates asset-based playlist loading into the StreamPanel, and introduces UI primitives (Badge, Item) plus a Separator migration and demo/registry/docs updates. ChangesError Screen, Stream Presets, Asset Integration, and UI Components
Sequence DiagramsequenceDiagram
participant PresetsOverlay
participant initStreamSupport
participant probeDrmSupport
participant computeSupport
participant getStreamSupport
PresetsOverlay->>initStreamSupport: initStreamSupport(allPresets)
initStreamSupport->>probeDrmSupport: probe required DRM features
probeDrmSupport->>initStreamSupport: drm probe results
initStreamSupport->>computeSupport: computeSupport per preset (manifest/codec/DRM)
PresetsOverlay->>getStreamSupport: getStreamSupport(preset) during render
getStreamSupport->>computeSupport: return cached or compute on demand
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a reusable playback ErrorScreen UI component to the Limeplay registry, wires player/asset error state to support richer error objects (including Shaka errors), and upgrades the docs demo stream picker with new presets + capability gating.
Changes:
- Introduces
ErrorScreen+getErrorDetails()with an example demo and docs page. - Stores richer playback errors (Shaka/native/unknown) and updates demo player to render an error overlay with retry/debug actions.
- Expands stream presets and adds browser capability checks (manifest/codec/DRM) to disable unsupported demo streams.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/www/registry/default/ui/error-screen.tsx | New error overlay component + error mapping helpers. |
| apps/www/registry/default/internal/media.ts | Re-exports useMediaApi for internal consumers. |
| apps/www/registry/default/hooks/use-player.ts | Stores Shaka error detail on playback slice and emits playback error event. |
| apps/www/registry/default/hooks/use-playback.ts | Broadens playback.error type and tweaks pause behavior in error state. |
| apps/www/registry/default/hooks/use-asset.ts | Broadens onLoadError signature and adjusts error normalization. |
| apps/www/registry/default/examples/player-root-demo.tsx | Demo: adds asset-based stream loading + renders error overlay with retry/docs link. |
| apps/www/registry/default/examples/error-screen-demo.tsx | New standalone demo for ErrorScreen. |
| apps/www/registry/collection/registry-ui.ts | Registers error-screen as a registry UI item. |
| apps/www/registry/collection/registry-examples.ts | Registers error-screen-demo and updates demo dependencies. |
| apps/www/next-env.d.ts | Formatting-only change to the generated Next env typings import. |
| apps/www/lib/stream-support.ts | New support probing/checking utilities (manifest/codec/DRM). |
| apps/www/lib/stream-presets.ts | Reworks preset catalog into a single grouped preset list with richer metadata. |
| apps/www/content/docs/components/meta.json | Adds error-screen to the components docs sidebar meta. |
| apps/www/content/docs/components/index.mdx | Adds Error Screen to the components index table. |
| apps/www/content/docs/components/error-screen.mdx | New docs page describing installation/usage for Error Screen. |
| apps/www/components/ui/separator.tsx | Migrates Separator implementation to Base UI. |
| apps/www/components/ui/item.tsx | Adds a new Item UI primitive used by Error Screen. |
| apps/www/components/ui/badge.tsx | Adds a new Badge UI primitive used by the stream preset UI. |
| apps/www/components/stream-panel/use-stream-panel-sync.ts | Switches stream loading to useAsset and sets playback error state on load failure. |
| apps/www/components/stream-panel/presets-overlay.tsx | Shows feature badges and disables unsupported presets based on support checks. |
| apps/www/components/stream-panel/panel-popover.tsx | Groups presets by new group field and adjusts layout sizing. |
| import { mergeProps } from "@base-ui/react/merge-props" | ||
| import { useRender } from "@base-ui/react/use-render" | ||
| import { cva, type VariantProps } from "class-variance-authority" | ||
| import * as React from "react" | ||
|
|
| import { mergeProps } from "@base-ui/react/merge-props" | ||
| import { useRender } from "@base-ui/react/use-render" | ||
| import { cva, type VariantProps } from "class-variance-authority" | ||
|
|
||
| import { cn } from "@/lib/utils" | ||
|
|
| import { | ||
| Item, | ||
| ItemActions, | ||
| ItemContent, | ||
| ItemDescription, | ||
| ItemTitle, | ||
| } from "@/components/ui/item" | ||
| import { cn } from "@/lib/utils" |
| onPlayerError?.( | ||
| error instanceof Error | ||
| ? error | ||
| : Object.assign(new Error(String(error)), error as object), |
| if (format === "dash") { | ||
| const hasMse = | ||
| typeof MediaSource !== "undefined" || "ManagedMediaSource" in globalThis | ||
| if (!hasMse) return "DASH requires Media Source Extensions" | ||
| return null | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition | ||
| if (format === "hls") { | ||
| const hasMse = | ||
| typeof MediaSource !== "undefined" || "ManagedMediaSource" in globalThis | ||
|
|
||
| if (hasMse) { | ||
| const hlsSupported = | ||
| MediaSource.isTypeSupported('video/mp4; codecs="avc1.42E01E"') || | ||
| MediaSource.isTypeSupported("video/mp2t") | ||
| if (hlsSupported) return null |
| canPlayThrough: boolean | ||
| ended: boolean | ||
| error: MediaError | null | ||
| error: unknown |
| --- | ||
|
|
||
| <ComponentPreview | ||
| name="error-screen-demo" |
| { | ||
| files: [ | ||
| { | ||
| path: "ui/error-screen.tsx", | ||
| target: `${TARGET_BASE_PATH}/error-screen.tsx`, | ||
| type: "registry:ui", | ||
| }, | ||
| ], | ||
| name: "error-screen", | ||
| registryDependencies: ["media-provider"], | ||
| type: "registry:ui", | ||
| }, |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/www/components/stream-panel/panel-popover.tsx (1)
71-72: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse granular docs-dial selectors here.
Subscribing to the whole store makes this panel rerender on unrelated dial changes and breaks the repo selector rule. Please select the specific fields/actions you use instead of
useDocsDialStore(). As per coding guidelines,Always use granular per-feature selectors like useXxxStore(s => s.field) for state access—never access entire slices.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/www/components/stream-panel/panel-popover.tsx` around lines 71 - 72, The panel currently subscribes to the entire docs-dial store via useDocsDialStore(), causing unnecessary rerenders; replace that call with granular selectors for each piece of state or action the file actually uses (e.g., useDocsDialStore(s => s.currentRepo), useDocsDialStore(s => s.selectedPreset), and useDocsDialStore(s => s.setSelectedRepo) or whatever exact fields/actions this component reads or invokes) and keep getPresetsForType(playerType) as-is; ensure you import/use the selectors where needed so the component only subscribes to those specific fields rather than the whole store to comply with the repo selector rule.apps/www/components/stream-panel/use-stream-panel-sync.ts (1)
60-76:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreset changes currently trigger two loads.
This hook already loads on
presetIdchanges in the effect, sohandlePresetChange()causes a secondloadPlaylist()for every selection path that also updates the docs dial store. Keep one source of truth here, otherwise the player does duplicate work and error handling can race.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/www/components/stream-panel/use-stream-panel-sync.ts` around lines 60 - 76, The hook is calling loadPlaylist twice: once in the useEffect that reacts to presetId and again directly in handlePresetChange; remove the direct call to loadPlaylist from handlePresetChange and instead make handlePresetChange update the shared preset selection (the value that drives presetId) so the existing useEffect (which uses prevPresetId, initialLoadDone, getPresetById and loadPlaylist) remains the single source of truth for loading; alternatively, if handlePresetChange must trigger the change path, have it call the same setter/dispatch that updates presetId rather than invoking loadPlaylist([preset]) directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/www/components/stream-panel/presets-overlay.tsx`:
- Around line 114-119: Currently the preset button uses disabled and
pointer-events-none (class) with title={support.reason}, which prevents the
tooltip from appearing and hides the unsupported reason; update the rendering in
presets-overlay.tsx so the reason is visible or attach the tooltip to a
non-disabled wrapper: either render support.reason as inline text inside the
button (e.g., next to the label when support.supported is false) or move
title/tooltip to a parent element (wrap the button with a span/div that is not
disabled and uses the tooltip trigger) while keeping value={preset.id},
key={preset.id}, and the visual disabled state via className
("pointer-events-none opacity-40") but not the disabled attribute so the wrapper
can show the tooltip. Ensure you update the same pattern used around
support.supported/support.reason for the other occurrence that corresponds to
the second block.
In `@apps/www/components/stream-panel/use-stream-panel-sync.ts`:
- Around line 81-109: handleLoadStream currently uses safeParseJson and proceeds
to call loadPlaylist even when parsing fails; update the flow so malformed JSON
stops the load and surfaces a validation error: make safeParseJson throw or
return an explicit failure indicator, then in handleLoadStream check the parse
result (when config is provided) and if parsing failed, do not build the
StreamPreset or call loadPlaylist but instead surface an error (e.g.,
return/fail caller, set an error state or throw) so callers know the config is
invalid; refer to safeParseJson, handleLoadStream, loadPlaylist and StreamPreset
when making these changes.
In `@apps/www/lib/stream-support.ts`:
- Around line 141-150: The initStreamSupport flow currently returns early if
supportResultsCache or probePromise exists, which skips probing or merging later
preset collections; instead, when called with a new presets array you should:
detect presets not yet present in supportResultsCache (or not covered by a
pending probePromise), call probeAllDrm only for DRM presets that are missing,
and merge results into the existing supportResultsCache rather than
short-circuiting; update the logic around probePromise, supportResultsCache,
probeAllDrm, presets and computeSupport to accumulate new preset ids into
supportResultsCache and ensure computeSupport reads the merged cache so
subsequent calls probe and record support for newly added DRM/audio presets.
- Around line 32-39: The HLS probe calls MediaSource.isTypeSupported unguarded
even when hasMse may be true due to ManagedMediaSource only; update the check
around hlsSupported so you only call MediaSource.isTypeSupported when typeof
MediaSource !== "undefined" (i.e., ensure MediaSource exists), e.g., compute
hlsSupported using a guard like (typeof MediaSource !== "undefined" &&
(MediaSource.isTypeSupported(...))) before returning null, leaving the
ManagedMediaSource-only path untouched; adjust the logic around hasMse,
hlsSupported and the return to avoid ReferenceError when only ManagedMediaSource
is present.
In `@apps/www/registry/collection/registry-examples.ts`:
- Around line 25-35: The registry entry for the example named
"error-screen-demo" is missing its npm dependency metadata; update the object
(the entry with name "error-screen-demo" and files path
"examples/error-screen-demo.tsx") to include a dependencies property listing all
imported npm packages (at minimum "lucide-react") per guidelines (exclude React
and Next.js), e.g., add dependencies: { "lucide-react": "appropriate-version" }
and include any other imports similarly.
In `@apps/www/registry/default/examples/error-screen-demo.tsx`:
- Around line 5-10: The example imports the Button from the app-level path;
update it to use the registry-local import convention: replace the `Button`
import from `@/components/ui/button` with the registry-local path (e.g.,
`@/registry/default/ui/button`) so all example references (such as Button,
alongside existing ErrorScreen, getErrorDetails, and ShakaErrorLike from
`@/registry/default/ui/error-screen`) use `@/registry/default/...` sources.
In `@apps/www/registry/default/ui/error-screen.tsx`:
- Around line 3-6: The ErrorScreen component currently hardcodes a root <div>,
preventing composition; modify ErrorScreen to accept an optional asChild prop
and use Slot from `@radix-ui/react-slot` as the root when asChild is true.
Concretely: import { Slot } from "`@radix-ui/react-slot`", extend the props
signature (reuse ComponentPropsWithoutRef<"div"> and add asChild?: boolean), and
replace the hardcoded <div ...> in the ErrorScreen render with const Root =
asChild ? Slot : "div" (or directly render <Slot> when asChild) so all existing
props, className and children are forwarded to Root; keep existing markup and
behavior but forward props to enable composition.
---
Outside diff comments:
In `@apps/www/components/stream-panel/panel-popover.tsx`:
- Around line 71-72: The panel currently subscribes to the entire docs-dial
store via useDocsDialStore(), causing unnecessary rerenders; replace that call
with granular selectors for each piece of state or action the file actually uses
(e.g., useDocsDialStore(s => s.currentRepo), useDocsDialStore(s =>
s.selectedPreset), and useDocsDialStore(s => s.setSelectedRepo) or whatever
exact fields/actions this component reads or invokes) and keep
getPresetsForType(playerType) as-is; ensure you import/use the selectors where
needed so the component only subscribes to those specific fields rather than the
whole store to comply with the repo selector rule.
In `@apps/www/components/stream-panel/use-stream-panel-sync.ts`:
- Around line 60-76: The hook is calling loadPlaylist twice: once in the
useEffect that reacts to presetId and again directly in handlePresetChange;
remove the direct call to loadPlaylist from handlePresetChange and instead make
handlePresetChange update the shared preset selection (the value that drives
presetId) so the existing useEffect (which uses prevPresetId, initialLoadDone,
getPresetById and loadPlaylist) remains the single source of truth for loading;
alternatively, if handlePresetChange must trigger the change path, have it call
the same setter/dispatch that updates presetId rather than invoking
loadPlaylist([preset]) directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ca5573c-c0de-4dd7-ae1f-b55805d7164e
⛔ Files ignored due to path filters (1)
apps/www/next-env.d.tsis excluded by none and included by none
📒 Files selected for processing (20)
apps/www/components/stream-panel/panel-popover.tsxapps/www/components/stream-panel/presets-overlay.tsxapps/www/components/stream-panel/use-stream-panel-sync.tsapps/www/components/ui/badge.tsxapps/www/components/ui/item.tsxapps/www/components/ui/separator.tsxapps/www/content/docs/components/error-screen.mdxapps/www/content/docs/components/index.mdxapps/www/content/docs/components/meta.jsonapps/www/lib/stream-presets.tsapps/www/lib/stream-support.tsapps/www/registry/collection/registry-examples.tsapps/www/registry/collection/registry-ui.tsapps/www/registry/default/examples/error-screen-demo.tsxapps/www/registry/default/examples/player-root-demo.tsxapps/www/registry/default/hooks/use-asset.tsapps/www/registry/default/hooks/use-playback.tsapps/www/registry/default/hooks/use-player.tsapps/www/registry/default/internal/media.tsapps/www/registry/default/ui/error-screen.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/www/components/stream-panel/use-stream-panel-sync.ts (1)
69-69: ⚡ Quick winAvoid
as unknown as Assetcasts on playlist loads.Lines 69, 75, and 106 bypass static checks. Prefer typing
useAsset/loadPlaylistsoStreamPresetcompatibility is enforced by the compiler.Type-safe direction
-import type { Asset, UseAssetOptions } from "`@/registry/default/hooks/use-asset`" +import type { UseAssetOptions } from "`@/registry/default/hooks/use-asset`" -const assetOptions = useMemo<UseAssetOptions<Asset>>( +const assetOptions = useMemo<UseAssetOptions<StreamPreset>>( () => ({ onLoadError: (_asset, error) => { store.setState(({ playback }) => { playback.status = "error" playback.error = error }) return "stop" }, }), [store] ) -const { loadPlaylist } = useAsset(assetOptions) +const { loadPlaylist } = useAsset<StreamPreset>(assetOptions) - loadPlaylist([preset as unknown as Asset]) + loadPlaylist([preset]) - loadPlaylist([preset as unknown as Asset]) + loadPlaylist([preset]) - loadPlaylist([asset as unknown as Asset]) + loadPlaylist([asset])Also applies to: 75-75, 106-106
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/www/components/stream-panel/use-stream-panel-sync.ts` at line 69, The code is bypassing TypeScript checks by using "as unknown as Asset" when calling loadPlaylist; instead, update the types so the compiler enforces compatibility: either change loadPlaylist's signature (or add an overload) to accept StreamPreset (or a discriminated union) or update useAsset to return the correct Asset shape (or provide a small mapper function convertStreamPresetToAsset(preset: StreamPreset): Asset) and call that before loadPlaylist; remove the double-cast at lines where loadPlaylist([preset as unknown as Asset]) is used so the call is type-safe and validated by the compiler (reference: loadPlaylist, useAsset, StreamPreset, Asset in use-stream-panel-sync.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/www/components/stream-panel/use-stream-panel-sync.ts`:
- Around line 117-123: safeParseJson currently returns parsed JSON without
ensuring it's an object, allowing arrays, null, or primitives to be treated as
Record<string, unknown>; update safeParseJson so after JSON.parse it checks that
the result is non-null, typeof === 'object' and !Array.isArray(value), and only
then return { ok: true, value }; otherwise return an { ok: false, error: 'parsed
JSON is not an object' } (include the original parse error when applicable) to
match the declared return type and prevent passing non-objects into
player.configure().
---
Nitpick comments:
In `@apps/www/components/stream-panel/use-stream-panel-sync.ts`:
- Line 69: The code is bypassing TypeScript checks by using "as unknown as
Asset" when calling loadPlaylist; instead, update the types so the compiler
enforces compatibility: either change loadPlaylist's signature (or add an
overload) to accept StreamPreset (or a discriminated union) or update useAsset
to return the correct Asset shape (or provide a small mapper function
convertStreamPresetToAsset(preset: StreamPreset): Asset) and call that before
loadPlaylist; remove the double-cast at lines where loadPlaylist([preset as
unknown as Asset]) is used so the call is type-safe and validated by the
compiler (reference: loadPlaylist, useAsset, StreamPreset, Asset in
use-stream-panel-sync.ts).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bd9eeb6d-eeb4-4fba-8e07-8653731c54bd
📒 Files selected for processing (4)
apps/www/components/stream-panel/presets-overlay.tsxapps/www/components/stream-panel/use-stream-panel-sync.tsapps/www/lib/stream-support.tsapps/www/registry/collection/registry-examples.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/www/components/stream-panel/presets-overlay.tsx
- apps/www/registry/collection/registry-examples.ts
- apps/www/lib/stream-support.ts
| function safeParseJson( | ||
| str: string | ||
| ): { error: string; ok: false } | { ok: true; value: Record<string, unknown> } { | ||
| try { | ||
| return { ok: true, value: JSON.parse(str) } | ||
| } catch (e) { | ||
| return { error: e instanceof Error ? e.message : String(e), ok: false } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify expected config shape and usage sites.
rg -n --type=ts 'type StreamPreset|interface StreamPreset|config\s*:' apps/www/lib/stream-presets.ts -C3
rg -n --type=ts '\bconfig\b' apps/www/registry/default/hooks/use-asset.ts apps/www/registry/default/internal/media.ts -C3Repository: WINOFFRG/limeplay
Length of output: 2563
🏁 Script executed:
rg -n 'safeParseJson' apps/www/components/stream-panel/ -A5 -B2Repository: WINOFFRG/limeplay
Length of output: 1584
🏁 Script executed:
cat -n apps/www/components/stream-panel/use-stream-panel-sync.ts | sed -n '100,140p'Repository: WINOFFRG/limeplay
Length of output: 803
🏁 Script executed:
rg -n 'config\s*:.*Record|config.*any|config.*unknown' apps/www/components/stream-panel/ -B2 -A2Repository: WINOFFRG/limeplay
Length of output: 43
🏁 Script executed:
sed -n '82,115p' apps/www/components/stream-panel/use-stream-panel-sync.ts | cat -nRepository: WINOFFRG/limeplay
Length of output: 1149
🏁 Script executed:
rg -n 'parsedConfig' apps/www/components/stream-panel/use-stream-panel-sync.ts -B1 -A3Repository: WINOFFRG/limeplay
Length of output: 509
Validate that parsed config is a JSON object, not just valid JSON.
Line 121 currently accepts arrays, primitives, and null as valid config, but the return type signature promises Record<string, unknown>. This violates the type contract and can cause runtime issues downstream when the config is passed to player.configure(), which expects an object.
The proposed fix correctly validates that the parsed value is an object:
Fix
function safeParseJson(
str: string
): { error: string; ok: false } | { ok: true; value: Record<string, unknown> } {
try {
- return { ok: true, value: JSON.parse(str) }
+ const value: unknown = JSON.parse(str)
+ if (!value || typeof value !== "object" || Array.isArray(value)) {
+ return { error: "Config must be a JSON object", ok: false }
+ }
+ return { ok: true, value: value as Record<string, unknown> }
} catch (e) {
return { error: e instanceof Error ? e.message : String(e), ok: false }
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/www/components/stream-panel/use-stream-panel-sync.ts` around lines 117 -
123, safeParseJson currently returns parsed JSON without ensuring it's an
object, allowing arrays, null, or primitives to be treated as Record<string,
unknown>; update safeParseJson so after JSON.parse it checks that the result is
non-null, typeof === 'object' and !Array.isArray(value), and only then return {
ok: true, value }; otherwise return an { ok: false, error: 'parsed JSON is not
an object' } (include the original parse error when applicable) to match the
declared return type and prevent passing non-objects into player.configure().
Summary by CodeRabbit
New Features
Bug Fixes
Documentation