mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-13 07:40:06 +00:00
fix(engine): carry programmatic forceScreenshot opt-out to concrete-resolved site
Miguel R3 blocker on #2359: the runtime helper only checked the env opt-out (PRODUCER_FORCE_SCREENSHOT=false), silently defeating the documented programmatic escape hatch (overrides.forceScreenshot === false) on the browserGpuMode:'auto' → software probe path. At the concrete-resolution site the boolean forceScreenshot === false is ambiguous between default and explicit opt-out — resolveConfig sees the provenance but the runtime helper does not. Fix: persist provenance on the resolved config. - New INTERNAL EngineConfig field forceScreenshotExplicitlyOptedOut, set by resolveConfig when EITHER env or programmatic explicit-false is present. Purpose-documented in the type as 'not intended to be set by callers'. - shouldClampToScreenshotForConcreteGpu gains an opts.programmaticOptOut parameter; returns false early when set. Env stays as the third arg (backward compatibility with existing tests). - frameCapture.ts and renderOrchestrator.ts pass config.forceScreenshotExplicitlyOptedOut through at both call sites, so the auto→software probe path preserves the same escape hatches as literal browserGpuMode:'software'. New tests: 5 additional cases across the helper (programmatic opt-out alone; programmatic beats missing env) and resolveConfig provenance (programmatic sets flag; env sets flag; neither leaves it undefined). Local: 61/61 engine config tests pass (was 56).
This commit is contained in:
@@ -395,6 +395,54 @@ describe("resolveConfig", () => {
|
|||||||
expect(shouldClampToScreenshotForConcreteGpu("software", false, env)).toBe(true);
|
expect(shouldClampToScreenshotForConcreteGpu("software", false, env)).toBe(true);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("honors the programmatic opt-out via opts.programmaticOptOut on software", () => {
|
||||||
|
// The auto→software probe path is what this really guards: `resolveConfig`
|
||||||
|
// sets `forceScreenshotExplicitlyOptedOut = true` when the caller passed
|
||||||
|
// `overrides.forceScreenshot === false`, and the helper reads it here so
|
||||||
|
// the concrete-resolution route matches the config-time behavior.
|
||||||
|
expect(
|
||||||
|
shouldClampToScreenshotForConcreteGpu("software", false, {} as NodeJS.ProcessEnv, {
|
||||||
|
programmaticOptOut: true,
|
||||||
|
}),
|
||||||
|
).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("programmatic opt-out beats a missing env opt-out (both escape hatches independent)", () => {
|
||||||
|
// Even with no env opt-out set, a programmatic opt-out preserves BeginFrame-
|
||||||
|
// on-software debugging on the auto→software probe path.
|
||||||
|
expect(
|
||||||
|
shouldClampToScreenshotForConcreteGpu(
|
||||||
|
"software",
|
||||||
|
false,
|
||||||
|
{ PRODUCER_FORCE_SCREENSHOT: "true" } as NodeJS.ProcessEnv,
|
||||||
|
{ programmaticOptOut: true },
|
||||||
|
),
|
||||||
|
).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("forceScreenshotExplicitlyOptedOut provenance", () => {
|
||||||
|
it("is set to true when programmatic override forceScreenshot=false is passed", () => {
|
||||||
|
setEnv("PRODUCER_BROWSER_GPU_MODE", "hardware");
|
||||||
|
unsetEnv("PRODUCER_FORCE_SCREENSHOT");
|
||||||
|
const config = resolveConfig({ forceScreenshot: false });
|
||||||
|
expect(config.forceScreenshotExplicitlyOptedOut).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("is set to true when env PRODUCER_FORCE_SCREENSHOT=false is set", () => {
|
||||||
|
setEnv("PRODUCER_BROWSER_GPU_MODE", "hardware");
|
||||||
|
setEnv("PRODUCER_FORCE_SCREENSHOT", "false");
|
||||||
|
const config = resolveConfig();
|
||||||
|
expect(config.forceScreenshotExplicitlyOptedOut).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("stays unset when neither opt-out is present (default)", () => {
|
||||||
|
setEnv("PRODUCER_BROWSER_GPU_MODE", "hardware");
|
||||||
|
unsetEnv("PRODUCER_FORCE_SCREENSHOT");
|
||||||
|
const config = resolveConfig();
|
||||||
|
expect(config.forceScreenshotExplicitlyOptedOut).toBeUndefined();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("lowMemoryMode", () => {
|
describe("lowMemoryMode", () => {
|
||||||
|
|||||||
@@ -88,6 +88,19 @@ export interface EngineConfig {
|
|||||||
* opt-out. Not intended to be set by callers.
|
* opt-out. Not intended to be set by callers.
|
||||||
*/
|
*/
|
||||||
pageSideCompositingAutoDisabled?: boolean;
|
pageSideCompositingAutoDisabled?: boolean;
|
||||||
|
/**
|
||||||
|
* INTERNAL. Set to `true` by `resolveConfig` when the caller explicitly
|
||||||
|
* opted out of the software-GPU→screenshot clamp — either via env
|
||||||
|
* `PRODUCER_FORCE_SCREENSHOT=false` or programmatic
|
||||||
|
* `overrides.forceScreenshot === false`. The concrete-resolved-GPU helper
|
||||||
|
* (`shouldClampToScreenshotForConcreteGpu`) reads this so the
|
||||||
|
* `browserGpuMode:"auto"` → software probe path preserves the same
|
||||||
|
* escape hatch as literal `browserGpuMode:"software"` (the boolean
|
||||||
|
* `forceScreenshot === false` at that point is otherwise ambiguous —
|
||||||
|
* default vs explicit opt-out — because the config resolves before
|
||||||
|
* the runtime probe fires). Not intended to be set by callers.
|
||||||
|
*/
|
||||||
|
forceScreenshotExplicitlyOptedOut?: boolean;
|
||||||
/**
|
/**
|
||||||
* Low-memory render profile. When `true`, the orchestrator collapses the
|
* Low-memory render profile. When `true`, the orchestrator collapses the
|
||||||
* pipeline to its cheapest shape on memory-constrained hosts: it skips the
|
* pipeline to its cheapest shape on memory-constrained hosts: it skips the
|
||||||
@@ -581,6 +594,12 @@ export function resolveConfig(overrides?: Partial<EngineConfig>): EngineConfig {
|
|||||||
// on-software debugging remains possible.
|
// on-software debugging remains possible.
|
||||||
const explicitForceScreenshotOptOut =
|
const explicitForceScreenshotOptOut =
|
||||||
env("PRODUCER_FORCE_SCREENSHOT") === "false" || overrides?.forceScreenshot === false;
|
env("PRODUCER_FORCE_SCREENSHOT") === "false" || overrides?.forceScreenshot === false;
|
||||||
|
// Persist provenance so the concrete-resolved-GPU helper can honor the
|
||||||
|
// programmatic opt-out too — at that point `forceScreenshot === false` is
|
||||||
|
// otherwise ambiguous between default and explicit opt-out.
|
||||||
|
if (explicitForceScreenshotOptOut) {
|
||||||
|
merged.forceScreenshotExplicitlyOptedOut = true;
|
||||||
|
}
|
||||||
if (
|
if (
|
||||||
merged.browserGpuMode === "software" &&
|
merged.browserGpuMode === "software" &&
|
||||||
!merged.forceScreenshot &&
|
!merged.forceScreenshot &&
|
||||||
@@ -615,23 +634,30 @@ export function resolveConfig(overrides?: Partial<EngineConfig>): EngineConfig {
|
|||||||
* Runtime-resolved companion to the software-GPU screenshot clamp in
|
* Runtime-resolved companion to the software-GPU screenshot clamp in
|
||||||
* `resolveConfig`. Returns `true` iff callers should treat this render as
|
* `resolveConfig`. Returns `true` iff callers should treat this render as
|
||||||
* `forceScreenshot=true` even though the config's stored `forceScreenshot`
|
* `forceScreenshot=true` even though the config's stored `forceScreenshot`
|
||||||
* is `false`. Fires when the concrete resolved GPU is software AND the
|
* is `false`. Fires when the concrete resolved GPU is software AND neither
|
||||||
* env-level opt-out (`PRODUCER_FORCE_SCREENSHOT=false`) is NOT set.
|
* the env opt-out (`PRODUCER_FORCE_SCREENSHOT=false`) nor the programmatic
|
||||||
|
* opt-out (`overrides.forceScreenshot === false`, carried via
|
||||||
|
* `cfg.forceScreenshotExplicitlyOptedOut`) is set.
|
||||||
*
|
*
|
||||||
* `resolveConfig`'s clamp only sees `browserGpuMode` as a string, so
|
* `resolveConfig`'s clamp only sees `browserGpuMode` as a string, so
|
||||||
* `"auto"` that runtime-probes to software slips through. This helper
|
* `"auto"` that runtime-probes to software slips through. This helper
|
||||||
* closes that gap at the concrete-resolution points (`frameCapture` and
|
* closes that gap at the concrete-resolution points (`frameCapture` and
|
||||||
* `renderOrchestrator`). Same invariant, same env opt-out, one predicate.
|
* `renderOrchestrator`). Same invariant, same escape hatches, one predicate.
|
||||||
*
|
*
|
||||||
* Callers should skip when the invariant is already satisfied
|
* Callers should skip when the invariant is already satisfied
|
||||||
* (`currentForceScreenshot === true`) to avoid redundant work.
|
* (`currentForceScreenshot === true`) to avoid redundant work. Pass
|
||||||
|
* `cfg.forceScreenshotExplicitlyOptedOut` via `opts.programmaticOptOut` so
|
||||||
|
* the `browserGpuMode:"auto"` → software probe path honors the same
|
||||||
|
* programmatic escape hatch as literal `browserGpuMode:"software"`.
|
||||||
*/
|
*/
|
||||||
export function shouldClampToScreenshotForConcreteGpu(
|
export function shouldClampToScreenshotForConcreteGpu(
|
||||||
resolvedGpuMode: "software" | "hardware",
|
resolvedGpuMode: "software" | "hardware",
|
||||||
currentForceScreenshot: boolean,
|
currentForceScreenshot: boolean,
|
||||||
env: NodeJS.ProcessEnv = process.env,
|
env: NodeJS.ProcessEnv = process.env,
|
||||||
|
opts: { programmaticOptOut?: boolean } = {},
|
||||||
): boolean {
|
): boolean {
|
||||||
if (currentForceScreenshot) return false;
|
if (currentForceScreenshot) return false;
|
||||||
if (resolvedGpuMode !== "software") return false;
|
if (resolvedGpuMode !== "software") return false;
|
||||||
|
if (opts.programmaticOptOut) return false;
|
||||||
return env["PRODUCER_FORCE_SCREENSHOT"] !== "false";
|
return env["PRODUCER_FORCE_SCREENSHOT"] !== "false";
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -825,9 +825,16 @@ export async function createCaptureSession(
|
|||||||
// point too — `resolveConfig` can only see the pre-resolve `browserGpuMode`
|
// point too — `resolveConfig` can only see the pre-resolve `browserGpuMode`
|
||||||
// string, so `"auto"` that probes to software would otherwise slip through
|
// string, so `"auto"` that probes to software would otherwise slip through
|
||||||
// and launch BeginFrame + SwiftShader (the exact combination the invariant
|
// and launch BeginFrame + SwiftShader (the exact combination the invariant
|
||||||
// is meant to prevent). Env-level opt-out preserved via the shared helper.
|
// is meant to prevent). Both env and programmatic opt-outs preserved via
|
||||||
|
// the shared helper (the programmatic one carried on the config as
|
||||||
|
// `forceScreenshotExplicitlyOptedOut`, since at this point the boolean
|
||||||
|
// `forceScreenshot === false` is otherwise ambiguous between default and
|
||||||
|
// explicit opt-out).
|
||||||
const effectiveForceScreenshot =
|
const effectiveForceScreenshot =
|
||||||
forceScreenshot || shouldClampToScreenshotForConcreteGpu(resolvedGpuMode, forceScreenshot);
|
forceScreenshot ||
|
||||||
|
shouldClampToScreenshotForConcreteGpu(resolvedGpuMode, forceScreenshot, process.env, {
|
||||||
|
programmaticOptOut: config?.forceScreenshotExplicitlyOptedOut ?? false,
|
||||||
|
});
|
||||||
const preMode: CaptureMode =
|
const preMode: CaptureMode =
|
||||||
headlessShell &&
|
headlessShell &&
|
||||||
isLinux &&
|
isLinux &&
|
||||||
|
|||||||
@@ -1914,13 +1914,16 @@ export async function executeRenderJob(
|
|||||||
// that probe to software: `resolveConfig` couldn't see this at config
|
// that probe to software: `resolveConfig` couldn't see this at config
|
||||||
// time, so `captureObservability.forceScreenshot` was still false,
|
// time, so `captureObservability.forceScreenshot` was still false,
|
||||||
// misreporting `captureMode: "beginframe"` for a session that will
|
// misreporting `captureMode: "beginframe"` for a session that will
|
||||||
// actually take the screenshot path. Env-level opt-out preserved via
|
// actually take the screenshot path. Both env and programmatic opt-outs
|
||||||
// the shared helper.
|
// preserved via the shared helper (the programmatic one carried on the
|
||||||
|
// config as `forceScreenshotExplicitlyOptedOut`).
|
||||||
const observabilityForceScreenshot =
|
const observabilityForceScreenshot =
|
||||||
captureObservability.forceScreenshot ||
|
captureObservability.forceScreenshot ||
|
||||||
shouldClampToScreenshotForConcreteGpu(
|
shouldClampToScreenshotForConcreteGpu(
|
||||||
resolvedBrowserGpuMode,
|
resolvedBrowserGpuMode,
|
||||||
captureObservability.forceScreenshot,
|
captureObservability.forceScreenshot,
|
||||||
|
process.env,
|
||||||
|
{ programmaticOptOut: cfg.forceScreenshotExplicitlyOptedOut ?? false },
|
||||||
);
|
);
|
||||||
updateCaptureObservability({
|
updateCaptureObservability({
|
||||||
browserGpuMode: resolvedBrowserGpuMode,
|
browserGpuMode: resolvedBrowserGpuMode,
|
||||||
|
|||||||
Reference in New Issue
Block a user