Skip to content

Drain In-Flight Encodes on Export Errors - #29

Open
lightninglu10 wants to merge 8 commits into
v1-editor-hardeningfrom
v1-export-encode-drain
Open

Drain In-Flight Encodes on Export Errors#29
lightninglu10 wants to merge 8 commits into
v1-editor-hardeningfrom
v1-export-encode-drain

Conversation

@lightninglu10

@lightninglu10lightninglu10 commented Aug 27, 2026

Copy link
Copy Markdown

Summary

Failed exports now always observe and drain in-flight encoder work while preserving the primary render or pending error, preventing late unhandled rejections from poisoning later exports.

Technical details

  • Uses one pending-encode drain authority for normal, error, and abort exits.
  • The normal path continues to propagate encoder errors.
  • A primary render or pending error wins over a secondary encode failure.
  • Abort continues to surface AbortError and discard an encoder error.
  • Sample closure, frame ordering, render/encode overlap, progress timing, and keyframe selection remain unchanged.

Test plan

  • Run the 14 focused race, ordering, keyframe, abort, progress, and sample-closure tests.
  • Run the 26 neighboring export tests.
  • Run the static check and production build.
  • Run the portable Node, Chrome, edit-operation, lifecycle, and media/audio contracts.

Open items

Intentionally stacked on v1-editor-hardening / PR #25. This is review-only until that base lands or integrates; do not retarget it to main.

@lightninglu10lightninglu10 self-assigned this Aug 27, 2026
@lightninglu10
lightninglu10 marked this pull request as ready for review August 27, 2026 05:26
@codepress-reviewer-dev

Copy link
Copy Markdown

Starting initial review of ae89a47c from baseline 1f8df903... 👀

View automation run

@codepress-reviewer-devcodepress-reviewer-devBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The shared first-failure latch and single drain authority give this concurrency fix a coherent ownership boundary: every in-flight encode is observed and its sample is closed, while error precedence remains stable across render, audio, encoder, abort, and cleanup races.

The focused coverage exercises the meaningful overlap and ordering cases, including late encoder failures during render errors and aborts. I found no actionable correctness or production-safety concerns in the intrinsic diff.

Reviewed `1f8df903`...`ae89a47c`. Diff collection and GitHub posting were handled by the automation.

View automation run

@codepress-reviewer-dev

codepress-reviewer-devBot commented Aug 27, 2026

Copy link
Copy Markdown

Review head 9cf093b81410d4b176c33f1f3b3ad19c12beb0ce; round 2; 3 reviewed file(s), 0 pending/skipped file(s). 0 subsystem record(s) retained; 0 omitted.

View automation run

@codepress-reviewer-dev

Copy link
Copy Markdown

Starting new changes review of 9cf093b8 from baseline ae89a47c... 👀

View automation run

@codepress-reviewer-devcodepress-reviewer-devBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The shared first-failure latch, separate cleanup latch, and non-rejecting encode settlement provide a coherent ownership boundary for this concurrency path: every in-flight encode is observed and its sample is closed, while primary error precedence remains stable across render, audio, encoder, abort, and cleanup races.

I also reviewed the complete expanded loop test file. Its race matrix and focused cases cover the meaningful source-order, late-rejection, drain, listener-cleanup, progress, ordering, keyframe, and sample-closure paths. I found no actionable correctness or production-safety concerns in the incremental diff.

Reviewed `ae89a47c`...`9cf093b8`. Diff collection and GitHub posting were handled by the automation.

View automation run

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@lightninglu10