Skip to content

fix(engine): preserve streaming encoder termination diagnostics - #3751

Closed
jrusso1020 wants to merge 1 commit into
mainfrom
fix/3744-streaming-encoder
Closed

fix(engine): preserve streaming encoder termination diagnostics#3751
jrusso1020 wants to merge 1 commit into
mainfrom
fix/3744-streaming-encoder

Conversation

@jrusso1020

Copy link
Copy Markdown
Collaborator

What

Preserve the streaming encoder's termination reason and frame counts in both getExitError() (used by capture-loop early failures) and close(). Watchdog termination remains a failure even if the child exits with code zero.

Why

Addresses the remaining observability gap in #3744. On current main, ManagedChildProcess knows whether it requested termination for inactivity or cancellation, but getExitError() discards that reason. close() only includes the timeout context on its nonzero-exit path. Neither error records how many frames were attempted or accepted.

The reported v0.6.52 also had an older backpressure bug: it reset the watchdog only when stdin.write() returned true, never after a buffered write drained, and capture callers ignored the return value. #1372 already added drain waiting, heartbeat refresh after drain, and capture-loop stop guards. That is a plausible mechanism for the old report, but the original composition is unavailable and the actual SIGTERM sender cannot be established from the supplied logs.

How

Snapshot attempted/accepted frame counts and pending-drain state immediately before a managed termination request, or at process settlement for other exits. Accepted counts writes queued by Node, including writes returning false; it does not claim FFmpeg encoded those frames. Late write attempts cannot overwrite the snapshot. Both error surfaces share the formatter and preserve external-interruption retry classification.

For slow software capture, use a current release with #1372 first. A progressing render can exceed the total inactivity budget. If disk-backed capture is needed, PRODUCER_ENABLE_STREAMING_ENCODE=false selects that path; PRODUCER_ENABLE_CHUNKED_ENCODE=true alone does not disable streaming. This change leaves those defaults unchanged.

Test plan

  • Unit tests added/updated: four regression assertions fail against unmodified main; all pass with this change.
  • 76 engine tests pass: streamingEncoder, managedChildProcess, runFfmpeg.
  • Four producer ensureFrameWritten tests pass.
  • Engine TypeScript check, changed-file oxlint/oxfmt, and diff whitespace check pass.
  • Manual testing performed: original production composition was not supplied; no claim of reproducing its DOM-heavy render.
  • Documentation update not applicable: diagnostics use the existing error API.

@jrusso1020 jrusso1020 closed this Sep 7, 2026
Sign up for free to 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