mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(engine,producer): classify blank vs psnr from a structural field, not the error message
Deepwork's request-changes on #2411 (twice): deFallbackReason's blank/psnr split still ran /blank/i.test(err.message) even after this PR's stated goal of moving off message-text parsing — a reworded message, a translated string, or a differently-shaped error crossing a module boundary could silently relabel a blank failure as psnr (or vice versa), corrupting the soak's telemetry taxonomy. DrawElementVerificationDetails now carries a required `kind: "blank" | "psnr"` field, set at all three real throw sites in captureStreamingStage.ts. The orchestrator derives deFallbackReason from getDrawElementVerificationDetails's kind instead of regexing the message. Making `kind` a required constructor argument means any future throw site that omits it fails to compile, closing the gap for good rather than just at today's three call sites. New tests in frameCapture.test.ts prove message-independence directly: kind survives a reworded message that says neither "blank" nor "psnr", and stays correctly "psnr" even when the message adversarially contains the substring "blank" — the exact scenario a regex-based classifier would get wrong.
This commit is contained in:
@@ -229,12 +229,14 @@ describe("navigation diagnostics", () => {
|
||||
describe("DrawElementVerificationError details", () => {
|
||||
it("carries frameIndex/failedDb/verifyThresholdDb when provided", () => {
|
||||
const err = new DrawElementVerificationError("drawElement self-verify failed at frame 649", {
|
||||
kind: "psnr",
|
||||
frameIndex: 649,
|
||||
failedDb: 28.4,
|
||||
verifyThresholdDb: 32,
|
||||
});
|
||||
expect(isDrawElementVerificationError(err)).toBe(true);
|
||||
expect(getDrawElementVerificationDetails(err)).toEqual({
|
||||
kind: "psnr",
|
||||
frameIndex: 649,
|
||||
failedDb: 28.4,
|
||||
verifyThresholdDb: 32,
|
||||
@@ -242,8 +244,11 @@ describe("DrawElementVerificationError details", () => {
|
||||
});
|
||||
|
||||
it("omits fields that weren't supplied (blank-frame throws have no dB)", () => {
|
||||
const err = new DrawElementVerificationError("blank drawElement frame 12", { frameIndex: 12 });
|
||||
expect(getDrawElementVerificationDetails(err)).toEqual({ frameIndex: 12 });
|
||||
const err = new DrawElementVerificationError("blank drawElement frame 12", {
|
||||
kind: "blank",
|
||||
frameIndex: 12,
|
||||
});
|
||||
expect(getDrawElementVerificationDetails(err)).toEqual({ kind: "blank", frameIndex: 12 });
|
||||
});
|
||||
|
||||
it("returns undefined for a non-verification error", () => {
|
||||
@@ -252,11 +257,49 @@ describe("DrawElementVerificationError details", () => {
|
||||
|
||||
it("finds details through a wrapping cause chain (producer's CaptureStageError)", () => {
|
||||
const inner = new DrawElementVerificationError("psnr breach", {
|
||||
kind: "psnr",
|
||||
frameIndex: 5,
|
||||
failedDb: 12.1,
|
||||
});
|
||||
const wrapper = new Error("capture stage failed", { cause: inner });
|
||||
expect(isDrawElementVerificationError(wrapper)).toBe(true);
|
||||
expect(getDrawElementVerificationDetails(wrapper)).toEqual({ frameIndex: 5, failedDb: 12.1 });
|
||||
expect(getDrawElementVerificationDetails(wrapper)).toEqual({
|
||||
kind: "psnr",
|
||||
frameIndex: 5,
|
||||
failedDb: 12.1,
|
||||
});
|
||||
});
|
||||
|
||||
it("carries kind='blank'/'psnr' as a structural field, not derived from message text", () => {
|
||||
const blankErr = new DrawElementVerificationError("blank drawElement frame 12", {
|
||||
kind: "blank",
|
||||
frameIndex: 12,
|
||||
});
|
||||
const psnrErr = new DrawElementVerificationError(
|
||||
"drawElement self-verify failed at frame 649",
|
||||
{ kind: "psnr", frameIndex: 649, failedDb: 28.4, verifyThresholdDb: 32 },
|
||||
);
|
||||
expect(getDrawElementVerificationDetails(blankErr)?.kind).toBe("blank");
|
||||
expect(getDrawElementVerificationDetails(psnrErr)?.kind).toBe("psnr");
|
||||
});
|
||||
|
||||
it("kind survives even when the message text disagrees with (or omits) the word psnr/blank", () => {
|
||||
// A reworded message that says neither "blank" nor "psnr" — a regex over
|
||||
// message text would have no signal here; the structural kind must still
|
||||
// report correctly.
|
||||
const reworded = new DrawElementVerificationError("frame 12 failed verification", {
|
||||
kind: "blank",
|
||||
frameIndex: 12,
|
||||
});
|
||||
expect(getDrawElementVerificationDetails(reworded)?.kind).toBe("blank");
|
||||
|
||||
// Adversarial: a psnr failure whose message happens to mention "blank"
|
||||
// (e.g. quoting a neighboring log line) — the structural kind must not
|
||||
// flip just because the substring "blank" appears in the text.
|
||||
const adversarial = new DrawElementVerificationError(
|
||||
"drawElement self-verify failed at frame 5 (previous frame was blank-guard-accepted)",
|
||||
{ kind: "psnr", frameIndex: 5, failedDb: 12.1, verifyThresholdDb: 32 },
|
||||
);
|
||||
expect(getDrawElementVerificationDetails(adversarial)?.kind).toBe("psnr");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -203,32 +203,38 @@ export interface CaptureSession {
|
||||
*/
|
||||
/**
|
||||
* Structured detail carried alongside the human-readable message — lets
|
||||
* telemetry report the actual failing dB / frame index instead of the
|
||||
* orchestrator having to regex them back out of formatted text. All optional:
|
||||
* a blank-frame trip has no PSNR score, so `failedDb`/`verifyThresholdDb`
|
||||
* are omitted for that throw site.
|
||||
* telemetry report the actual failure kind / failing dB / frame index
|
||||
* instead of the orchestrator having to regex them back out of formatted
|
||||
* text (a message-text dependency is exactly the failure mode this shape
|
||||
* exists to close — review finding: message wording, translation, or a
|
||||
* cross-module/serialized error must never be able to flip the reported
|
||||
* kind). All fields but `kind` are optional: a blank-frame trip has no PSNR
|
||||
* score, so `failedDb`/`verifyThresholdDb` are omitted for that throw site.
|
||||
*/
|
||||
export interface DrawElementVerificationDetails {
|
||||
kind: "blank" | "psnr";
|
||||
frameIndex?: number;
|
||||
failedDb?: number;
|
||||
verifyThresholdDb?: number;
|
||||
}
|
||||
|
||||
export class DrawElementVerificationError extends Error {
|
||||
readonly kind: "blank" | "psnr";
|
||||
readonly frameIndex?: number;
|
||||
readonly failedDb?: number;
|
||||
readonly verifyThresholdDb?: number;
|
||||
|
||||
constructor(message: string, details?: DrawElementVerificationDetails) {
|
||||
constructor(message: string, details: DrawElementVerificationDetails) {
|
||||
super(message);
|
||||
this.name = "DrawElementVerificationError";
|
||||
// Discriminant property, assigned dynamically: isDrawElementVerificationError
|
||||
// reads it structurally so detection survives duplicated module instances
|
||||
// across package boundaries (where instanceof fails).
|
||||
(this as unknown as { deVerificationFailure: boolean }).deVerificationFailure = true;
|
||||
this.frameIndex = details?.frameIndex;
|
||||
this.failedDb = details?.failedDb;
|
||||
this.verifyThresholdDb = details?.verifyThresholdDb;
|
||||
this.kind = details.kind;
|
||||
this.frameIndex = details.frameIndex;
|
||||
this.failedDb = details.failedDb;
|
||||
this.verifyThresholdDb = details.verifyThresholdDb;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -255,7 +261,14 @@ export function getDrawElementVerificationDetails(
|
||||
for (let depth = 0; depth < 5 && typeof e === "object" && e !== null; depth++) {
|
||||
const rec = e as { deVerificationFailure?: boolean } & Partial<DrawElementVerificationDetails>;
|
||||
if (rec.deVerificationFailure === true) {
|
||||
const details: DrawElementVerificationDetails = {};
|
||||
// Every construction path sets `kind` (required on the constructor), so
|
||||
// this only defends against a malformed cross-module-instance shape —
|
||||
// treat anything other than exactly "blank" as "psnr", the same
|
||||
// fallback polarity the old message regex had, but driven by a
|
||||
// structural field instead of parsing text.
|
||||
const details: DrawElementVerificationDetails = {
|
||||
kind: rec.kind === "blank" ? "blank" : "psnr",
|
||||
};
|
||||
if (typeof rec.frameIndex === "number") details.frameIndex = rec.frameIndex;
|
||||
if (typeof rec.failedDb === "number") details.failedDb = rec.failedDb;
|
||||
if (typeof rec.verifyThresholdDb === "number")
|
||||
|
||||
@@ -289,7 +289,7 @@ function createDrainFrameGuard(args: {
|
||||
} catch (err) {
|
||||
throw new DrawElementVerificationError(
|
||||
`blank drawElement frame ${idx}: ${buf.length}B < floor ${Math.round(floor)}B and recapture failed (${err instanceof Error ? err.message : String(err)})`,
|
||||
{ frameIndex: idx },
|
||||
{ kind: "blank", frameIndex: idx },
|
||||
);
|
||||
}
|
||||
if (retryBuf.equals(buf)) {
|
||||
@@ -307,7 +307,7 @@ function createDrainFrameGuard(args: {
|
||||
} else if (retryBuf.length < floor) {
|
||||
throw new DrawElementVerificationError(
|
||||
`blank drawElement frame ${idx}: ${buf.length}B (retry ${retryBuf.length}B) < floor ${Math.round(floor)}B`,
|
||||
{ frameIndex: idx },
|
||||
{ kind: "blank", frameIndex: idx },
|
||||
);
|
||||
} else {
|
||||
buf = retryBuf;
|
||||
@@ -341,7 +341,7 @@ function createDrainFrameGuard(args: {
|
||||
}
|
||||
throw new DrawElementVerificationError(
|
||||
`drawElement self-verify failed at frame ${idx}: ${db.toFixed(1)}dB < ${verifyMinDb}dB vs pre-injection screenshot${dumpDir ? ` (pair: ${dumpDir})` : ""}`,
|
||||
{ frameIndex: idx, failedDb: db, verifyThresholdDb: verifyMinDb },
|
||||
{ kind: "psnr", frameIndex: idx, failedDb: db, verifyThresholdDb: verifyMinDb },
|
||||
);
|
||||
}
|
||||
stats.verifyChecked += 1;
|
||||
|
||||
@@ -2729,15 +2729,17 @@ export async function executeRenderJob(
|
||||
throw err;
|
||||
const isMemoryExhaustion = !isVerifyError && isMemoryExhaustionError(err);
|
||||
deSelfVerifyFallback = isVerifyError;
|
||||
// `kind` is a structural field on the error (DrawElementVerificationDetails),
|
||||
// never derived from message text — a reworded message, a translated
|
||||
// string, or a cross-module/serialized error must never be able to
|
||||
// flip "blank" into "psnr" or vice versa (review finding).
|
||||
const verifyDetails = isVerifyError ? getDrawElementVerificationDetails(err) : undefined;
|
||||
deFallbackReason = isVerifyError
|
||||
? /blank/i.test(err instanceof Error ? err.message : "")
|
||||
? "blank"
|
||||
: "psnr"
|
||||
? (verifyDetails?.kind ?? "psnr")
|
||||
: isMemoryExhaustion
|
||||
? "oom"
|
||||
: "capture_error";
|
||||
if (isVerifyError) {
|
||||
const verifyDetails = getDrawElementVerificationDetails(err);
|
||||
deFallbackFailedDb = roundDb(verifyDetails?.failedDb);
|
||||
deFallbackFrameIndex = verifyDetails?.frameIndex;
|
||||
deFallbackThresholdDb = roundDb(verifyDetails?.verifyThresholdDb);
|
||||
|
||||
Reference in New Issue
Block a user