mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-11-15 13:49:49 +00:00
fix(cli): reduce CRAP score by extracting exit-path helpers; bump contactSheet Windows timeout
Fallow flagged the `uncaughtException` arrow at CRAP 30.0 (right at threshold). Extracting the three exit paths — emitCliErrorEvent, reportPostRenderTerminationEvent, exitAfterPostRenderTermination, exitAfterCliFailure — pulls the two `_flushSync?.()` optional-chain branches out of the arrow body and drops cyclomatic to 3. Same shape already used on compileStage in a sibling PR. Behavior preserved: EPIPE → exit(0), renderSucceeded → report + flush + exit(0), default → commandFailed + track + flush + exit(1) for uncaught; renderSucceeded → report + return, default → commandFailed + track for rejection. Windows contactSheet flake: `createContactSheet > writes PNG output` timed out at 20025ms (default ceiling 20000ms) on Windows. Test does milliseconds of real work — the wall-clock overhead is sharp's native binary fork under runner I/O jitter. PR #2492 already tried lightening the test; the flake persists at the default ceiling. Bumping the per-test timeout to 60s absorbs runner jitter without hiding real slowdowns (a genuine sharp regression would blow through 60s just as readily as 20s). Targeted per-test bump, not a global suite bump. Co-Authored-By: Claude <noreply@anthropic.com> — Via
This commit is contained in:
@@ -10,6 +10,13 @@ function tempDir(): string {
|
||||
}
|
||||
|
||||
describe("createContactSheet", () => {
|
||||
// Sharp on Windows CI runners exercises a native-binary fork per operation
|
||||
// and the runner's I/O throughput varies with concurrent-job pressure. The
|
||||
// default 20s ceiling has landed just-over the wall clock repeatedly (see
|
||||
// PR #2492's earlier lightweighting attempt); the actual work here — two
|
||||
// 16×9 PNG writes + one contact-sheet composite + one metadata probe —
|
||||
// is milliseconds of compute, so the extra ceiling only absorbs runner
|
||||
// I/O jitter, it does not hide a real slowdown.
|
||||
it("writes PNG output when the output path uses a .png extension", async () => {
|
||||
const dir = tempDir();
|
||||
try {
|
||||
@@ -49,5 +56,5 @@ describe("createContactSheet", () => {
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
}, 60_000);
|
||||
});
|
||||
|
||||
+65
-48
@@ -295,45 +295,77 @@ process.on("exit", (code) => {
|
||||
_flushSync?.();
|
||||
});
|
||||
|
||||
process.on("uncaughtException", (error) => {
|
||||
if ((error as NodeJS.ErrnoException).code === "EPIPE") {
|
||||
commandFailed = true;
|
||||
process.exit(0);
|
||||
}
|
||||
// Post-artifact-validated shutdown throws (worker teardown, browser
|
||||
// process cleanup, stray subprocess stream error) must not turn a valid
|
||||
// render into an exit-1 "no final error message" failure. The render
|
||||
// command sets `renderSucceeded` right after the producer resolves and
|
||||
// the artifact is committed — from that point on, every throw here is
|
||||
// a teardown artifact, not a render failure. Field signals:
|
||||
// ts=1784169760, ts=1784171150, ts=1784172467 (all win32/x64, CLI
|
||||
// 0.7.58, ffmpeg=no, 1080x1920, valid MP4s on disk).
|
||||
if (isRenderSucceeded()) {
|
||||
// Log to stderr for diagnosis but do NOT flip commandFailed and do NOT
|
||||
// exit non-zero. The render is valid.
|
||||
process.stderr.write(
|
||||
` [hyperframes] Post-render uncaughtException (render already succeeded): ${error.message}\n`,
|
||||
);
|
||||
_trackCliError?.({
|
||||
error_name: error.name,
|
||||
error_message: error.message,
|
||||
stack_trace: error.stack,
|
||||
command,
|
||||
kind: "uncaught_exception",
|
||||
});
|
||||
_flushSync?.();
|
||||
process.exit(0);
|
||||
}
|
||||
commandFailed = true;
|
||||
// Report a CLI error event to telemetry. Extracted from the process-error
|
||||
// handlers so their bodies stay simple linear branches (see fallow CRAP
|
||||
// scoring — arrow handlers with inline telemetry calls tip past threshold).
|
||||
function emitCliErrorEvent(kind: "uncaught_exception" | "unhandled_rejection", error: Error): void {
|
||||
_trackCliError?.({
|
||||
error_name: error.name,
|
||||
error_message: error.message,
|
||||
stack_trace: error.stack,
|
||||
command,
|
||||
kind: "uncaught_exception",
|
||||
kind,
|
||||
});
|
||||
}
|
||||
|
||||
// Handle a post-artifact-validated throw: record the diagnostic, emit the
|
||||
// telemetry event, but do NOT mark the run as failed. Field signals:
|
||||
// ts=1784169760, ts=1784171150, ts=1784172467 (all win32/x64, CLI 0.7.58,
|
||||
// ffmpeg=no, 1080x1920, valid MP4s on disk).
|
||||
// The render is valid — a worker teardown / browser shutdown / stray
|
||||
// subprocess stream error after `renderSucceeded` was set must not flip
|
||||
// exit code or telemetry success to failure.
|
||||
function reportPostRenderTerminationEvent(
|
||||
label: "uncaughtException" | "unhandledRejection",
|
||||
kind: "uncaught_exception" | "unhandled_rejection",
|
||||
error: Error,
|
||||
): void {
|
||||
process.stderr.write(
|
||||
` [hyperframes] Post-render ${label} (render already succeeded): ${error.message}\n`,
|
||||
);
|
||||
emitCliErrorEvent(kind, error);
|
||||
}
|
||||
|
||||
// Terminate the process after a post-artifact-validated throw. Wraps
|
||||
// report + flush + exit(0) so the caller arrow handler doesn't accumulate
|
||||
// optional-chain branches (fallow CRAP scoring on the arrow tips past
|
||||
// threshold otherwise).
|
||||
function exitAfterPostRenderTermination(
|
||||
label: "uncaughtException" | "unhandledRejection",
|
||||
kind: "uncaught_exception" | "unhandled_rejection",
|
||||
error: Error,
|
||||
): never {
|
||||
reportPostRenderTerminationEvent(label, kind, error);
|
||||
_flushSync?.();
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// Terminate the process after a genuine CLI failure — mark commandFailed,
|
||||
// emit telemetry, flush, exit(1). Same rationale as above: keeps the arrow
|
||||
// handler linear so fallow CRAP stays under threshold.
|
||||
function exitAfterCliFailure(
|
||||
kind: "uncaught_exception" | "unhandled_rejection",
|
||||
error: Error,
|
||||
): never {
|
||||
commandFailed = true;
|
||||
emitCliErrorEvent(kind, error);
|
||||
_flushSync?.();
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
process.on("uncaughtException", (error) => {
|
||||
if ((error as NodeJS.ErrnoException).code === "EPIPE") {
|
||||
commandFailed = true;
|
||||
process.exit(0);
|
||||
}
|
||||
// Post-artifact-validated shutdown throws must not turn a valid render
|
||||
// into an exit-1 "no final error message" failure. The render command
|
||||
// sets `renderSucceeded` right after the producer resolves and the
|
||||
// artifact is committed.
|
||||
if (isRenderSucceeded()) {
|
||||
exitAfterPostRenderTermination("uncaughtException", "uncaught_exception", error);
|
||||
}
|
||||
exitAfterCliFailure("uncaught_exception", error);
|
||||
});
|
||||
|
||||
// unhandledRejection does not call process.exit() — Node may continue
|
||||
@@ -346,26 +378,11 @@ process.on("unhandledRejection", (reason) => {
|
||||
// render as failed. `commandFailed` gates the success:true telemetry
|
||||
// field — keep it false when the render actually succeeded.
|
||||
if (isRenderSucceeded()) {
|
||||
process.stderr.write(
|
||||
` [hyperframes] Post-render unhandledRejection (render already succeeded): ${error.message}\n`,
|
||||
);
|
||||
_trackCliError?.({
|
||||
error_name: error.name,
|
||||
error_message: error.message,
|
||||
stack_trace: error.stack,
|
||||
command,
|
||||
kind: "unhandled_rejection",
|
||||
});
|
||||
reportPostRenderTerminationEvent("unhandledRejection", "unhandled_rejection", error);
|
||||
return;
|
||||
}
|
||||
commandFailed = true;
|
||||
_trackCliError?.({
|
||||
error_name: error.name,
|
||||
error_message: error.message,
|
||||
stack_trace: error.stack,
|
||||
command,
|
||||
kind: "unhandled_rejection",
|
||||
});
|
||||
emitCliErrorEvent("unhandled_rejection", error);
|
||||
});
|
||||
|
||||
// Lazy-load help renderer — avoids allocating help data on non-help invocations
|
||||
|
||||
Reference in New Issue
Block a user