From 7d3497438b24fcf307e594e0b60d9466f7360a56 Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Wed, 26 Aug 2026 20:40:30 -0700 Subject: [PATCH 1/6] fix(export): drain in-flight encodes on errors --- .../export/utils/pipelined-frame-loop.test.ts | 75 +++++++++++++ .../export/utils/pipelined-frame-loop.ts | 104 ++++++++++-------- 2 files changed, 135 insertions(+), 44 deletions(-) diff --git a/src/features/export/utils/pipelined-frame-loop.test.ts b/src/features/export/utils/pipelined-frame-loop.test.ts index e4e503dce..3d8298808 100644 --- a/src/features/export/utils/pipelined-frame-loop.test.ts +++ b/src/features/export/utils/pipelined-frame-loop.test.ts @@ -182,6 +182,81 @@ describe('runPipelinedFrameLoop', () => { expect(samples[1]?.closed).toBe(true) }) + it('preserves a render error while observing a late encoder rejection', async () => { + const renderError = new Error('render failed') + const encoderError = new Error('encoder failed after render') + const encode = deferred() + const unhandledRejections: unknown[] = [] + const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) + process.on('unhandledRejection', onUnhandledRejection) + + try { + const { events, samples, run } = createHarness(3, { + renderImpl: (frame) => { + if (frame === 1) throw renderError + }, + encodeImpl: () => encode.promise, + }) + + const outcome = run().then( + () => null, + (error: unknown) => error, + ) + await tick() + expect(events).toContain('render-1') + + encode.reject(encoderError) + expect(await outcome).toBe(renderError) + await tick() + expect(unhandledRejections).toEqual([]) + expect(samples[0]?.closed).toBe(true) + } finally { + process.off('unhandledRejection', onUnhandledRejection) + } + }) + + it('preserves a pending error while observing a late encoder rejection', async () => { + const pendingError = new Error('audio task failed') + const encoderError = new Error('encoder failed after pending error') + const encodes: Deferred[] = [] + const unhandledRejections: unknown[] = [] + const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) + process.on('unhandledRejection', onUnhandledRejection) + let raised: unknown + + try { + const { events, samples, run } = createHarness(4, { + getPendingError: () => raised, + renderImpl: (frame) => { + if (frame === 1) raised = pendingError + }, + encodeImpl: () => { + const encode = deferred() + encodes.push(encode) + return encode.promise + }, + }) + + const outcome = run().then( + () => null, + (error: unknown) => error, + ) + await tick() + expect(events).toContain('render-1') + encodes[0]?.resolve() + await tick() + expect(events).toContain('encode-start-1') + + encodes[1]?.reject(encoderError) + expect(await outcome).toBe(pendingError) + await tick() + expect(unhandledRejections).toEqual([]) + expect(samples[1]?.closed).toBe(true) + } finally { + process.off('unhandledRejection', onUnhandledRejection) + } + }) + it('honours an abort signalled before the loop starts', async () => { const controller = new AbortController() controller.abort() diff --git a/src/features/export/utils/pipelined-frame-loop.ts b/src/features/export/utils/pipelined-frame-loop.ts index 583a5fb36..611b152ee 100644 --- a/src/features/export/utils/pipelined-frame-loop.ts +++ b/src/features/export/utils/pipelined-frame-loop.ts @@ -7,10 +7,9 @@ * previous encode has drained, so frames reach the encoder in order. * * Behavior must stay bit-identical to the original inline loop — this is the - * export hot path. Known pre-existing hole kept on purpose: when the loop - * exits via a non-abort error (renderFrame throw or pending error) while an - * encode is in flight, that encode promise is never awaited; only the abort - * path drains it. + * export hot path. Every exit drains an in-flight encode so its sample closes + * and its rejection is observed. A render or pending error remains the primary + * error when that drain also fails. */ export interface CloseableSample { @@ -65,56 +64,73 @@ export async function runPipelinedFrameLoop( let pendingEncode: Promise | null = null - for (let frame = 0; frame < totalFrames; frame++) { - const pendingError = getPendingError?.() - if (pendingError) throw pendingError + const drainPendingEncode = async () => { + if (!pendingEncode) return + const encode = pendingEncode + try { + await encode + } finally { + pendingEncode = null + } + } + + try { + for (let frame = 0; frame < totalFrames; frame++) { + const pendingError = getPendingError?.() + if (pendingError) throw pendingError - // Check for abort — drain any in-flight encode first so the encoder - // is idle before we cancel the output. Discard encoder errors since - // we are aborting anyway and must always surface AbortError. - if (signal?.aborted) { - if (pendingEncode) { + // Check for abort — drain any in-flight encode first so the encoder + // is idle before we cancel the output. Discard encoder errors since + // we are aborting anyway and must always surface AbortError. + if (signal?.aborted) { try { - await pendingEncode + await drainPendingEncode() } catch { /* discarded — aborting */ } + await onAbort() + throw new DOMException('Render cancelled', 'AbortError') } - await onAbort() - throw new DOMException('Render cancelled', 'AbortError') - } - // Render frame first — this overlaps with the previous frame's encode - // that is still in flight. The previous sample already copied its - // pixels, so writing to the capture surface here cannot corrupt it. - await renderFrame(frame) + // Render frame first — this overlaps with the previous frame's encode + // that is still in flight. The previous sample already copied its + // pixels, so writing to the capture surface here cannot corrupt it. + await renderFrame(frame) - // Now wait for the previous encode to finish before capturing a new - // sample. This ensures at most one encode is in flight and that frames - // are fed to the encoder in order. - if (pendingEncode) await pendingEncode + // Now wait for the previous encode to finish before capturing a new + // sample. This ensures at most one encode is in flight and that frames + // are fed to the encoder in order. + await drainPendingEncode() - // Snapshot pixels into a sample. The capture copies pixel data - // immediately — the surface is free for the next render. - const sample = captureSample(frame) + // Snapshot pixels into a sample. The capture copies pixel data + // immediately — the surface is free for the next render. + const sample = captureSample(frame) - // Kick off encoding in the background. NOT awaited here — it runs - // concurrently with the next iteration's renderFrame(). - const isKeyFrame = frame === 0 - pendingEncode = (async () => { - try { - await encodeSample(sample, isKeyFrame) - } finally { - // The encoder does NOT close samples. We must close to release the - // underlying frame's GPU memory, otherwise the browser throttles - // after ~8-16 outstanding frames. - sample.close() - } - })() + // Kick off encoding in the background. NOT awaited here — it runs + // concurrently with the next iteration's renderFrame(). + const isKeyFrame = frame === 0 + pendingEncode = (async () => { + try { + await encodeSample(sample, isKeyFrame) + } finally { + // The encoder does NOT close samples. We must close to release the + // underlying frame's GPU memory, otherwise the browser throttles + // after ~8-16 outstanding frames. + sample.close() + } + })() - onFrameProgress(frame) - } + onFrameProgress(frame) + } - // Drain the final in-flight encode before finalizing - if (pendingEncode) await pendingEncode + // Drain the final in-flight encode before finalizing + await drainPendingEncode() + } catch (primaryError) { + try { + await drainPendingEncode() + } catch { + // Preserve the error that selected this exit path. + } + throw primaryError + } } From e80e35b946811d571dca4d20b17d29c2428917cf Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Wed, 26 Aug 2026 21:12:42 -0700 Subject: [PATCH 2/6] fix(export): observe encode failures immediately --- .../export/utils/pipelined-frame-loop.test.ts | 165 ++++++++++++++++++ .../export/utils/pipelined-frame-loop.ts | 122 ++++++++++--- 2 files changed, 264 insertions(+), 23 deletions(-) diff --git a/src/features/export/utils/pipelined-frame-loop.test.ts b/src/features/export/utils/pipelined-frame-loop.test.ts index 3d8298808..5a1c02d6b 100644 --- a/src/features/export/utils/pipelined-frame-loop.test.ts +++ b/src/features/export/utils/pipelined-frame-loop.test.ts @@ -39,6 +39,7 @@ interface HarnessOptions { getPendingError?: () => unknown renderImpl?: (frame: number) => void | Promise encodeImpl?: (sample: FakeSample, keyFrame: boolean) => Promise + closeImpl?: (sample: FakeSample) => void } function createHarness(totalFrames: number, opts: HarnessOptions = {}) { @@ -61,6 +62,7 @@ function createHarness(totalFrames: number, opts: HarnessOptions = {}) { close() { this.closed = true events.push(`close-${frame}`) + opts.closeImpl?.(this) }, } samples.push(sample) @@ -182,6 +184,125 @@ describe('runPipelinedFrameLoop', () => { expect(samples[1]?.closed).toBe(true) }) + it('observes an immediate encode rejection while the next render stays pending', async () => { + const encoderError = new Error('encoder rejected immediately') + const nextRender = deferred() + const unhandledRejections: unknown[] = [] + const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) + process.on('unhandledRejection', onUnhandledRejection) + let outcome: Promise | undefined + + try { + const { events, samples, run } = createHarness(2, { + renderImpl: (frame) => (frame === 1 ? nextRender.promise : undefined), + encodeImpl: (sample) => + sample.frame === 0 ? Promise.reject(encoderError) : Promise.resolve(), + }) + + outcome = run().then( + () => null, + (error: unknown) => error, + ) + await tick() + await tick() + + expect(events).toContain('render-1') + expect(unhandledRejections).toEqual([]) + + nextRender.resolve() + expect(await outcome).toBe(encoderError) + expect(samples[0]?.closed).toBe(true) + } finally { + nextRender.resolve() + await outcome + process.off('unhandledRejection', onUnhandledRejection) + } + }) + + it('preserves an earlier audio error when video rejects during a pending render', async () => { + const audioError = new Error('audio task failed first') + const encoderError = new Error('video encoder failed later') + const encode = deferred() + const nextRender = deferred() + const unhandledRejections: unknown[] = [] + const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) + process.on('unhandledRejection', onUnhandledRejection) + let pendingError: unknown + let outcome: Promise | undefined + + try { + const { events, samples, run } = createHarness(2, { + getPendingError: () => pendingError, + renderImpl: (frame) => (frame === 1 ? nextRender.promise : undefined), + encodeImpl: () => encode.promise, + }) + + outcome = run().then( + () => null, + (error: unknown) => error, + ) + await tick() + expect(events).toContain('render-1') + + pendingError = audioError + encode.reject(encoderError) + await tick() + expect(unhandledRejections).toEqual([]) + + nextRender.resolve() + expect(await outcome).toBe(audioError) + expect(samples[0]?.closed).toBe(true) + } finally { + nextRender.resolve() + encode.resolve() + await outcome + process.off('unhandledRejection', onUnhandledRejection) + } + }) + + it('preserves the video error when sample cleanup and rendering fail later', async () => { + const encoderError = new Error('video encoder failed first') + const cleanupError = new Error('sample cleanup failed later') + const renderError = new Error('render failed last') + const encode = deferred() + const nextRender = deferred() + const unhandledRejections: unknown[] = [] + const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) + process.on('unhandledRejection', onUnhandledRejection) + let outcome: Promise | undefined + + try { + const { events, samples, run } = createHarness(2, { + renderImpl: (frame) => (frame === 1 ? nextRender.promise : undefined), + encodeImpl: () => encode.promise, + closeImpl: () => { + throw cleanupError + }, + }) + + outcome = run().then( + () => null, + (error: unknown) => error, + ) + await tick() + expect(events).toContain('render-1') + + encode.reject(encoderError) + await tick() + nextRender.reject(renderError) + + expect(await outcome).toBe(encoderError) + await tick() + expect(unhandledRejections).toEqual([]) + expect(samples[0]?.closed).toBe(true) + } finally { + encode.resolve() + nextRender.resolve() + await outcome + process.off('unhandledRejection', onUnhandledRejection) + } + }) + it('preserves a render error while observing a late encoder rejection', async () => { const renderError = new Error('render failed') const encoderError = new Error('encoder failed after render') @@ -311,6 +432,50 @@ describe('runPipelinedFrameLoop', () => { expect(events).not.toContain('capture-2') }) + it('observes and drains a pending encode when abort wins during rendering', async () => { + const controller = new AbortController() + const encoderError = new Error('encoder failed after abort') + const encode = deferred() + const nextRender = deferred() + const unhandledRejections: unknown[] = [] + const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) + process.on('unhandledRejection', onUnhandledRejection) + let outcome: Promise | undefined + + try { + const { events, samples, run } = createHarness(2, { + signal: controller.signal, + renderImpl: (frame) => (frame === 1 ? nextRender.promise : undefined), + encodeImpl: () => encode.promise, + }) + + outcome = run().then( + () => null, + (error: unknown) => error, + ) + await tick() + expect(events).toContain('render-1') + + controller.abort() + encode.reject(encoderError) + await tick() + expect(unhandledRejections).toEqual([]) + + nextRender.resolve() + const error = await outcome + expect(error).toBeInstanceOf(DOMException) + expect((error as DOMException).name).toBe('AbortError') + expect(events).toContain('abort-cancel') + expect(samples[0]?.closed).toBe(true) + } finally { + controller.abort() + encode.resolve() + nextRender.resolve() + await outcome + process.off('unhandledRejection', onUnhandledRejection) + } + }) + it('throws a pending error at the top of the next iteration', async () => { const pendingError = new Error('audio task failed') let raised: unknown diff --git a/src/features/export/utils/pipelined-frame-loop.ts b/src/features/export/utils/pipelined-frame-loop.ts index 611b152ee..6b9023ba9 100644 --- a/src/features/export/utils/pipelined-frame-loop.ts +++ b/src/features/export/utils/pipelined-frame-loop.ts @@ -8,8 +8,9 @@ * * Behavior must stay bit-identical to the original inline loop — this is the * export hot path. Every exit drains an in-flight encode so its sample closes - * and its rejection is observed. A render or pending error remains the primary - * error when that drain also fails. + * and its rejection is observed. Failures retain their occurrence order: a + * render, pending audio, or abort failure that happens first is not replaced + * by a later encode or cleanup failure, and vice versa. */ export interface CloseableSample { @@ -41,13 +42,22 @@ export interface PipelinedFrameLoopDeps { encodeSample: (sample: S, keyFrame: boolean) => Promise /** * Abort path: called after the in-flight encode has been drained (its - * errors discarded), before the AbortError is thrown. + * errors observed), before the selected failure is thrown. */ onAbort: () => Promise /** Called once per frame, synchronously after its encode is kicked off. */ onFrameProgress: (frame: number) => void } +interface EncodeSettlement { + status: 'fulfilled' | 'rejected' + reason?: unknown +} + +interface RecordedFailure { + error: unknown +} + export async function runPipelinedFrameLoop( deps: PipelinedFrameLoopDeps, ): Promise { @@ -62,7 +72,37 @@ export async function runPipelinedFrameLoop( onFrameProgress, } = deps - let pendingEncode: Promise | null = null + // The promise stored here never rejects. Encode and sample-cleanup failures + // are reflected into a settlement immediately, so an encoder rejection is + // observed even while renderFrame remains pending for another event turn. + let pendingEncode: Promise | null = null + let firstFailure: RecordedFailure | null = null + let abortError: DOMException | null = null + let abortCleanupStarted = false + + const recordFailure = (error: unknown) => { + firstFailure ??= { error } + } + + const getAbortError = () => { + abortError ??= new DOMException('Render cancelled', 'AbortError') + return abortError + } + + const recordEncodeFailure = (error: unknown) => { + // A pending audio error or abort may have happened while renderFrame was + // still pending, before this encode rejected. Observe those primary exit + // conditions before recording the later encoder failure. + try { + const pendingError = getPendingError?.() + if (pendingError) recordFailure(pendingError) + } catch (pendingError) { + recordFailure(pendingError) + } + + if (signal?.aborted) recordFailure(getAbortError()) + recordFailure(error) + } const drainPendingEncode = async () => { if (!pendingEncode) return @@ -74,22 +114,37 @@ export async function runPipelinedFrameLoop( } } + const runAbortCleanup = async () => { + if (abortCleanupStarted) return + abortCleanupStarted = true + try { + await onAbort() + } catch (error) { + recordFailure(error) + } + } + + const throwFirstFailure = (): void => { + const failure = firstFailure + if (failure) throw failure.error + } + try { for (let frame = 0; frame < totalFrames; frame++) { const pendingError = getPendingError?.() - if (pendingError) throw pendingError + if (pendingError) { + recordFailure(pendingError) + throw pendingError + } - // Check for abort — drain any in-flight encode first so the encoder - // is idle before we cancel the output. Discard encoder errors since - // we are aborting anyway and must always surface AbortError. + // Check for abort — drain any in-flight encode first so the encoder is + // idle before we cancel the output. The first recorded failure wins, so + // this AbortError is preserved over an encoder failure during the drain. if (signal?.aborted) { - try { - await drainPendingEncode() - } catch { - /* discarded — aborting */ - } - await onAbort() - throw new DOMException('Render cancelled', 'AbortError') + recordFailure(getAbortError()) + await drainPendingEncode() + await runAbortCleanup() + throwFirstFailure() } // Render frame first — this overlaps with the previous frame's encode @@ -101,6 +156,10 @@ export async function runPipelinedFrameLoop( // sample. This ensures at most one encode is in flight and that frames // are fed to the encoder in order. await drainPendingEncode() + if (firstFailure) { + if (signal?.aborted) await runAbortCleanup() + throwFirstFailure() + } // Snapshot pixels into a sample. The capture copies pixel data // immediately — the surface is free for the next render. @@ -109,15 +168,30 @@ export async function runPipelinedFrameLoop( // Kick off encoding in the background. NOT awaited here — it runs // concurrently with the next iteration's renderFrame(). const isKeyFrame = frame === 0 - pendingEncode = (async () => { + pendingEncode = (async (): Promise => { + let failure: RecordedFailure | null = null try { await encodeSample(sample, isKeyFrame) - } finally { + } catch (error) { + failure = { error } + recordEncodeFailure(error) + } + + try { // The encoder does NOT close samples. We must close to release the // underlying frame's GPU memory, otherwise the browser throttles // after ~8-16 outstanding frames. sample.close() + } catch (error) { + // If encoding already failed, it happened before cleanup and remains + // the failure represented by this settlement. + if (!failure) { + failure = { error } + recordFailure(error) + } } + + return failure ? { status: 'rejected', reason: failure.error } : { status: 'fulfilled' } })() onFrameProgress(frame) @@ -125,12 +199,14 @@ export async function runPipelinedFrameLoop( // Drain the final in-flight encode before finalizing await drainPendingEncode() - } catch (primaryError) { - try { - await drainPendingEncode() - } catch { - // Preserve the error that selected this exit path. + if (firstFailure) { + if (signal?.aborted) await runAbortCleanup() + throwFirstFailure() } - throw primaryError + } catch (primaryError) { + recordFailure(primaryError) + await drainPendingEncode() + if (signal?.aborted) await runAbortCleanup() + throwFirstFailure() } } From 977473c9b7d78321687cb7c8c69edd94f1fa261d Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Wed, 26 Aug 2026 21:18:16 -0700 Subject: [PATCH 3/6] refactor(export): keep encode loop complexity bounded --- .../export/utils/pipelined-frame-loop.ts | 87 ++++++++++--------- 1 file changed, 48 insertions(+), 39 deletions(-) diff --git a/src/features/export/utils/pipelined-frame-loop.ts b/src/features/export/utils/pipelined-frame-loop.ts index 6b9023ba9..ad7dcb243 100644 --- a/src/features/export/utils/pipelined-frame-loop.ts +++ b/src/features/export/utils/pipelined-frame-loop.ts @@ -58,6 +58,38 @@ interface RecordedFailure { error: unknown } +async function encodeAndCloseSample( + sample: S, + keyFrame: boolean, + encodeSample: (sample: S, keyFrame: boolean) => Promise, + recordEncodeFailure: (error: unknown) => void, + recordFailure: (error: unknown) => void, +): Promise { + let failure: RecordedFailure | null = null + try { + await encodeSample(sample, keyFrame) + } catch (error) { + failure = { error } + recordEncodeFailure(error) + } + + try { + // The encoder does NOT close samples. We must close to release the + // underlying frame's GPU memory, otherwise the browser throttles after + // ~8-16 outstanding frames. + sample.close() + } catch (error) { + // If encoding already failed, it happened before cleanup and remains the + // failure represented by this settlement. + if (!failure) { + failure = { error } + recordFailure(error) + } + } + + return failure ? { status: 'rejected', reason: failure.error } : { status: 'fulfilled' } +} + export async function runPipelinedFrameLoop( deps: PipelinedFrameLoopDeps, ): Promise { @@ -124,9 +156,11 @@ export async function runPipelinedFrameLoop( } } - const throwFirstFailure = (): void => { + const throwRecordedFailureAfterDrain = async () => { const failure = firstFailure - if (failure) throw failure.error + if (!failure) return + if (signal?.aborted) await runAbortCleanup() + throw failure.error } try { @@ -143,8 +177,7 @@ export async function runPipelinedFrameLoop( if (signal?.aborted) { recordFailure(getAbortError()) await drainPendingEncode() - await runAbortCleanup() - throwFirstFailure() + await throwRecordedFailureAfterDrain() } // Render frame first — this overlaps with the previous frame's encode @@ -156,10 +189,7 @@ export async function runPipelinedFrameLoop( // sample. This ensures at most one encode is in flight and that frames // are fed to the encoder in order. await drainPendingEncode() - if (firstFailure) { - if (signal?.aborted) await runAbortCleanup() - throwFirstFailure() - } + await throwRecordedFailureAfterDrain() // Snapshot pixels into a sample. The capture copies pixel data // immediately — the surface is free for the next render. @@ -168,45 +198,24 @@ export async function runPipelinedFrameLoop( // Kick off encoding in the background. NOT awaited here — it runs // concurrently with the next iteration's renderFrame(). const isKeyFrame = frame === 0 - pendingEncode = (async (): Promise => { - let failure: RecordedFailure | null = null - try { - await encodeSample(sample, isKeyFrame) - } catch (error) { - failure = { error } - recordEncodeFailure(error) - } - - try { - // The encoder does NOT close samples. We must close to release the - // underlying frame's GPU memory, otherwise the browser throttles - // after ~8-16 outstanding frames. - sample.close() - } catch (error) { - // If encoding already failed, it happened before cleanup and remains - // the failure represented by this settlement. - if (!failure) { - failure = { error } - recordFailure(error) - } - } - - return failure ? { status: 'rejected', reason: failure.error } : { status: 'fulfilled' } - })() + pendingEncode = encodeAndCloseSample( + sample, + isKeyFrame, + encodeSample, + recordEncodeFailure, + recordFailure, + ) onFrameProgress(frame) } // Drain the final in-flight encode before finalizing await drainPendingEncode() - if (firstFailure) { - if (signal?.aborted) await runAbortCleanup() - throwFirstFailure() - } + await throwRecordedFailureAfterDrain() } catch (primaryError) { recordFailure(primaryError) await drainPendingEncode() - if (signal?.aborted) await runAbortCleanup() - throwFirstFailure() + await throwRecordedFailureAfterDrain() + throw primaryError } } From 5f31b1c6e5ad086464ab7e211849ce2763064688 Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Wed, 26 Aug 2026 21:41:19 -0700 Subject: [PATCH 4/6] fix(export): preserve first frame-loop failure --- .../export/utils/pipelined-frame-loop.test.ts | 118 ++++++++++++++++++ .../export/utils/pipelined-frame-loop.ts | 66 ++++++---- 2 files changed, 158 insertions(+), 26 deletions(-) diff --git a/src/features/export/utils/pipelined-frame-loop.test.ts b/src/features/export/utils/pipelined-frame-loop.test.ts index 5a1c02d6b..3404d3c5e 100644 --- a/src/features/export/utils/pipelined-frame-loop.test.ts +++ b/src/features/export/utils/pipelined-frame-loop.test.ts @@ -40,6 +40,7 @@ interface HarnessOptions { renderImpl?: (frame: number) => void | Promise encodeImpl?: (sample: FakeSample, keyFrame: boolean) => Promise closeImpl?: (sample: FakeSample) => void + onAbortImpl?: () => void | Promise } function createHarness(totalFrames: number, opts: HarnessOptions = {}) { @@ -77,6 +78,7 @@ function createHarness(totalFrames: number, opts: HarnessOptions = {}) { }, onAbort: async () => { events.push('abort-cancel') + await opts.onAbortImpl?.() }, onFrameProgress: (frame) => { events.push(`progress-${frame}`) @@ -260,6 +262,63 @@ describe('runPipelinedFrameLoop', () => { } }) + it('preserves earlier audio over abort and a rejecting render before encode drains', async () => { + const controller = new AbortController() + const audioError = new Error('audio task failed first') + const renderError = new Error('render failed later') + const cleanupError = new Error('sample cleanup failed last') + const encode = deferred() + const nextRender = deferred() + const unhandledRejections: unknown[] = [] + const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) + process.on('unhandledRejection', onUnhandledRejection) + let pendingError: unknown + let outcome: Promise | undefined + + try { + const { events, samples, run } = createHarness(2, { + signal: controller.signal, + getPendingError: () => pendingError, + renderImpl: (frame) => (frame === 1 ? nextRender.promise : undefined), + encodeImpl: () => encode.promise, + closeImpl: () => { + throw cleanupError + }, + }) + + let settled = false + outcome = run().then( + () => null, + (error: unknown) => error, + ) + void outcome.finally(() => { + settled = true + }) + await tick() + expect(events).toContain('render-1') + + pendingError = audioError + controller.abort() + nextRender.reject(renderError) + await tick() + expect(settled).toBe(false) + expect(unhandledRejections).toEqual([]) + + encode.resolve() + expect(await outcome).toBe(audioError) + expect(events).toContain('abort-cancel') + expect(samples[0]?.closed).toBe(true) + await tick() + expect(unhandledRejections).toEqual([]) + } finally { + controller.abort() + nextRender.resolve() + encode.resolve() + await outcome + process.off('unhandledRejection', onUnhandledRejection) + } + }) + it('preserves the video error when sample cleanup and rendering fail later', async () => { const encoderError = new Error('video encoder failed first') const cleanupError = new Error('sample cleanup failed later') @@ -476,6 +535,65 @@ describe('runPipelinedFrameLoop', () => { } }) + it('preserves earlier abort over audio and a rejecting render before encode drains', async () => { + const controller = new AbortController() + const audioError = new Error('audio task failed later') + const renderError = new Error('render failed later') + const abortCleanupError = new Error('abort cleanup failed last') + const encode = deferred() + const nextRender = deferred() + const unhandledRejections: unknown[] = [] + const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) + process.on('unhandledRejection', onUnhandledRejection) + let pendingError: unknown + let outcome: Promise | undefined + + try { + const { events, samples, run } = createHarness(2, { + signal: controller.signal, + getPendingError: () => pendingError, + renderImpl: (frame) => (frame === 1 ? nextRender.promise : undefined), + encodeImpl: () => encode.promise, + onAbortImpl: () => { + throw abortCleanupError + }, + }) + + let settled = false + outcome = run().then( + () => null, + (error: unknown) => error, + ) + void outcome.finally(() => { + settled = true + }) + await tick() + expect(events).toContain('render-1') + + controller.abort() + pendingError = audioError + nextRender.reject(renderError) + await tick() + expect(settled).toBe(false) + expect(unhandledRejections).toEqual([]) + + encode.resolve() + const error = await outcome + expect(error).toBeInstanceOf(DOMException) + expect((error as DOMException).name).toBe('AbortError') + expect(events).toContain('abort-cancel') + expect(samples[0]?.closed).toBe(true) + await tick() + expect(unhandledRejections).toEqual([]) + } finally { + controller.abort() + nextRender.resolve() + encode.resolve() + await outcome + process.off('unhandledRejection', onUnhandledRejection) + } + }) + it('throws a pending error at the top of the next iteration', async () => { const pendingError = new Error('audio task failed') let raised: unknown diff --git a/src/features/export/utils/pipelined-frame-loop.ts b/src/features/export/utils/pipelined-frame-loop.ts index ad7dcb243..e2733ef20 100644 --- a/src/features/export/utils/pipelined-frame-loop.ts +++ b/src/features/export/utils/pipelined-frame-loop.ts @@ -62,15 +62,14 @@ async function encodeAndCloseSample( sample: S, keyFrame: boolean, encodeSample: (sample: S, keyFrame: boolean) => Promise, - recordEncodeFailure: (error: unknown) => void, - recordFailure: (error: unknown) => void, + recordSettledFailure: (error: unknown) => void, ): Promise { let failure: RecordedFailure | null = null try { await encodeSample(sample, keyFrame) } catch (error) { failure = { error } - recordEncodeFailure(error) + recordSettledFailure(error) } try { @@ -83,7 +82,7 @@ async function encodeAndCloseSample( // failure represented by this settlement. if (!failure) { failure = { error } - recordFailure(error) + recordSettledFailure(error) } } @@ -121,26 +120,41 @@ export async function runPipelinedFrameLoop( return abortError } - const recordEncodeFailure = (error: unknown) => { - // A pending audio error or abort may have happened while renderFrame was - // still pending, before this encode rejected. Observe those primary exit - // conditions before recording the later encoder failure. + const recordPendingFailure = (): RecordedFailure | null => { try { const pendingError = getPendingError?.() - if (pendingError) recordFailure(pendingError) + if (!pendingError) return null + const failure = { error: pendingError } + recordFailure(pendingError) + return failure } catch (pendingError) { recordFailure(pendingError) + return { error: pendingError } } + } + const recordObservableFailures = () => { + // Sampling pending audio before recording an already-fired abort preserves + // their event order: the abort listener observes audio that failed first, + // while an abort already recorded by the listener remains primary over an + // audio failure that appears later. + recordPendingFailure() if (signal?.aborted) recordFailure(getAbortError()) + } + + const recordSettledFailure = (error: unknown) => { + // Audio or abort may have happened while renderFrame or this encode was + // pending. Observe those primary exit conditions before the later encode + // or sample-cleanup failure. + recordObservableFailures() recordFailure(error) } - const drainPendingEncode = async () => { - if (!pendingEncode) return + const drainPendingEncode = async (): Promise => { + if (!pendingEncode) return null const encode = pendingEncode try { - await encode + return await encode } finally { pendingEncode = null } @@ -163,13 +177,12 @@ export async function runPipelinedFrameLoop( throw failure.error } + signal?.addEventListener('abort', recordObservableFailures, { once: true }) + try { for (let frame = 0; frame < totalFrames; frame++) { - const pendingError = getPendingError?.() - if (pendingError) { - recordFailure(pendingError) - throw pendingError - } + const pendingFailure = recordPendingFailure() + if (pendingFailure) throw pendingFailure.error // Check for abort — drain any in-flight encode first so the encoder is // idle before we cancel the output. The first recorded failure wins, so @@ -188,8 +201,8 @@ export async function runPipelinedFrameLoop( // Now wait for the previous encode to finish before capturing a new // sample. This ensures at most one encode is in flight and that frames // are fed to the encoder in order. - await drainPendingEncode() - await throwRecordedFailureAfterDrain() + const previousEncode = await drainPendingEncode() + if (previousEncode?.status === 'rejected') await throwRecordedFailureAfterDrain() // Snapshot pixels into a sample. The capture copies pixel data // immediately — the surface is free for the next render. @@ -198,24 +211,25 @@ export async function runPipelinedFrameLoop( // Kick off encoding in the background. NOT awaited here — it runs // concurrently with the next iteration's renderFrame(). const isKeyFrame = frame === 0 - pendingEncode = encodeAndCloseSample( - sample, - isKeyFrame, - encodeSample, - recordEncodeFailure, - recordFailure, - ) + pendingEncode = encodeAndCloseSample(sample, isKeyFrame, encodeSample, recordSettledFailure) onFrameProgress(frame) } // Drain the final in-flight encode before finalizing await drainPendingEncode() + if (totalFrames > 0) recordObservableFailures() await throwRecordedFailureAfterDrain() } catch (primaryError) { + // A render/capture/progress rejection reaches this catch on a later promise + // turn. Sample failures already exposed by the concurrent channels before + // assigning that newly caught error. + recordObservableFailures() recordFailure(primaryError) await drainPendingEncode() await throwRecordedFailureAfterDrain() throw primaryError + } finally { + signal?.removeEventListener('abort', recordObservableFailures) } } From c0c288fbfa95de7e59c11951b27d4da074382816 Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Wed, 26 Aug 2026 21:59:36 -0700 Subject: [PATCH 5/6] fix(export): order frame failures at source --- .../utils/canvas-render-orchestrator.ts | 32 ++- .../export/utils/pipelined-frame-loop.test.ts | 211 +++++++++++++++--- .../export/utils/pipelined-frame-loop.ts | 130 ++++++----- 3 files changed, 273 insertions(+), 100 deletions(-) diff --git a/src/features/export/utils/canvas-render-orchestrator.ts b/src/features/export/utils/canvas-render-orchestrator.ts index 7748689ad..c2ee46c25 100644 --- a/src/features/export/utils/canvas-render-orchestrator.ts +++ b/src/features/export/utils/canvas-render-orchestrator.ts @@ -28,7 +28,7 @@ import { createExportOutputTarget } from './export-output-target' // Subsystems import { createCompositionRenderer } from './client-render-engine' -import { runPipelinedFrameLoop } from './pipelined-frame-loop' +import { createPipelinedFrameLoopFailureState, runPipelinedFrameLoop } from './pipelined-frame-loop' function getLog() { return createLogger('CanvasRenderOrchestrator') @@ -714,6 +714,7 @@ export async function renderComposition(options: RenderEngineOptions): Promise { if (videoRenderingStarted) return const boundedSeconds = Math.min(durationSeconds, completedSeconds) @@ -760,6 +761,7 @@ export async function renderComposition(options: RenderEngineOptions): Promise { audioError = error + frameLoopFailureState.reportFailure(error) }) onProgress({ @@ -787,21 +789,31 @@ export async function renderComposition(options: RenderEngineOptions): Promise audioError, - renderFrame: async (frame) => { - await renderer.renderFrame(frame) - // Scale to output resolution if needed - if (needsScaling) { - outputCtx.clearRect(0, 0, exportWidth, exportHeight) - outputCtx.drawImage(renderCanvas, 0, 0, exportWidth, exportHeight) + failureState: frameLoopFailureState, + renderFrame: async (frame, reportFailure) => { + try { + await renderer.renderFrame(frame) + // Scale to output resolution if needed + if (needsScaling) { + outputCtx.clearRect(0, 0, exportWidth, exportHeight) + outputCtx.drawImage(renderCanvas, 0, 0, exportWidth, exportHeight) + } + } catch (error) { + reportFailure(error) + throw error } }, // VideoSampleSource does NOT close samples (unlike CanvasSource) — the // loop closes each sample to release the VideoFrame's GPU memory. captureSample: (frame) => new VideoSample(outputCanvas, { timestamp: frame / fps, duration: 1 / fps }), - encodeSample: (sample, keyFrame) => - keyFrame ? videoSource.add(sample, { keyFrame: true }) : videoSource.add(sample), + encodeSample: (sample, keyFrame, reportFailure) => { + const encoding = keyFrame + ? videoSource.add(sample, { keyFrame: true }) + : videoSource.add(sample) + void encoding.catch(reportFailure) + return encoding + }, onAbort: () => output.cancel(), onFrameProgress: (frame) => { onProgress({ diff --git a/src/features/export/utils/pipelined-frame-loop.test.ts b/src/features/export/utils/pipelined-frame-loop.test.ts index 3404d3c5e..edae84d36 100644 --- a/src/features/export/utils/pipelined-frame-loop.test.ts +++ b/src/features/export/utils/pipelined-frame-loop.test.ts @@ -5,8 +5,12 @@ // shape on mocks without importing production code). End-to-end protection of // the full orchestrator remains the headless chrome e2e (headless/test.mjs). -import { describe, it, expect } from 'vite-plus/test' -import { runPipelinedFrameLoop } from './pipelined-frame-loop' +import { describe, it, expect, vi } from 'vite-plus/test' +import { + createPipelinedFrameLoopFailureState, + runPipelinedFrameLoop, + type PipelinedFrameLoopFailureState, +} from './pipelined-frame-loop' interface Deferred { promise: Promise @@ -36,7 +40,7 @@ interface FakeSample { interface HarnessOptions { signal?: AbortSignal - getPendingError?: () => unknown + failureState?: PipelinedFrameLoopFailureState renderImpl?: (frame: number) => void | Promise encodeImpl?: (sample: FakeSample, keyFrame: boolean) => Promise closeImpl?: (sample: FakeSample) => void @@ -46,15 +50,21 @@ interface HarnessOptions { function createHarness(totalFrames: number, opts: HarnessOptions = {}) { const events: string[] = [] const samples: FakeSample[] = [] + const failureState = opts.failureState ?? createPipelinedFrameLoopFailureState() const run = () => runPipelinedFrameLoop({ totalFrames, signal: opts.signal, - getPendingError: opts.getPendingError, - renderFrame: async (frame) => { + failureState, + renderFrame: async (frame, reportFailure) => { events.push(`render-${frame}`) - await opts.renderImpl?.(frame) + try { + await opts.renderImpl?.(frame) + } catch (error) { + reportFailure(error) + throw error + } }, captureSample: (frame) => { const sample: FakeSample = { @@ -70,9 +80,11 @@ function createHarness(totalFrames: number, opts: HarnessOptions = {}) { events.push(`capture-${frame}`) return sample }, - encodeSample: (sample, keyFrame) => { + encodeSample: (sample, keyFrame, reportFailure) => { events.push(`encode-start-${sample.frame}${keyFrame ? '-key' : ''}`) - return (opts.encodeImpl?.(sample, keyFrame) ?? Promise.resolve()).then(() => { + const encoding = opts.encodeImpl?.(sample, keyFrame) ?? Promise.resolve() + void encoding.catch(reportFailure) + return encoding.then(() => { events.push(`encode-end-${sample.frame}`) }) }, @@ -85,7 +97,7 @@ function createHarness(totalFrames: number, opts: HarnessOptions = {}) { }, }) - return { events, samples, run } + return { events, samples, failureState, run } } const indexOf = (events: string[], event: string) => { @@ -94,7 +106,139 @@ const indexOf = (events: string[], event: string) => { return index } +type FailureSource = 'render' | 'encode' | 'abort' | 'audio' + +interface FailureOrderCase { + name: string + first: FailureSource + second: FailureSource + expected: FailureSource +} + +const failureOrderCases: FailureOrderCase[] = [ + { name: 'render first then abort', first: 'render', second: 'abort', expected: 'render' }, + { name: 'render first then audio', first: 'render', second: 'audio', expected: 'render' }, + { name: 'encode first then abort', first: 'encode', second: 'abort', expected: 'encode' }, + { name: 'abort first then render', first: 'abort', second: 'render', expected: 'abort' }, + { name: 'abort first then encode', first: 'abort', second: 'encode', expected: 'abort' }, + { name: 'audio first then render', first: 'audio', second: 'render', expected: 'audio' }, +] + +const failureOrderMatrix = failureOrderCases.flatMap((testCase) => [ + { ...testCase, timing: 'same turn' as const }, + { ...testCase, timing: 'one microtask apart' as const }, +]) + describe('runPipelinedFrameLoop', () => { + it.each(failureOrderMatrix)( + 'preserves source order: $name ($timing)', + async ({ first, second, expected, timing }) => { + const controller = new AbortController() + const render = deferred() + const encode = deferred() + const audio = deferred() + const errors: Record = { + render: new Error('render source failed'), + encode: new Error('encode source failed'), + audio: new Error('audio source failed'), + abort: null, + } + const cleanupError = new Error('abort cleanup must not mask the primary failure') + const failureState = createPipelinedFrameLoopFailureState() + const observedAudio = audio.promise.then( + () => undefined, + (error: unknown) => failureState.reportFailure(error), + ) + const unhandledRejections: unknown[] = [] + const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) + const removeListener = vi.spyOn(controller.signal, 'removeEventListener') + process.on('unhandledRejection', onUnhandledRejection) + + let renderSettled = false + let encodeSettled = false + let audioSettled = false + let outcome: Promise | undefined + + const fire = (source: FailureSource) => { + switch (source) { + case 'render': + renderSettled = true + render.reject(errors.render) + break + case 'encode': + encodeSettled = true + encode.reject(errors.encode) + break + case 'audio': + audioSettled = true + audio.reject(errors.audio) + break + case 'abort': + controller.abort() + break + } + } + + try { + const { events, samples, run } = createHarness(2, { + signal: controller.signal, + failureState, + renderImpl: (frame) => (frame === 1 ? render.promise : undefined), + encodeImpl: () => encode.promise, + onAbortImpl: () => { + throw cleanupError + }, + }) + + outcome = run().then( + () => null, + (error: unknown) => error, + ) + await tick() + expect(events).toContain('render-1') + + // Calls earlier in this list define same-turn ties. Promise reactions + // and the queued abort publication retain that source enqueue order. + fire(first) + if (timing === 'one microtask apart') await Promise.resolve() + fire(second) + + if (!renderSettled) render.resolve() + if (!encodeSettled) encode.resolve() + if (!audioSettled) audio.resolve() + + const error = await outcome + if (expected === 'abort') { + expect(error).toBeInstanceOf(DOMException) + expect((error as DOMException).name).toBe('AbortError') + } else { + expect(error).toBe(errors[expected]) + } + + await observedAudio + await tick() + expect(unhandledRejections).toEqual([]) + expect(samples).toHaveLength(1) + expect(samples[0]?.closed).toBe(true) + expect(events).toContain('close-0') + expect(events.includes('abort-cancel')).toBe(controller.signal.aborted) + if (controller.signal.aborted) { + expect(indexOf(events, 'close-0')).toBeLessThan(indexOf(events, 'abort-cancel')) + } + expect(removeListener).toHaveBeenCalledWith('abort', expect.any(Function)) + } finally { + controller.abort() + render.resolve() + encode.resolve() + audio.resolve() + await observedAudio + await outcome + removeListener.mockRestore() + process.off('unhandledRejection', onUnhandledRejection) + } + }, + ) + it('encodes all frames in order and closes every sample', async () => { const { events, samples, run } = createHarness(5) await run() @@ -229,12 +373,10 @@ describe('runPipelinedFrameLoop', () => { const unhandledRejections: unknown[] = [] const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) process.on('unhandledRejection', onUnhandledRejection) - let pendingError: unknown let outcome: Promise | undefined try { - const { events, samples, run } = createHarness(2, { - getPendingError: () => pendingError, + const { events, samples, failureState, run } = createHarness(2, { renderImpl: (frame) => (frame === 1 ? nextRender.promise : undefined), encodeImpl: () => encode.promise, }) @@ -246,7 +388,7 @@ describe('runPipelinedFrameLoop', () => { await tick() expect(events).toContain('render-1') - pendingError = audioError + failureState.reportFailure(audioError) encode.reject(encoderError) await tick() expect(unhandledRejections).toEqual([]) @@ -272,13 +414,11 @@ describe('runPipelinedFrameLoop', () => { const unhandledRejections: unknown[] = [] const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) process.on('unhandledRejection', onUnhandledRejection) - let pendingError: unknown let outcome: Promise | undefined try { - const { events, samples, run } = createHarness(2, { + const { events, samples, failureState, run } = createHarness(2, { signal: controller.signal, - getPendingError: () => pendingError, renderImpl: (frame) => (frame === 1 ? nextRender.promise : undefined), encodeImpl: () => encode.promise, closeImpl: () => { @@ -297,7 +437,7 @@ describe('runPipelinedFrameLoop', () => { await tick() expect(events).toContain('render-1') - pendingError = audioError + failureState.reportFailure(audioError) controller.abort() nextRender.reject(renderError) await tick() @@ -402,13 +542,13 @@ describe('runPipelinedFrameLoop', () => { const unhandledRejections: unknown[] = [] const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) process.on('unhandledRejection', onUnhandledRejection) - let raised: unknown + const failureState = createPipelinedFrameLoopFailureState() try { const { events, samples, run } = createHarness(4, { - getPendingError: () => raised, + failureState, renderImpl: (frame) => { - if (frame === 1) raised = pendingError + if (frame === 1) failureState.reportFailure(pendingError) }, encodeImpl: () => { const encode = deferred() @@ -545,13 +685,17 @@ describe('runPipelinedFrameLoop', () => { const unhandledRejections: unknown[] = [] const onUnhandledRejection = (reason: unknown) => unhandledRejections.push(reason) process.on('unhandledRejection', onUnhandledRejection) - let pendingError: unknown let outcome: Promise | undefined try { + const audio = deferred() + const failureState = createPipelinedFrameLoopFailureState() + const observedAudio = audio.promise.catch((error: unknown) => { + failureState.reportFailure(error) + }) const { events, samples, run } = createHarness(2, { signal: controller.signal, - getPendingError: () => pendingError, + failureState, renderImpl: (frame) => (frame === 1 ? nextRender.promise : undefined), encodeImpl: () => encode.promise, onAbortImpl: () => { @@ -571,7 +715,7 @@ describe('runPipelinedFrameLoop', () => { expect(events).toContain('render-1') controller.abort() - pendingError = audioError + audio.reject(audioError) nextRender.reject(renderError) await tick() expect(settled).toBe(false) @@ -583,6 +727,7 @@ describe('runPipelinedFrameLoop', () => { expect((error as DOMException).name).toBe('AbortError') expect(events).toContain('abort-cancel') expect(samples[0]?.closed).toBe(true) + await observedAudio await tick() expect(unhandledRejections).toEqual([]) } finally { @@ -596,11 +741,11 @@ describe('runPipelinedFrameLoop', () => { it('throws a pending error at the top of the next iteration', async () => { const pendingError = new Error('audio task failed') - let raised: unknown + const failureState = createPipelinedFrameLoopFailureState() const { events, run } = createHarness(5, { - getPendingError: () => raised, + failureState, renderImpl: (frame) => { - if (frame === 1) raised = pendingError + if (frame === 1) failureState.reportFailure(pendingError) }, }) @@ -609,20 +754,20 @@ describe('runPipelinedFrameLoop', () => { expect(events).not.toContain('render-2') }) - it('ignores falsy pending errors (truthiness semantics)', async () => { - for (const falsy of [undefined, '', 0, null]) { - const { samples, run } = createHarness(2, { getPendingError: () => falsy }) - await run() - expect(samples).toHaveLength(2) - } + it('continues until an external source publishes a failure', async () => { + const { samples, run } = createHarness(2) + await run() + expect(samples).toHaveLength(2) }) it('resolves immediately for zero frames without touching any callback', async () => { const controller = new AbortController() controller.abort() + const failureState = createPipelinedFrameLoopFailureState() + failureState.reportFailure(new Error('never checked')) const { events, run } = createHarness(0, { signal: controller.signal, - getPendingError: () => new Error('never checked'), + failureState, }) await run() // Pre-loop abort/error checks are the caller's responsibility. diff --git a/src/features/export/utils/pipelined-frame-loop.ts b/src/features/export/utils/pipelined-frame-loop.ts index e2733ef20..0a2672561 100644 --- a/src/features/export/utils/pipelined-frame-loop.ts +++ b/src/features/export/utils/pipelined-frame-loop.ts @@ -17,19 +17,51 @@ export interface CloseableSample { close(): void } +interface RecordedFailure { + error: unknown +} + +export interface PipelinedFrameLoopFailureState { + readonly firstFailure: RecordedFailure | null + reportFailure(error: unknown): void +} + +/** + * Shared first-error latch for concurrently running export sources. + * + * Sources publish when their failure becomes observable. Promise sources must + * attach their rejection observer immediately; abort publication is queued as + * a microtask so same-turn promise rejection and abort events are ordered by + * the source events that queued their observers, not by a synchronous abort + * listener racing ahead of already-fired promise rejections. + */ +export function createPipelinedFrameLoopFailureState(): PipelinedFrameLoopFailureState { + let firstFailure: RecordedFailure | null = null + return { + get firstFailure() { + return firstFailure + }, + reportFailure(error) { + firstFailure ??= { error } + }, + } +} + export interface PipelinedFrameLoopDeps { totalFrames: number signal?: AbortSignal /** - * Read (not throw) a pending async error, e.g. a failed audio task. - * Checked with a truthiness test at the top of every iteration. + * Shared source-event latch. An independently running source such as audio + * must attach a rejection observer immediately and publish into this state. */ - getPendingError?: () => unknown + failureState?: PipelinedFrameLoopFailureState /** * Render the frame to the capture surface, including any scale-to-output - * blit. Overlaps with the previous frame's in-flight encode. + * blit. Overlaps with the previous frame's in-flight encode. The callback + * must be invoked by the rejection observer attached directly to the source + * promise, before rethrowing through any async wrapper. */ - renderFrame: (frame: number) => Promise + renderFrame: (frame: number, reportFailure: (error: unknown) => void) => Promise /** * Snapshot the capture surface (e.g. VideoSample construction). Called * strictly after the previous encode has drained; must stay synchronous. @@ -37,9 +69,14 @@ export interface PipelinedFrameLoopDeps { captureSample: (frame: number) => S /** * Feed the sample to the encoder. `keyFrame` is true only for frame 0. - * The loop closes the sample when the returned promise settles. + * The loop closes the sample when the returned promise settles. As with + * renderFrame, report a rejection from an observer on the source promise. */ - encodeSample: (sample: S, keyFrame: boolean) => Promise + encodeSample: ( + sample: S, + keyFrame: boolean, + reportFailure: (error: unknown) => void, + ) => Promise /** * Abort path: called after the in-flight encode has been drained (its * errors observed), before the selected failure is thrown. @@ -54,19 +91,19 @@ interface EncodeSettlement { reason?: unknown } -interface RecordedFailure { - error: unknown -} - async function encodeAndCloseSample( sample: S, keyFrame: boolean, - encodeSample: (sample: S, keyFrame: boolean) => Promise, + encodeSample: ( + sample: S, + keyFrame: boolean, + reportFailure: (error: unknown) => void, + ) => Promise, recordSettledFailure: (error: unknown) => void, ): Promise { let failure: RecordedFailure | null = null try { - await encodeSample(sample, keyFrame) + await encodeSample(sample, keyFrame, recordSettledFailure) } catch (error) { failure = { error } recordSettledFailure(error) @@ -95,7 +132,7 @@ export async function runPipelinedFrameLoop( const { totalFrames, signal, - getPendingError, + failureState = createPipelinedFrameLoopFailureState(), renderFrame, captureSample, encodeSample, @@ -107,12 +144,13 @@ export async function runPipelinedFrameLoop( // are reflected into a settlement immediately, so an encoder rejection is // observed even while renderFrame remains pending for another event turn. let pendingEncode: Promise | null = null - let firstFailure: RecordedFailure | null = null let abortError: DOMException | null = null let abortCleanupStarted = false + let abortPublicationQueued = false + let listenerActive = true const recordFailure = (error: unknown) => { - firstFailure ??= { error } + failureState.reportFailure(error) } const getAbortError = () => { @@ -120,34 +158,12 @@ export async function runPipelinedFrameLoop( return abortError } - const recordPendingFailure = (): RecordedFailure | null => { - try { - const pendingError = getPendingError?.() - if (!pendingError) return null - const failure = { error: pendingError } - recordFailure(pendingError) - return failure - } catch (pendingError) { - recordFailure(pendingError) - return { error: pendingError } - } - } - - const recordObservableFailures = () => { - // Sampling pending audio before recording an already-fired abort preserves - // their event order: the abort listener observes audio that failed first, - // while an abort already recorded by the listener remains primary over an - // audio failure that appears later. - recordPendingFailure() - if (signal?.aborted) recordFailure(getAbortError()) - } - - const recordSettledFailure = (error: unknown) => { - // Audio or abort may have happened while renderFrame or this encode was - // pending. Observe those primary exit conditions before the later encode - // or sample-cleanup failure. - recordObservableFailures() - recordFailure(error) + const publishAbort = () => { + if (abortPublicationQueued) return + abortPublicationQueued = true + queueMicrotask(() => { + if (listenerActive) recordFailure(getAbortError()) + }) } const drainPendingEncode = async (): Promise => { @@ -171,24 +187,28 @@ export async function runPipelinedFrameLoop( } const throwRecordedFailureAfterDrain = async () => { - const failure = firstFailure + const failure = failureState.firstFailure if (!failure) return if (signal?.aborted) await runAbortCleanup() throw failure.error } - signal?.addEventListener('abort', recordObservableFailures, { once: true }) + signal?.addEventListener('abort', publishAbort, { once: true }) try { for (let frame = 0; frame < totalFrames; frame++) { - const pendingFailure = recordPendingFailure() + const pendingFailure = failureState.firstFailure if (pendingFailure) throw pendingFailure.error // Check for abort — drain any in-flight encode first so the encoder is // idle before we cancel the output. The first recorded failure wins, so // this AbortError is preserved over an encoder failure during the drain. if (signal?.aborted) { - recordFailure(getAbortError()) + publishAbort() + // Let reactions queued by source failures that fired before abort run + // before the queued abort publication. If abort fired first, its + // publication was queued first and remains primary. + await Promise.resolve() await drainPendingEncode() await throwRecordedFailureAfterDrain() } @@ -196,7 +216,7 @@ export async function runPipelinedFrameLoop( // Render frame first — this overlaps with the previous frame's encode // that is still in flight. The previous sample already copied its // pixels, so writing to the capture surface here cannot corrupt it. - await renderFrame(frame) + await renderFrame(frame, recordFailure) // Now wait for the previous encode to finish before capturing a new // sample. This ensures at most one encode is in flight and that frames @@ -211,25 +231,21 @@ export async function runPipelinedFrameLoop( // Kick off encoding in the background. NOT awaited here — it runs // concurrently with the next iteration's renderFrame(). const isKeyFrame = frame === 0 - pendingEncode = encodeAndCloseSample(sample, isKeyFrame, encodeSample, recordSettledFailure) + pendingEncode = encodeAndCloseSample(sample, isKeyFrame, encodeSample, recordFailure) onFrameProgress(frame) } // Drain the final in-flight encode before finalizing await drainPendingEncode() - if (totalFrames > 0) recordObservableFailures() - await throwRecordedFailureAfterDrain() + if (totalFrames > 0) await throwRecordedFailureAfterDrain() } catch (primaryError) { - // A render/capture/progress rejection reaches this catch on a later promise - // turn. Sample failures already exposed by the concurrent channels before - // assigning that newly caught error. - recordObservableFailures() recordFailure(primaryError) await drainPendingEncode() await throwRecordedFailureAfterDrain() throw primaryError } finally { - signal?.removeEventListener('abort', recordObservableFailures) + listenerActive = false + signal?.removeEventListener('abort', publishAbort) } } From 9cf093b81410d4b176c33f1f3b3ad19c12beb0ce Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Thu, 27 Aug 2026 02:19:11 -0700 Subject: [PATCH 6/6] fix(export): preserve primary frame-loop failures --- .../export/utils/pipelined-frame-loop.test.ts | 143 ++++++++++++ .../export/utils/pipelined-frame-loop.ts | 219 +++++++++++++----- 2 files changed, 301 insertions(+), 61 deletions(-) diff --git a/src/features/export/utils/pipelined-frame-loop.test.ts b/src/features/export/utils/pipelined-frame-loop.test.ts index edae84d36..8b5a0e7c1 100644 --- a/src/features/export/utils/pipelined-frame-loop.test.ts +++ b/src/features/export/utils/pipelined-frame-loop.test.ts @@ -239,6 +239,149 @@ describe('runPipelinedFrameLoop', () => { }, ) + it('preserves a render rejection boundary over queued encode-success cleanup', async () => { + const renderError = new Error('render source rejected first') + const closeError = new Error('sample close ran before the render observer') + let closeCount = 0 + const sample: FakeSample = { + frame: 0, + closed: false, + close() { + closeCount++ + this.closed = true + throw closeError + }, + } + + const outcome = runPipelinedFrameLoop({ + totalFrames: 2, + renderFrame: (frame, reportFailure) => { + if (frame === 0) return Promise.resolve() + + // The encode-success continuation is already queued, so it will run + // sample.close() before this rejection observer. The render rejection + // is nevertheless a primary source boundary and must own the result. + const rendering = Promise.reject(renderError) + void rendering.catch(reportFailure) + return rendering + }, + captureSample: () => sample, + encodeSample: (_sample, _keyFrame, reportFailure) => { + const encoding = Promise.resolve() + void encoding.catch(reportFailure) + return encoding + }, + onAbort: () => Promise.resolve(), + onFrameProgress: () => undefined, + }).then( + () => null, + (error: unknown) => error, + ) + + const error = await outcome + expect(error).toBe(renderError) + expect(closeCount).toBe(1) + }) + + it('preserves an abort boundary over a later synchronous render throw', async () => { + const controller = new AbortController() + const renderError = new Error('render threw after abort') + const onAbort = vi.fn(() => Promise.resolve()) + + const error = await runPipelinedFrameLoop({ + totalFrames: 1, + signal: controller.signal, + renderFrame: () => { + controller.abort() + throw renderError + }, + captureSample: () => { + throw new Error('capture must not run') + }, + encodeSample: () => Promise.resolve(), + onAbort, + onFrameProgress: () => undefined, + }).then( + () => null, + (failure: unknown) => failure, + ) + + expect(error).toBeInstanceOf(DOMException) + expect((error as DOMException).name).toBe('AbortError') + expect(onAbort).toHaveBeenCalledOnce() + }) + + it('lets an already-queued render rejection observer beat a later abort publication', async () => { + const controller = new AbortController() + const renderError = new Error('render rejected before abort') + + const error = await runPipelinedFrameLoop({ + totalFrames: 1, + signal: controller.signal, + renderFrame: (_frame, reportFailure) => { + const rendering = Promise.reject(renderError) + void rendering.catch(reportFailure) + controller.abort() + return rendering + }, + captureSample: () => { + throw new Error('capture must not run') + }, + encodeSample: () => Promise.resolve(), + onAbort: () => Promise.resolve(), + onFrameProgress: () => undefined, + }).then( + () => null, + (failure: unknown) => failure, + ) + + expect(error).toBe(renderError) + }) + + it('preserves an established primary when listener removal throws', async () => { + const controller = new AbortController() + const renderError = new Error('primary render failure') + const listenerError = new Error('listener removal failed') + const removeListener = vi + .spyOn(controller.signal, 'removeEventListener') + .mockImplementation(() => { + throw listenerError + }) + + try { + const { run } = createHarness(1, { + signal: controller.signal, + renderImpl: () => { + throw renderError + }, + }) + + await expect(run()).rejects.toBe(renderError) + expect(removeListener).toHaveBeenCalledOnce() + } finally { + removeListener.mockRestore() + } + }) + + it('surfaces listener-removal failure when there is no primary failure', async () => { + const controller = new AbortController() + const listenerError = new Error('listener removal failed') + const removeListener = vi + .spyOn(controller.signal, 'removeEventListener') + .mockImplementation(() => { + throw listenerError + }) + + try { + const { run } = createHarness(1, { signal: controller.signal }) + + await expect(run()).rejects.toBe(listenerError) + expect(removeListener).toHaveBeenCalledOnce() + } finally { + removeListener.mockRestore() + } + }) + it('encodes all frames in order and closes every sample', async () => { const { events, samples, run } = createHarness(5) await run() diff --git a/src/features/export/utils/pipelined-frame-loop.ts b/src/features/export/utils/pipelined-frame-loop.ts index 0a2672561..e13a5c245 100644 --- a/src/features/export/utils/pipelined-frame-loop.ts +++ b/src/features/export/utils/pipelined-frame-loop.ts @@ -8,9 +8,9 @@ * * Behavior must stay bit-identical to the original inline loop — this is the * export hot path. Every exit drains an in-flight encode so its sample closes - * and its rejection is observed. Failures retain their occurrence order: a - * render, pending audio, or abort failure that happens first is not replaced - * by a later encode or cleanup failure, and vice versa. + * and its rejection is observed. Primary render, encode, audio, and abort + * failures retain their source-boundary order. Cleanup failures are tracked + * separately and surface only when no primary operation failed. */ export interface CloseableSample { @@ -23,26 +23,41 @@ interface RecordedFailure { export interface PipelinedFrameLoopFailureState { readonly firstFailure: RecordedFailure | null + readonly firstPrimaryFailure: RecordedFailure | null + readonly firstCleanupFailure: RecordedFailure | null reportFailure(error: unknown): void + reportCleanupFailure(error: unknown): void } /** - * Shared first-error latch for concurrently running export sources. + * Shared failure ownership for concurrently running export sources. * * Sources publish when their failure becomes observable. Promise sources must * attach their rejection observer immediately; abort publication is queued as * a microtask so same-turn promise rejection and abort events are ordered by * the source events that queued their observers, not by a synchronous abort - * listener racing ahead of already-fired promise rejections. + * listener racing ahead of already-fired promise rejections. Cleanup has its + * own first-error latch so continuation order cannot let cleanup mask a + * primary source failure. */ export function createPipelinedFrameLoopFailureState(): PipelinedFrameLoopFailureState { - let firstFailure: RecordedFailure | null = null + let firstPrimaryFailure: RecordedFailure | null = null + let firstCleanupFailure: RecordedFailure | null = null return { get firstFailure() { - return firstFailure + return firstPrimaryFailure ?? firstCleanupFailure + }, + get firstPrimaryFailure() { + return firstPrimaryFailure + }, + get firstCleanupFailure() { + return firstCleanupFailure }, reportFailure(error) { - firstFailure ??= { error } + firstPrimaryFailure ??= { error } + }, + reportCleanupFailure(error) { + firstCleanupFailure ??= { error } }, } } @@ -86,7 +101,7 @@ export interface PipelinedFrameLoopDeps { onFrameProgress: (frame: number) => void } -interface EncodeSettlement { +interface OperationSettlement { status: 'fulfilled' | 'rejected' reason?: unknown } @@ -100,13 +115,27 @@ async function encodeAndCloseSample( reportFailure: (error: unknown) => void, ) => Promise, recordSettledFailure: (error: unknown) => void, -): Promise { + recordSynchronousFailure: (error: unknown) => Promise, + recordCleanupFailure: (error: unknown) => void, +): Promise { let failure: RecordedFailure | null = null + let encoding: Promise | null = null try { - await encodeSample(sample, keyFrame, recordSettledFailure) + encoding = encodeSample(sample, keyFrame, recordSettledFailure) } catch (error) { failure = { error } - recordSettledFailure(error) + await recordSynchronousFailure(error) + } + + if (encoding) { + const settlement: OperationSettlement = await encoding.then( + (): OperationSettlement => ({ status: 'fulfilled' }), + (error: unknown): OperationSettlement => { + recordSettledFailure(error) + return { status: 'rejected', reason: error } + }, + ) + if (settlement.status === 'rejected') failure = { error: settlement.reason } } try { @@ -119,7 +148,7 @@ async function encodeAndCloseSample( // failure represented by this settlement. if (!failure) { failure = { error } - recordSettledFailure(error) + recordCleanupFailure(error) } } @@ -143,7 +172,7 @@ export async function runPipelinedFrameLoop( // The promise stored here never rejects. Encode and sample-cleanup failures // are reflected into a settlement immediately, so an encoder rejection is // observed even while renderFrame remains pending for another event turn. - let pendingEncode: Promise | null = null + let pendingEncode: Promise | null = null let abortError: DOMException | null = null let abortCleanupStarted = false let abortPublicationQueued = false @@ -153,6 +182,10 @@ export async function runPipelinedFrameLoop( failureState.reportFailure(error) } + const recordCleanupFailure = (error: unknown) => { + failureState.reportCleanupFailure(error) + } + const getAbortError = () => { abortError ??= new DOMException('Render cancelled', 'AbortError') return abortError @@ -166,7 +199,18 @@ export async function runPipelinedFrameLoop( }) } - const drainPendingEncode = async (): Promise => { + const recordSynchronousFailure = async (error: unknown) => { + if (abortPublicationQueued) { + // Abort reserves its boundary synchronously but publishes in a + // microtask. Yield once so an observer queued before the abort can + // publish first, while the abort itself stays ahead of this later + // synchronous throw. + await Promise.resolve() + } + recordFailure(error) + } + + const drainPendingEncode = async (): Promise => { if (!pendingEncode) return null const encode = pendingEncode try { @@ -182,70 +226,123 @@ export async function runPipelinedFrameLoop( try { await onAbort() } catch (error) { - recordFailure(error) + recordCleanupFailure(error) } } const throwRecordedFailureAfterDrain = async () => { + if (signal?.aborted) await runAbortCleanup() const failure = failureState.firstFailure if (!failure) return - if (signal?.aborted) await runAbortCleanup() throw failure.error } + const renderAndObserve = async (frame: number) => { + let rendering: Promise + try { + rendering = renderFrame(frame, recordFailure) + } catch (error) { + await recordSynchronousFailure(error) + throw error + } + const settlement: OperationSettlement = await rendering.then( + (): OperationSettlement => ({ status: 'fulfilled' }), + (error: unknown): OperationSettlement => { + recordFailure(error) + return { status: 'rejected', reason: error } + }, + ) + if (settlement.status === 'rejected') throw settlement.reason + } + + const isRecordedFailure = (error: unknown) => + failureState.firstPrimaryFailure?.error === error || + failureState.firstCleanupFailure?.error === error + + const drainFinalEncode = async () => { + await drainPendingEncode() + if (totalFrames > 0) await throwRecordedFailureAfterDrain() + } + signal?.addEventListener('abort', publishAbort, { once: true }) - try { - for (let frame = 0; frame < totalFrames; frame++) { - const pendingFailure = failureState.firstFailure - if (pendingFailure) throw pendingFailure.error - - // Check for abort — drain any in-flight encode first so the encoder is - // idle before we cancel the output. The first recorded failure wins, so - // this AbortError is preserved over an encoder failure during the drain. - if (signal?.aborted) { - publishAbort() - // Let reactions queued by source failures that fired before abort run - // before the queued abort publication. If abort fired first, its - // publication was queued first and remains primary. - await Promise.resolve() - await drainPendingEncode() - await throwRecordedFailureAfterDrain() - } + const runLoop = async () => { + try { + for (let frame = 0; frame < totalFrames; frame++) { + const pendingFailure = failureState.firstFailure + if (pendingFailure) throw pendingFailure.error + + // Check for abort — drain any in-flight encode first so the encoder is + // idle before we cancel the output. The first recorded failure wins, so + // this AbortError is preserved over an encoder failure during the drain. + if (signal?.aborted) { + publishAbort() + // Let reactions queued by source failures that fired before abort run + // before the queued abort publication. If abort fired first, its + // publication was queued first and remains primary. + await Promise.resolve() + await drainPendingEncode() + await throwRecordedFailureAfterDrain() + } - // Render frame first — this overlaps with the previous frame's encode - // that is still in flight. The previous sample already copied its - // pixels, so writing to the capture surface here cannot corrupt it. - await renderFrame(frame, recordFailure) + // Render frame first — this overlaps with the previous frame's encode + // that is still in flight. The previous sample already copied its + // pixels, so writing to the capture surface here cannot corrupt it. + await renderAndObserve(frame) - // Now wait for the previous encode to finish before capturing a new - // sample. This ensures at most one encode is in flight and that frames - // are fed to the encoder in order. - const previousEncode = await drainPendingEncode() - if (previousEncode?.status === 'rejected') await throwRecordedFailureAfterDrain() + // Now wait for the previous encode to finish before capturing a new + // sample. This ensures at most one encode is in flight and that frames + // are fed to the encoder in order. + const previousEncode = await drainPendingEncode() + if (previousEncode?.status === 'rejected') await throwRecordedFailureAfterDrain() - // Snapshot pixels into a sample. The capture copies pixel data - // immediately — the surface is free for the next render. - const sample = captureSample(frame) + // Snapshot pixels into a sample. The capture copies pixel data + // immediately — the surface is free for the next render. + const sample = captureSample(frame) - // Kick off encoding in the background. NOT awaited here — it runs - // concurrently with the next iteration's renderFrame(). - const isKeyFrame = frame === 0 - pendingEncode = encodeAndCloseSample(sample, isKeyFrame, encodeSample, recordFailure) + // Kick off encoding in the background. NOT awaited here — it runs + // concurrently with the next iteration's renderFrame(). + const isKeyFrame = frame === 0 + pendingEncode = encodeAndCloseSample( + sample, + isKeyFrame, + encodeSample, + recordFailure, + recordSynchronousFailure, + recordCleanupFailure, + ) - onFrameProgress(frame) + onFrameProgress(frame) + } + + // Drain the final in-flight encode before finalizing + await drainFinalEncode() + } catch (primaryError) { + if (!isRecordedFailure(primaryError)) await recordSynchronousFailure(primaryError) + await drainPendingEncode() + await throwRecordedFailureAfterDrain() + throw primaryError } + } - // Drain the final in-flight encode before finalizing - await drainPendingEncode() - if (totalFrames > 0) await throwRecordedFailureAfterDrain() - } catch (primaryError) { - recordFailure(primaryError) - await drainPendingEncode() - await throwRecordedFailureAfterDrain() - throw primaryError - } finally { - listenerActive = false + const loopSettlement: OperationSettlement = await runLoop().then( + (): OperationSettlement => ({ status: 'fulfilled' }), + (error: unknown): OperationSettlement => ({ status: 'rejected', reason: error }), + ) + + listenerActive = false + let listenerRemovalFailure: RecordedFailure | null = null + try { signal?.removeEventListener('abort', publishAbort) + } catch (error) { + listenerRemovalFailure = { error } + recordCleanupFailure(error) + } + + if (loopSettlement.status === 'rejected') { + throw failureState.firstFailure?.error ?? loopSettlement.reason + } + if (listenerRemovalFailure) { + throw failureState.firstFailure?.error ?? listenerRemovalFailure.error } }