refactor: implement timeout handling for video export processes and enhance error management#17
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces robust timeout handling and backpressure management during video exporting. It adds a withAbortableTimeout helper, implements a VideoEncodeQueue to bound pending encode frames, and introduces periodic yielding to the browser main thread during frame capture. Additionally, tests have been expanded to cover timeouts and backpressure scenarios. The review feedback suggests improving type safety by making the cancel method optional on CancellableOutput and using optional chaining when invoking it, as well as simplifying the promise-wrapping logic in VideoEncodeQueue.enqueue.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| type CancellableOutput = Output & { | ||
| cancel(): Promise<void>; | ||
| }; |
There was a problem hiding this comment.
Defining cancel as an optional method on CancellableOutput is safer and more accurate, as cancel might not be present on all versions of the Output class from mediabunny. This allows the TypeScript compiler to enforce safety checks when calling it.
type CancellableOutput = Output & {\n\tcancel?(): Promise<void>;\n};| public enqueue(frameIndex: number): void { | ||
| let promise: Promise<void>; | ||
| try { | ||
| promise = Promise.resolve(this._source.add(frameIndex / this._plan.frameRate, 1 / this._plan.frameRate)); | ||
| } catch (error) { | ||
| promise = Promise.reject(error); | ||
| } | ||
| promise.catch(() => undefined); | ||
| this._pending.push(promise); | ||
| } |
There was a problem hiding this comment.
The synchronous try/catch block and Promise.resolve wrapping can be simplified by using Promise.resolve().then(...). This automatically catches any synchronous errors thrown by this._source.add and converts them into a rejected promise, making the code cleaner and avoiding mutable let bindings.
\tpublic enqueue(frameIndex: number): void {\n\t\tconst promise = Promise.resolve().then(() =>\n\t\t\tthis._source.add(frameIndex / this._plan.frameRate, 1 / this._plan.frameRate)\n\t\t);\n\t\tpromise.catch(() => undefined);\n\t\tthis._pending.push(promise);\n\t}| try { | ||
| await cancellableOutput.cancel(); | ||
| } catch { | ||
| // Best-effort cleanup only; the original export error is more useful. | ||
| } |
There was a problem hiding this comment.
Since cancellableOutput is cast from Output to CancellableOutput (suggesting cancel may not be part of the standard Output type definition or could be missing in some versions/environments), it is safer to use optional chaining (cancel?.()) to avoid throwing a TypeError during error cleanup.
\t\t\ttry {\n\t\t\t\tawait cancellableOutput.cancel?.();\n\t\t\t} catch {\n\t\t\t\t// Best-effort cleanup only; the original export error is more useful.\n\t\t\t}1 participant
No description provided.