mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(engine,producer,cli): close review gaps in sub-timeline fail-fast
Address PR #2045 review feedback: - Share a SubTimelineWaitOutcome type (engine) end-to-end instead of widening to string across CapturePerfSummary / RenderPerfSummary / telemetry, so the three layers can't drift. - Dedupe scriptLoadFailures on push — a 4xx response and its trailing requestfailed both recorded the same URL, doubling the failed-URL list in the fail-fast warning. - Thread the sub-timeline-wait outcome into render_error (not just render_complete): a render that fail-fasts and then fails downstream (pollVideosReady, extract, encode) previously dropped this signal on the floor. dedupPerfs is now function-scoped so the catch path can read it, same treatment as the existing captureAttempts array.
This commit is contained in:
@@ -1,8 +1,11 @@
|
||||
import { redactTelemetryString, type OutputResolutionIssueKind } from "@hyperframes/core";
|
||||
import type { SubTimelineWaitOutcome } from "@hyperframes/engine";
|
||||
import { trackEvent } from "./client.js";
|
||||
import { readConfig } from "./config.js";
|
||||
|
||||
export interface RenderObservabilityTelemetryPayload {
|
||||
/** Worst sub-composition timeline wait outcome across sessions. */
|
||||
subTimelineWait?: SubTimelineWaitOutcome;
|
||||
observabilityRenderJobId?: string;
|
||||
observabilityCompositionHash?: string;
|
||||
observabilityEventCount?: number;
|
||||
@@ -47,6 +50,7 @@ export interface RenderObservabilityTelemetryPayload {
|
||||
|
||||
function renderObservabilityEventProperties(props: RenderObservabilityTelemetryPayload) {
|
||||
return {
|
||||
sub_timeline_wait: props.subTimelineWait,
|
||||
observability_render_job_id: props.observabilityRenderJobId,
|
||||
observability_composition_hash: props.observabilityCompositionHash,
|
||||
observability_event_count: props.observabilityEventCount,
|
||||
@@ -148,7 +152,6 @@ export function trackRenderComplete(
|
||||
captureAvgMs?: number;
|
||||
/** Warmup-robust per-frame capture median (basis for speedup estimates). */
|
||||
captureP50Ms?: number;
|
||||
subTimelineWait?: string;
|
||||
/** <video> element count (speedup segmentation: injection comps read lower). */
|
||||
videoCount?: number;
|
||||
capturePeakMs?: number;
|
||||
@@ -222,7 +225,6 @@ export function trackRenderComplete(
|
||||
speed_ratio: props.speedRatio,
|
||||
capture_avg_ms: props.captureAvgMs,
|
||||
capture_p50_ms: props.captureP50Ms,
|
||||
sub_timeline_wait: props.subTimelineWait,
|
||||
video_count: props.videoCount,
|
||||
capture_peak_ms: props.capturePeakMs,
|
||||
peak_memory_mb: props.peakMemoryMb,
|
||||
|
||||
@@ -58,7 +58,10 @@ export function renderObservabilityTelemetryPayload(
|
||||
export function renderJobObservabilityTelemetryPayload(
|
||||
job: RenderJob | undefined,
|
||||
): RenderObservabilityTelemetryPayload {
|
||||
return renderObservabilityTelemetryPayload(
|
||||
job?.errorDetails?.observability ?? job?.perfSummary?.observability,
|
||||
);
|
||||
return {
|
||||
...renderObservabilityTelemetryPayload(
|
||||
job?.errorDetails?.observability ?? job?.perfSummary?.observability,
|
||||
),
|
||||
subTimelineWait: job?.errorDetails?.subTimelineWait ?? job?.perfSummary?.subTimelineWait,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -40,6 +40,7 @@ export type {
|
||||
CaptureResult,
|
||||
CaptureBufferResult,
|
||||
CapturePerfSummary,
|
||||
SubTimelineWaitOutcome,
|
||||
} from "./types.js";
|
||||
|
||||
// ── Configuration ──────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -50,6 +50,7 @@ import type {
|
||||
CaptureResult,
|
||||
CaptureBufferResult,
|
||||
CapturePerfSummary,
|
||||
SubTimelineWaitOutcome,
|
||||
} from "../types.js";
|
||||
|
||||
export type { CaptureOptions, CaptureResult, CaptureBufferResult, CapturePerfSummary };
|
||||
@@ -104,7 +105,7 @@ export interface CaptureSession {
|
||||
*/
|
||||
scriptLoadFailures: string[];
|
||||
/** Outcome of the sub-composition timeline wait: ready | timeout | script_failure. */
|
||||
subTimelineWaitOutcome?: "ready" | "timeout" | "script_failure";
|
||||
subTimelineWaitOutcome?: SubTimelineWaitOutcome;
|
||||
initTelemetry?: {
|
||||
initDurationMs: number;
|
||||
tweenCount: number;
|
||||
@@ -1159,7 +1160,7 @@ export async function pollSubCompositionTimelines(
|
||||
// is cut to `scriptFailureGraceMs` from its start.
|
||||
getScriptLoadFailures?: () => readonly string[],
|
||||
scriptFailureGraceMs: number = 2_000,
|
||||
): Promise<"ready" | "timeout" | "script_failure"> {
|
||||
): Promise<SubTimelineWaitOutcome> {
|
||||
// Hosts may opt out of the timeline wait with `data-no-timeline` —
|
||||
// compositions driven purely by CSS animations / rAF (the render-compat
|
||||
// contract) never register window.__timelines[id], and without the opt-out
|
||||
@@ -1408,6 +1409,16 @@ async function waitForOptionalTailwindReady(page: Page, timeoutMs: number): Prom
|
||||
}
|
||||
}
|
||||
|
||||
// A 4xx `response` and a `requestfailed` can both fire for the same script
|
||||
// (e.g. a `requestfailed` following the 4xx), and repeated <script> tags for
|
||||
// the same URL duplicate it further — dedupe so the fail-fast warning names
|
||||
// each failed URL once.
|
||||
function recordScriptLoadFailure(session: CaptureSession, url: string): void {
|
||||
if (!session.scriptLoadFailures.includes(url)) {
|
||||
session.scriptLoadFailures.push(url);
|
||||
}
|
||||
}
|
||||
|
||||
// fallow-ignore-next-line unit-size
|
||||
export async function initializeSession(session: CaptureSession): Promise<void> {
|
||||
const { page, serverUrl } = session;
|
||||
@@ -1439,7 +1450,7 @@ export async function initializeSession(session: CaptureSession): Promise<void>
|
||||
|
||||
page.on("requestfailed", (request) => {
|
||||
if (request.resourceType() === "script") {
|
||||
session.scriptLoadFailures.push(request.url());
|
||||
recordScriptLoadFailure(session, request.url());
|
||||
}
|
||||
appendBrowserDiagnostic(
|
||||
session,
|
||||
@@ -1458,7 +1469,7 @@ export async function initializeSession(session: CaptureSession): Promise<void>
|
||||
|
||||
const request = response.request();
|
||||
if (request.resourceType() === "script") {
|
||||
session.scriptLoadFailures.push(response.url());
|
||||
recordScriptLoadFailure(session, response.url());
|
||||
}
|
||||
appendBrowserDiagnostic(
|
||||
session,
|
||||
|
||||
@@ -6,6 +6,13 @@
|
||||
*/
|
||||
import type { Fps } from "@hyperframes/core";
|
||||
|
||||
/**
|
||||
* Outcome of waiting for a sub-composition's GSAP timelines to register.
|
||||
* Threaded string-typed through `CapturePerfSummary` / `RenderPerfSummary` /
|
||||
* telemetry so a single alias keeps the values in sync end-to-end.
|
||||
*/
|
||||
export type SubTimelineWaitOutcome = "ready" | "timeout" | "script_failure";
|
||||
|
||||
// ── Seek Protocol ──────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
@@ -183,8 +190,8 @@ export interface CapturePerfSummary {
|
||||
* averages). Basis for in-the-wild speedup estimates. 0 when no frames.
|
||||
*/
|
||||
p50TotalMs: number;
|
||||
/** Sub-composition timeline wait outcome: ready | timeout | script_failure (absent pre-init). */
|
||||
subTimelineWaitOutcome?: string;
|
||||
/** Sub-composition timeline wait outcome (absent pre-init). */
|
||||
subTimelineWaitOutcome?: SubTimelineWaitOutcome;
|
||||
/**
|
||||
* Frames served from the static-dedup cache instead of a real seek+screenshot
|
||||
* (opt-out HF_STATIC_DEDUP=false). 0 when dedup was off or never armed. NOT counted
|
||||
|
||||
@@ -5,7 +5,11 @@
|
||||
|
||||
import { rmSync } from "node:fs";
|
||||
import { freemem } from "node:os";
|
||||
import { type CaptureSession, closeCaptureSession } from "@hyperframes/engine";
|
||||
import {
|
||||
type CaptureSession,
|
||||
type SubTimelineWaitOutcome,
|
||||
closeCaptureSession,
|
||||
} from "@hyperframes/engine";
|
||||
import type { FileServerHandle } from "../fileServer.js";
|
||||
import { defaultLogger, type ProducerLogger } from "../../logger.js";
|
||||
import type { HdrDiagnostics, RenderJob } from "../renderOrchestrator.js";
|
||||
@@ -82,6 +86,7 @@ export function buildRenderErrorDetails(input: {
|
||||
perfStages: Record<string, number>;
|
||||
hdrDiagnostics: HdrDiagnostics;
|
||||
observability?: RenderObservabilitySummary;
|
||||
subTimelineWait?: SubTimelineWaitOutcome;
|
||||
}): NonNullable<RenderJob["errorDetails"]> {
|
||||
const errorMessage = normalizeErrorMessage(input.error);
|
||||
const errorStack = input.error instanceof Error ? input.error.stack : undefined;
|
||||
@@ -99,5 +104,6 @@ export function buildRenderErrorDetails(input: {
|
||||
? { ...input.hdrDiagnostics }
|
||||
: undefined,
|
||||
observability: input.observability,
|
||||
subTimelineWait: input.subTimelineWait,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
*/
|
||||
|
||||
import { fpsToNumber } from "@hyperframes/core";
|
||||
import type { CapturePerfSummary } from "@hyperframes/engine";
|
||||
import type { CapturePerfSummary, SubTimelineWaitOutcome } from "@hyperframes/engine";
|
||||
import type { CaptureCalibrationSample, CaptureCostEstimate } from "./captureCost.js";
|
||||
import type {
|
||||
CaptureAttemptSummary,
|
||||
@@ -15,6 +15,25 @@ import type {
|
||||
import { type HdrPerfCollector, finalizeHdrPerf } from "./hdrPerf.js";
|
||||
import type { RenderObservabilitySummary } from "./observability.js";
|
||||
|
||||
/**
|
||||
* Worst sub-composition timeline wait outcome across sessions: script_failure
|
||||
* > timeout > ready. Shared by the success-path perf summary and the error
|
||||
* path (a fail-fast can still be followed by an unrelated downstream
|
||||
* failure, e.g. in `pollVideosReady` or encode — that render fires
|
||||
* `render_error`, not `render_complete`, and should carry this too).
|
||||
*/
|
||||
export function worstSubTimelineWaitOutcome(
|
||||
perfs: CapturePerfSummary[],
|
||||
): SubTimelineWaitOutcome | undefined {
|
||||
const outcomes = perfs
|
||||
.map((p) => p.subTimelineWaitOutcome)
|
||||
.filter((o): o is SubTimelineWaitOutcome => !!o);
|
||||
if (outcomes.length === 0) return undefined;
|
||||
if (outcomes.includes("script_failure")) return "script_failure";
|
||||
if (outcomes.includes("timeout")) return "timeout";
|
||||
return "ready";
|
||||
}
|
||||
|
||||
/**
|
||||
* Append each parallel worker's static-dedup perf into the render-level sink
|
||||
* (skipping workers that reported none). Shared by the disk + streaming parallel
|
||||
@@ -189,16 +208,7 @@ export function buildRenderPerfSummary(input: {
|
||||
input.totalFrames,
|
||||
)
|
||||
: undefined,
|
||||
subTimelineWait: (() => {
|
||||
const outcomes = input.dedupPerfs
|
||||
.map((p) => p.subTimelineWaitOutcome)
|
||||
.filter((o): o is string => !!o);
|
||||
if (outcomes.length === 0) return undefined;
|
||||
// Worst outcome wins: script_failure > timeout > ready.
|
||||
if (outcomes.includes("script_failure")) return "script_failure";
|
||||
if (outcomes.includes("timeout")) return "timeout";
|
||||
return "ready";
|
||||
})(),
|
||||
subTimelineWait: worstSubTimelineWaitOutcome(input.dedupPerfs),
|
||||
captureP50Ms: (() => {
|
||||
// Per-frame median from the engine's samples; when parallel workers
|
||||
// report separately, take the busiest session's median.
|
||||
|
||||
@@ -67,6 +67,7 @@ import {
|
||||
LOW_MEMORY_TOTAL_MB_THRESHOLD,
|
||||
assertConfiguredFfmpegBinariesExist,
|
||||
type CapturePerfSummary,
|
||||
type SubTimelineWaitOutcome,
|
||||
resolveBrowserGpuMode,
|
||||
resolveHeadlessShellPath,
|
||||
scaleProtocolTimeoutForComposition,
|
||||
@@ -90,7 +91,11 @@ import { buildRenderErrorDetails, cleanupRenderResources, safeCleanup } from "./
|
||||
import { normalizeErrorMessage } from "../utils/errorMessage.js";
|
||||
import { formatCaptureFrameName } from "../utils/paths.js";
|
||||
import { resolveEffectiveHdrMode } from "./render/hdrMode.js";
|
||||
import { buildRenderPerfSummary, pushWorkerDedupPerfs } from "./render/perfSummary.js";
|
||||
import {
|
||||
buildRenderPerfSummary,
|
||||
pushWorkerDedupPerfs,
|
||||
worstSubTimelineWaitOutcome,
|
||||
} from "./render/perfSummary.js";
|
||||
import { getCaptureStageBrowserConsole } from "./render/captureStageError.js";
|
||||
import { resolveVideoCaptureBeyondViewport } from "./render/captureBeyondViewport.js";
|
||||
import {
|
||||
@@ -322,8 +327,8 @@ export interface RenderPerfSummary {
|
||||
* captured the most frames when parallel workers report separately.
|
||||
*/
|
||||
captureP50Ms?: number;
|
||||
/** Worst sub-composition timeline wait outcome across sessions: ready | timeout | script_failure. */
|
||||
subTimelineWait?: string;
|
||||
/** Worst sub-composition timeline wait outcome across sessions. */
|
||||
subTimelineWait?: SubTimelineWaitOutcome;
|
||||
capturePeakMs?: number;
|
||||
captureCalibration?: {
|
||||
sampledFrames: number[];
|
||||
@@ -457,6 +462,8 @@ export interface RenderJob {
|
||||
perfStages?: Record<string, number>;
|
||||
hdrDiagnostics?: HdrDiagnostics;
|
||||
observability?: RenderObservabilitySummary;
|
||||
/** Worst sub-composition timeline wait outcome across sessions captured before the failure. */
|
||||
subTimelineWait?: SubTimelineWaitOutcome;
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1189,6 +1196,11 @@ export async function executeRenderJob(
|
||||
// can read it — the catch records transient-retry burn on renders that still
|
||||
// failed, which is the more actionable signal for tuning the retry cap.
|
||||
const captureAttempts: CaptureAttemptSummary[] = [];
|
||||
// Static-dedup perf, appended per sequential session / per parallel worker
|
||||
// by the capture stage. Also function-scoped so the catch block can read
|
||||
// the sub-timeline-wait outcome for a render that fails downstream of a
|
||||
// fail-fast (aggregated into the success-path perf summary below too).
|
||||
const dedupPerfs: CapturePerfSummary[] = [];
|
||||
const recordTransientRetryObservability = (): void => {
|
||||
const count = captureAttempts.filter((a) => a.reason === "transient-retry").length;
|
||||
if (count > 0) updateCaptureObservability({ transientRetries: count });
|
||||
@@ -1824,11 +1836,6 @@ export async function executeRenderJob(
|
||||
}
|
||||
}
|
||||
|
||||
// `captureAttempts` is declared at function scope above (shared with the
|
||||
// catch block). Static-dedup perf, appended per sequential session / per
|
||||
// parallel worker by the capture stage, aggregated into the perf summary below.
|
||||
const dedupPerfs: CapturePerfSummary[] = [];
|
||||
|
||||
// png-sequence is "no container" — outputPath is treated as a directory and
|
||||
// the encode/mux/faststart stages are skipped entirely. The empty extension
|
||||
// keeps `videoOnlyPath` (which is constructed below) sensible even though
|
||||
@@ -2434,6 +2441,7 @@ export async function executeRenderJob(
|
||||
perfStages,
|
||||
hdrDiagnostics,
|
||||
observability: observabilitySummary,
|
||||
subTimelineWait: worstSubTimelineWaitOutcome(dedupPerfs),
|
||||
});
|
||||
|
||||
log.info("[Render] Failure summary", {
|
||||
|
||||
Reference in New Issue
Block a user