mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-01 19:42:03 +00:00
Closes #3370 ## What When `hyperframeRuntimeLoader` could not locate `hyperframe.manifest.json`, the loader reported a single fallback path that was never searched for (`/usr/local/lib/core/dist/hyperframe.manifest.json`). Inside a Docker render the user is then told to look at the wrong directory; the file that was actually missing (`/usr/local/lib/node_modules/hyperframes/dist/hyperframe.manifest.json`) was nowhere in the message. ## Why `resolveHyperframeManifestPath()` built a 5-element `candidates` array, walked it with `existsSync`, and on total miss returned the last candidate. The error then quoted that candidate verbatim. The reporter even shows the exact reproducing command from a published image. A second issue rode the same failure path: `packages/cli/src/commands/render.ts:902` keeps attaching the hint `"Try --docker for containerized rendering"` to users who are *already inside* the container. The container sets `ENV CONTAINER=true` and nothing reads it. A third small thing came along: `CWD_RELATIVE_MANIFEST_PATHS[0]` was a byte-identical duplicate of `SIBLING_MANIFEST_PATH` — same path, two names. ## How 1. Hoist the candidate list to a single `MANIFEST_CANDIDATES` owner in `hyperframeRuntimeLoader.ts` and share it between the resolver and the error reporter. De-duplicate while doing it. 2. Add `triedManifestPaths()` as a tiny export so callers (and tests) can see what was actually searched. 3. Replace the source-text regex test that asserted on string positions inside `const candidates = [...]` with a behaviour test that points `PRODUCER_HYPERFRAME_MANIFEST_PATH` at a missing file and asserts the thrown error names it. Also exercise the no-override branch to confirm the sibling path is the first entry. 4. In `render.ts`, check `process.env.CONTAINER === "true"` before attaching the `--docker` hint. The chrome-launch and macos-old-chrome remediation branches already short-circuit before the hint, so an empty string is a safe value when the user is in the container. ## Test plan - [x] `bunx vitest run src/services/hyperframeRuntimeLoader.test.ts` — 7/7 pass (`hyperframeRuntimeLoader error path (#3370)` describe covers the missing-manifest message and the tried-paths export). - [x] `bunx tsc --noEmit` in `packages/producer` and `packages/cli` — clean. - [x] `bunx oxfmt --check` and `bunx oxlint` on the touched files — clean. - [x] `bunx fallow audit --base origin/main` — no new findings on the touched files. - [x] Targeted producer unit lane: `node scripts/run-test-lane.mjs unit` — same 7 pre-existing failures as `origin/main` before the change (htmlCompiler.parity, audioPadTrim.integration); no regressions introduced. Files touched: - `packages/producer/src/services/hyperframeRuntimeLoader.ts` - `packages/producer/src/services/hyperframeRuntimeLoader.test.ts` - `packages/cli/src/commands/render.ts`
This commit is contained in:
@@ -902,12 +902,15 @@ export async function renderLocal(
|
||||
await producer.executeRenderJob(job, projectDir, outputPath, onProgress);
|
||||
} catch (error: unknown) {
|
||||
maybeConsumeDeParallelRouterTrial(deParallelRouterActive, job, options.quiet);
|
||||
// The render container sets `ENV CONTAINER=true`; suggesting `--docker`
|
||||
// from inside it is a misdirection (heygen-com/hyperframes#3370).
|
||||
const inContainer = process.env.CONTAINER === "true";
|
||||
handleRenderError(
|
||||
error,
|
||||
options,
|
||||
startTime,
|
||||
false,
|
||||
"Try --docker for containerized rendering",
|
||||
inContainer ? "" : "Try --docker for containerized rendering",
|
||||
job.failedStage,
|
||||
job,
|
||||
);
|
||||
|
||||
@@ -36,40 +36,71 @@ describe("resolveHyperframeManifestPath", () => {
|
||||
expect(SIBLING_PATH).toContain("producer/src/services/hyperframe.manifest.json");
|
||||
});
|
||||
|
||||
it("includes sibling path as first candidate in resolution order", async () => {
|
||||
// Import the actual source and verify the sibling path is found when it
|
||||
// exists. In the monorepo, the monorepo-relative path also exists, so we
|
||||
// verify the sibling would win by checking its position in candidates.
|
||||
//
|
||||
// We can't easily mock existsSync in ESM, but we CAN verify the
|
||||
// structural invariant: the function checks SIBLING first by reading the
|
||||
// source and confirming the candidate array order.
|
||||
const { readFileSync } = await import("node:fs");
|
||||
const source = readFileSync(resolve(THIS_DIR, "hyperframeRuntimeLoader.ts"), "utf8");
|
||||
|
||||
// The candidates array must list SIBLING_MANIFEST_PATH before the others
|
||||
const candidatesMatch = source.match(/const candidates = \[([\s\S]*?)\];/);
|
||||
expect(candidatesMatch).not.toBeNull();
|
||||
const candidatesBody = candidatesMatch![1];
|
||||
|
||||
const siblingIdx = candidatesBody.indexOf("SIBLING_MANIFEST_PATH");
|
||||
const cwdIdx = candidatesBody.indexOf("CWD_RELATIVE_MANIFEST_PATHS");
|
||||
const moduleIdx = candidatesBody.indexOf("MODULE_RELATIVE_MANIFEST_PATH");
|
||||
|
||||
expect(siblingIdx).toBeGreaterThan(-1);
|
||||
expect(siblingIdx).toBeLessThan(cwdIdx);
|
||||
expect(cwdIdx).toBeLessThan(moduleIdx);
|
||||
it("prefers sibling path when it exists, otherwise picks the first existing candidate", async () => {
|
||||
// Behaviour-level replacement for the old source-text test that
|
||||
// asserted on string positions inside `const candidates = [...]`. We
|
||||
// prove the behavioural invariant instead: the resolver returns the
|
||||
// first candidate that actually exists on disk.
|
||||
const { resolveHyperframeManifestPath } = await import("./hyperframeRuntimeLoader.js");
|
||||
const resolved = resolveHyperframeManifestPath();
|
||||
expect(existsSync(resolved)).toBe(true);
|
||||
// The sibling would win when present. In dev, the monorepo-relative
|
||||
// core/dist is the real fallback; either way the path must exist.
|
||||
if (existsSync(SIBLING_PATH)) {
|
||||
expect(resolved).toBe(SIBLING_PATH);
|
||||
}
|
||||
});
|
||||
|
||||
it("finds manifest via monorepo-relative path in dev (integration check)", async () => {
|
||||
// In the monorepo, the core/dist manifest should exist from the build.
|
||||
// This acts as a smoke test that the resolution works in the dev env.
|
||||
it("falls back to MONOREPO_PATH when present in dev (smoke test)", async () => {
|
||||
if (!existsSync(MONOREPO_PATH)) {
|
||||
// Skip if core hasn't been built — this is expected in CI before build
|
||||
return;
|
||||
}
|
||||
const { resolveHyperframeManifestPath } = await import("./hyperframeRuntimeLoader.js");
|
||||
const result = resolveHyperframeManifestPath();
|
||||
expect(existsSync(result)).toBe(true);
|
||||
expect(resolveHyperframeManifestPath()).toBe(MONOREPO_PATH);
|
||||
});
|
||||
});
|
||||
|
||||
describe("hyperframeRuntimeLoader error path (#3370)", () => {
|
||||
const originalEnv = process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH;
|
||||
|
||||
beforeEach(() => {
|
||||
delete process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalEnv !== undefined) {
|
||||
process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH = originalEnv;
|
||||
} else {
|
||||
delete process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH;
|
||||
}
|
||||
});
|
||||
|
||||
it("names the env-override path when PRODUCER_HYPERFRAME_MANIFEST_PATH is set and missing", async () => {
|
||||
// Force the env-override branch with a missing file. The thrown error
|
||||
// must name the override, not any fallback candidate.
|
||||
process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH = "/nonexistent/override/manifest.json";
|
||||
const { resolveVerifiedHyperframeRuntime } = await import("./hyperframeRuntimeLoader.js");
|
||||
expect(() => resolveVerifiedHyperframeRuntime()).toThrow(
|
||||
/nonexistent\/override\/manifest\.json/,
|
||||
);
|
||||
});
|
||||
|
||||
it("triedManifestPaths returns only the override when PRODUCER_HYPERFRAME_MANIFEST_PATH is set", async () => {
|
||||
process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH = "/another/missing/override.json";
|
||||
const { triedManifestPaths } = await import("./hyperframeRuntimeLoader.js");
|
||||
expect(triedManifestPaths()).toEqual(["/another/missing/override.json"]);
|
||||
});
|
||||
|
||||
it("triedManifestPaths lists every candidate when no override is set", async () => {
|
||||
delete process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH;
|
||||
const { triedManifestPaths } = await import("./hyperframeRuntimeLoader.js");
|
||||
const tried = triedManifestPaths();
|
||||
expect(tried.length).toBeGreaterThanOrEqual(4);
|
||||
// The first candidate must be the sibling path so the user sees it
|
||||
// first in the error message (heygen-com/hyperframes#3370).
|
||||
expect(tried[0]).toBe(
|
||||
resolve(dirname(fileURLToPath(import.meta.url)), "hyperframe.manifest.json"),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,13 +9,17 @@ const MODULE_RELATIVE_MANIFEST_PATH = resolve(
|
||||
PRODUCER_DIR,
|
||||
"../../../core/dist/hyperframe.manifest.json",
|
||||
);
|
||||
const CWD_RELATIVE_MANIFEST_PATHS = [
|
||||
// When bundled to a single file (dist/public-server.js), the manifest
|
||||
// is copied as a sibling by build.mjs
|
||||
resolve(PRODUCER_DIR, "hyperframe.manifest.json"),
|
||||
// Order matters: a bundled CLI ships the manifest as a sibling of the
|
||||
// packaged module; dev runs reach it via monorepo-relative paths. Listed
|
||||
// once here so the resolver and the missing-manifest error share the same
|
||||
// owner — printing only the fallback candidate misdirects the user
|
||||
// (heygen-com/hyperframes#3370).
|
||||
const MANIFEST_CANDIDATES: readonly string[] = [
|
||||
SIBLING_MANIFEST_PATH,
|
||||
resolve(process.cwd(), "packages/core/dist/hyperframe.manifest.json"),
|
||||
resolve(process.cwd(), "../core/dist/hyperframe.manifest.json"),
|
||||
resolve(process.cwd(), "core/dist/hyperframe.manifest.json"),
|
||||
MODULE_RELATIVE_MANIFEST_PATH,
|
||||
];
|
||||
|
||||
type HyperframeRuntimeManifest = {
|
||||
@@ -34,20 +38,21 @@ export type ResolvedHyperframeRuntime = {
|
||||
};
|
||||
|
||||
export function resolveHyperframeManifestPath(): string {
|
||||
if (process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH) {
|
||||
return process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH;
|
||||
const envOverride = process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH;
|
||||
if (envOverride) {
|
||||
return envOverride;
|
||||
}
|
||||
const candidates = [
|
||||
SIBLING_MANIFEST_PATH,
|
||||
...CWD_RELATIVE_MANIFEST_PATHS,
|
||||
MODULE_RELATIVE_MANIFEST_PATH,
|
||||
];
|
||||
for (const candidate of candidates) {
|
||||
if (existsSync(candidate)) {
|
||||
return candidate;
|
||||
}
|
||||
}
|
||||
return MODULE_RELATIVE_MANIFEST_PATH;
|
||||
const found = MANIFEST_CANDIDATES.find((candidate) => existsSync(candidate));
|
||||
// Fall back to the last candidate only when nothing exists. The caller will
|
||||
// read its iife/artifact and throw; returning a stable but unreachable path
|
||||
// keeps the existing API contract.
|
||||
return found ?? MODULE_RELATIVE_MANIFEST_PATH;
|
||||
}
|
||||
|
||||
export function triedManifestPaths(): readonly string[] {
|
||||
return process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH
|
||||
? [process.env.PRODUCER_HYPERFRAME_MANIFEST_PATH]
|
||||
: MANIFEST_CANDIDATES;
|
||||
}
|
||||
|
||||
export function getVerifiedHyperframeRuntimeSource(): string {
|
||||
@@ -57,8 +62,9 @@ export function getVerifiedHyperframeRuntimeSource(): string {
|
||||
export function resolveVerifiedHyperframeRuntime(): ResolvedHyperframeRuntime {
|
||||
const manifestPath = resolveHyperframeManifestPath();
|
||||
if (!existsSync(manifestPath)) {
|
||||
const tried = triedManifestPaths().join(", ");
|
||||
throw new Error(
|
||||
`[HyperframeRuntimeLoader] Missing manifest at ${manifestPath}. Build core runtime artifacts before rendering.`,
|
||||
`[HyperframeRuntimeLoader] Missing manifest. Tried: ${tried}. Searched from cwd=${process.cwd()}. Build core runtime artifacts before rendering.`,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user