From cd86b1db7dfa605c17ff38f52fc330e022bf2b68 Mon Sep 17 00:00:00 2001 From: dazzatronus Date: Thu, 24 Sep 2026 22:44:00 +1000 Subject: [PATCH] fix: report clips that fall back to a placeholder --- src/components/canvas/players/image-player.ts | 7 +++- src/components/canvas/players/player.ts | 11 +++++ src/components/canvas/players/svg-player.ts | 37 +++++++++++++---- src/components/canvas/players/video-player.ts | 3 ++ src/core/edit-session.ts | 35 +++++++++++----- src/core/player-reconciler.ts | 15 +++++++ tests/edit-load.test.ts | 31 ++++++++++++++ tests/svg-player.test.ts | 40 +++++++++++++++++++ 8 files changed, 159 insertions(+), 20 deletions(-) diff --git a/src/components/canvas/players/image-player.ts b/src/components/canvas/players/image-player.ts index eea72c5c..8ba4ab8b 100644 --- a/src/components/canvas/players/image-player.ts +++ b/src/components/canvas/players/image-player.ts @@ -25,7 +25,8 @@ export class ImagePlayer extends Player { try { await this.loadTexture(); this.configureKeyframes(); - } catch { + } catch (error) { + this.recordLoadError(error); this.createFallbackGraphic(); } } @@ -76,10 +77,12 @@ export class ImagePlayer extends Player { public override async reloadAsset(): Promise { this.disposeTexture(); this.clearPlaceholder(); + this.loadError = null; try { await this.loadTexture(); - } catch { + } catch (error) { + this.recordLoadError(error); this.createFallbackGraphic(); } } diff --git a/src/components/canvas/players/player.ts b/src/components/canvas/players/player.ts index 37e100a8..bda30a07 100644 --- a/src/components/canvas/players/player.ts +++ b/src/components/canvas/players/player.ts @@ -76,6 +76,12 @@ export abstract class Player extends Entity { /** True when the player's asset needs external resolution (e.g. alias caption awaiting transcription). */ public needsResolution = false; + /** + * Why the asset couldn't be shown, or null. A failing player draws a placeholder and still + * resolves load() so the rest of the edit keeps loading; this is how the failure is reported. + */ + public loadError: string | null = null; + protected edit: Edit; public clipConfiguration: ResolvedClip; @@ -231,7 +237,12 @@ export abstract class Player extends Entity { } } + protected recordLoadError(error: unknown): void { + this.loadError = error instanceof Error ? error.message : String(error); + } + public override async load(): Promise { + this.loadError = null; if (this.lumaWrapper?.destroyed) { this.lumaWrapper = new pixi.Container(); this.getContainer().addChild(this.lumaWrapper); diff --git a/src/components/canvas/players/svg-player.ts b/src/components/canvas/players/svg-player.ts index 54b027fa..65d38dbb 100644 --- a/src/components/canvas/players/svg-player.ts +++ b/src/components/canvas/players/svg-player.ts @@ -15,6 +15,7 @@ export class SvgPlayer extends Player { private renderedWidth: number = 0; private renderedHeight: number = 0; private pendingRender: Promise | null = null; + private placeholder: pixi.Graphics | null = null; constructor(edit: Edit, clipConfiguration: ResolvedClip) { super(edit, clipConfiguration, PlayerType.Svg); @@ -53,6 +54,7 @@ export class SvgPlayer extends Player { try { const validationResult = SvgAssetSchema.safeParse(svgAsset); if (!validationResult.success) { + this.recordLoadError(`Invalid svg asset: ${validationResult.error.issues.map(i => i.message).join("; ")}`); this.createFallbackGraphic(); return; } @@ -62,26 +64,42 @@ export class SvgPlayer extends Player { this.configureKeyframes(); } catch (error) { console.error("Failed to render SVG asset:", error); + this.recordLoadError(error); this.createFallbackGraphic(); } } public override async reloadAsset(): Promise { - await this.rerenderAtCurrentDimensions(); + this.loadError = null; + try { + await this.rerenderAtCurrentDimensions(); + } catch (error) { + console.error("Failed to render SVG asset:", error); + this.recordLoadError(error); + this.createFallbackGraphic(); + } } private createFallbackGraphic(): void { const width = this.clipConfiguration.width || this.edit.size.width; const height = this.clipConfiguration.height || this.edit.size.height; - const graphics = createPlaceholderGraphic(width, height); + this.clearPlaceholder(); + this.placeholder = createPlaceholderGraphic(width, height); this.renderedWidth = width; this.renderedHeight = height; - this.contentContainer.addChild(graphics); + this.contentContainer.addChild(this.placeholder); this.configureKeyframes(); } + private clearPlaceholder(): void { + if (!this.placeholder) return; + this.contentContainer.removeChild(this.placeholder); + this.placeholder.destroy(); + this.placeholder = null; + } + public override update(deltaTime: number, elapsed: number): void { super.update(deltaTime, elapsed); } @@ -90,6 +108,7 @@ export class SvgPlayer extends Player { super.dispose(); this.pendingRender = null; + this.placeholder = null; if (this.sprite) { this.sprite.destroy(); @@ -136,9 +155,9 @@ export class SvgPlayer extends Player { } private async rerenderAtCurrentDimensions(): Promise { - // Wait for any pending render to complete + // Wait for any pending render to complete; its failure is reported by whoever started it. if (this.pendingRender) { - await this.pendingRender; + await this.pendingRender.catch(() => undefined); } // Clean up old sprite/texture @@ -154,8 +173,12 @@ export class SvgPlayer extends Player { // Start new render this.pendingRender = this.doRender(); - await this.pendingRender; - this.pendingRender = null; + try { + await this.pendingRender; + } finally { + this.pendingRender = null; + } + this.clearPlaceholder(); } private async doRender(): Promise { diff --git a/src/components/canvas/players/video-player.ts b/src/components/canvas/players/video-player.ts index 353cf861..848acafb 100644 --- a/src/components/canvas/players/video-player.ts +++ b/src/components/canvas/players/video-player.ts @@ -44,6 +44,7 @@ export class VideoPlayer extends Player { this.configureKeyframes(); } catch (error) { console.warn(`[VideoPlayer.load] FAILED clipId=${this.clipId}:`, error); + this.recordLoadError(error); this.createFallbackGraphic(); } finally { this.skipVideoUpdate = false; @@ -157,12 +158,14 @@ export class VideoPlayer extends Player { this.syncTimer = 0; this.activeSyncTimer = 0; + this.loadError = null; try { this.disposeVideo(); this.clearPlaceholder(); await this.loadVideo(); } catch (error) { console.warn(`[VideoPlayer.reloadAsset] FAILED clipId=${this.clipId}:`, error); + this.recordLoadError(error); this.createFallbackGraphic(); } finally { this.skipVideoUpdate = false; diff --git a/src/core/edit-session.ts b/src/core/edit-session.ts index 2ff39b15..f15fd81b 100644 --- a/src/core/edit-session.ts +++ b/src/core/edit-session.ts @@ -887,6 +887,10 @@ export class Edit { * @internal */ public getClipError(trackIdx: number, clipIdx: number): { error: string; assetType: string } | null { + const player = this.getPlayerClip(trackIdx, clipIdx); + if (player?.loadError) { + return { error: player.loadError, assetType: (player.clipConfiguration.asset as { type?: string })?.type ?? "unknown" }; + } const recorded = this.clipErrors.get(`${trackIdx}-${clipIdx}`); if (recorded) return recorded; const asset = this.getResolvedClip(trackIdx, clipIdx)?.asset; @@ -1741,18 +1745,27 @@ export class Edit { this.addPlayerToContainer(trackIdx, clip); - clip.load().catch(error => { - // Capture load errors for restored clips (same pattern as initial load) - const assetType = (clip.clipConfiguration?.asset as { type?: string })?.type ?? "unknown"; - const errorMessage = error instanceof Error ? error.message : String(error); - this.clipErrors.set(`${trackIdx}-${insertIdx}`, { error: errorMessage, assetType }); - this.internalEvents.emit(EditEvent.ClipLoadFailed, { - trackIndex: trackIdx, - clipIndex: insertIdx, - error: errorMessage, - assetType + clip + .load() + .then(() => { + if (!clip.loadError) return; + const indices = this.findClipIndices(clip); + if (!indices) return; + const assetType = (clip.clipConfiguration?.asset as { type?: string })?.type ?? "unknown"; + this.internalEvents.emit(EditEvent.ClipLoadFailed, { ...indices, error: clip.loadError, assetType }); + }) + .catch(error => { + // Capture load errors for restored clips (same pattern as initial load) + const assetType = (clip.clipConfiguration?.asset as { type?: string })?.type ?? "unknown"; + const errorMessage = error instanceof Error ? error.message : String(error); + this.clipErrors.set(`${trackIdx}-${insertIdx}`, { error: errorMessage, assetType }); + this.internalEvents.emit(EditEvent.ClipLoadFailed, { + trackIndex: trackIdx, + clipIndex: insertIdx, + error: errorMessage, + assetType + }); }); - }); this.updateTotalDuration(); }, diff --git a/src/core/player-reconciler.ts b/src/core/player-reconciler.ts index 04795627..99be26ed 100644 --- a/src/core/player-reconciler.ts +++ b/src/core/player-reconciler.ts @@ -210,6 +210,8 @@ export class PlayerReconciler { const loadPromise = player .load() .then(() => { + if (player.loadError) this.reportLoadFailure(player, player.loadError); + // Emit PlayerLoaded for all players this.edit.getInternalEvents().emit(InternalEvent.PlayerLoaded, { player, @@ -345,6 +347,17 @@ export class PlayerReconciler { player.reconfigureAfterRestore(); } + /** + * Announce a player that couldn't show its asset. Its position is looked up now, not when the + * load started, because clips can move while an asset is still loading. + */ + private reportLoadFailure(player: Player, error: string): void { + const indices = this.edit.findClipIndices(player); + if (!indices) return; + const assetType = (player.clipConfiguration.asset as { type?: string })?.type ?? "unknown"; + this.edit.getInternalEvents().emit(EditEvent.ClipLoadFailed, { ...indices, error, assetType }); + } + /** * Check if asset properties changed (excluding type, which is handled separately). */ @@ -376,9 +389,11 @@ export class PlayerReconciler { .reloadAsset() .then(() => { player.reconfigureAfterRestore(); + if (player.loadError) this.reportLoadFailure(player, player.loadError); }) .catch(error => { console.error("Failed to reload asset:", error); + this.reportLoadFailure(player, error instanceof Error ? error.message : String(error)); }); } else { player.reconfigureAfterRestore(); diff --git a/tests/edit-load.test.ts b/tests/edit-load.test.ts index 13085a02..2137904c 100644 --- a/tests/edit-load.test.ts +++ b/tests/edit-load.test.ts @@ -1271,6 +1271,37 @@ describe("Edit loadEdit()", () => { const { tracks } = getEditState(edit); expect(tracks[0].length).toBe(2); }); + + it("reports a clip whose player fell back to a placeholder", async () => { + const loadFailedHandler = jest.fn(); + events.on("clip:loadFailed", loadFailedHandler); + + // Real players draw a placeholder and resolve load() rather than rejecting. + const { ImagePlayer: ImagePlayerMock } = jest.requireMock("@canvas/players/image-player"); + ImagePlayerMock.mockImplementationOnce((editInstance: Edit, config: ResolvedClip) => { + const player = createMockPlayer(editInstance, config, PlayerType.Image); + player["load"] = jest.fn(async () => { + player["loadError"] = "Invalid image source 'bad.jpg'."; + }); + return player; + }); + + await edit.loadEdit( + createMinimalEdit([ + { + clips: [ + { asset: { type: "image", src: "https://example.com/bad.jpg" }, start: 0, length: 3, fit: "crop" }, + { asset: { type: "image", src: "https://example.com/good.jpg" }, start: 3, length: 3, fit: "crop" } + ] + } + ]) + ); + + expect(loadFailedHandler).toHaveBeenCalledWith( + expect.objectContaining({ trackIndex: 0, clipIndex: 0, error: "Invalid image source 'bad.jpg'.", assetType: "image" }) + ); + expect(edit.getClipError(0, 0)).toEqual({ error: "Invalid image source 'bad.jpg'.", assetType: "image" }); + }); }); }); diff --git a/tests/svg-player.test.ts b/tests/svg-player.test.ts index 321edcfb..f89bc49f 100644 --- a/tests/svg-player.test.ts +++ b/tests/svg-player.test.ts @@ -468,6 +468,46 @@ describe("SvgPlayer", () => { consoleSpy.mockRestore(); }); + it("records why it fell back when rendering fails", async () => { + mockRenderSvgAssetToPng.mockRejectedValueOnce(new Error("Render failed")); + const player = new SvgPlayer(createMockEdit(), createSvgClipConfig()); + const consoleSpy = jest.spyOn(console, "error").mockImplementation(); + + await player.load(); + + expect(player.loadError).toBe("Render failed"); + consoleSpy.mockRestore(); + }); + + it("records why it fell back when validation fails", async () => { + const player = new SvgPlayer(createMockEdit(), createInvalidSvgClipConfig()); + + await player.load(); + + expect(player.loadError).toEqual(expect.any(String)); + }); + + it("has no load error after a successful render", async () => { + const player = new SvgPlayer(createMockEdit(), createSvgClipConfig()); + + await player.load(); + + expect(player.loadError).toBeNull(); + }); + + it("falls back and records the error when a re-render fails", async () => { + const player = new SvgPlayer(createMockEdit(), createSvgClipConfig()); + await player.load(); + mockRenderSvgAssetToPng.mockRejectedValueOnce(new Error("Render failed")); + const consoleSpy = jest.spyOn(console, "error").mockImplementation(); + + await expect(player.reloadAsset()).resolves.toBeUndefined(); + + expect(createPlaceholderGraphic).toHaveBeenCalled(); + expect(player.loadError).toBe("Render failed"); + consoleSpy.mockRestore(); + }); + it("creates fallback graphic when WASM init fails", async () => { // Canvas's initResvg handles the fetch internally; we simulate a // failure by making the SDK-side initResvg call reject.