skills/heygen-com__hyperframes
1
0
Fork 0
mirror of https://github.com/heygen-com/hyperframes.git synced 2026-09-14 18:01:20 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(cli): keep slow browser installs from losing their lock [P0] (#2116)

Browse Source 
* fix(cli): keep slow browser installs from losing their lock

* refactor(cli): simplify browser install lock policy

* fix(cli): keep browser lock heartbeat failures non-fatal
This commit is contained in:
Miguel Ángel
2026-07-09 21:30:30 -04:00
committed by GitHub
parent 3aa1cf0d83
commit 7bd8c0942e
2 changed files with 119 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/cli/src/browser/manager.test.ts
+78 -18
View File
@@ -50,14 +50,21 @@ const HF_BINARY = join(
"chrome-headless-shell",
);
const SYSTEM_CHROME = "/usr/bin/google-chrome";
const TEST_LOCK_TIMINGS = {
staleMs: 50,
pollMs: 5,
heartbeatMs: 10,
waitNoticeMs: 1_000,
};
interface FsMockOptions {
existing: ReadonlySet<string>;
/** map of dir path -> entries returned by readdirSync */
dirs?: Record<string, string[]>;
touchError?: Error;
}
function installFsMocks({ existing, dirs }: FsMockOptions) {
function installFsMocks({ existing, dirs, touchError }: FsMockOptions) {
// Mutable, and returned, so tests can pre-seed a "lock already held" path or
// assert the lock dir doesn't leak after ensureBrowser resolves.
const paths = new Set(existing);
@@ -97,6 +104,15 @@ function installFsMocks({ existing, dirs }: FsMockOptions) {
}
return { mtimeMs: mtimes.get(p) ?? 0 };
},
utimesSync: (p: string, _atime: Date, mtime: Date) => {
if (touchError) throw touchError;
if (!paths.has(p)) {
const err = new Error(`ENOENT: no such file or directory, utimes '${p}'`);
(err as NodeJS.ErrnoException).code = "ENOENT";
throw err;
}
mtimes.set(p, mtime.getTime());
},
}));
vi.doMock("node:os", () => ({
homedir: () => FAKE_HOME,
@@ -322,17 +338,10 @@ describe("findBrowser — cache resolution", () => {
});
it("withInstallLock reclaims a lock held past the timeout instead of hanging forever", async () => {
// A crashed/killed process could leave the lock directory behind
// permanently. Reclaiming after a timeout (rather than hanging or
// refusing forever) is the behavior that makes the lock safe to add at
// all — otherwise one bad exit wedges every future render. Exercises
// withInstallLock directly with tiny real timeouts (it takes an
// injectable timeoutMs/pollMs for exactly this) rather than mocking
// Date.now()/setTimeout through the full ensureBrowser call graph.
const paths = installFsMocks({ existing: new Set([CACHE_ROOT, HF_LOCK]) });
const { withInstallLock } = await import("./manager.js");
const result = await withInstallLock(async () => "done", 10, 5);
const result = await withInstallLock(async () => "done", TEST_LOCK_TIMINGS);
expect(result).toBe("done");
expect(paths.has(HF_LOCK)).toBe(false);
@@ -346,21 +355,72 @@ describe("findBrowser — cache resolution", () => {
const paths = installFsMocks({ existing: new Set([CACHE_ROOT, HF_LOCK]) });
const { withInstallLock } = await import("./manager.js");
const first = withInstallLock(
async () => {
await new Promise((resolve) => setTimeout(resolve, 30));
return "first";
},
10,
5,
);
const second = withInstallLock(async () => "second", 10, 5);
const reclaimOnlyTimings = { ...TEST_LOCK_TIMINGS, heartbeatMs: 1_000 };
const first = withInstallLock(async () => {
await new Promise((resolve) => setTimeout(resolve, 30));
return "first";
}, reclaimOnlyTimings);
const second = withInstallLock(async () => "second", reclaimOnlyTimings);
await expect(Promise.all([first, second])).resolves.toEqual(["first", "second"]);
expect(paths.has(HF_LOCK)).toBe(false);
expect(paths.has(HF_RECLAIM_LOCK)).toBe(false);
});
it("withInstallLock does not let a second caller run concurrently with a slow-but-alive holder", async () => {
const paths = installFsMocks({ existing: new Set([CACHE_ROOT]) });
const { withInstallLock } = await import("./manager.js");
let concurrent = 0;
let maxConcurrent = 0;
const trackConcurrency = async (label: string, durationMs: number) => {
concurrent += 1;
maxConcurrent = Math.max(maxConcurrent, concurrent);
await new Promise((resolve) => setTimeout(resolve, durationMs));
concurrent -= 1;
return label;
};
const first = withInstallLock(() => trackConcurrency("first", 120), TEST_LOCK_TIMINGS);
await new Promise((resolve) => setTimeout(resolve, 5)); // let `first` acquire the lock
const second = withInstallLock(() => trackConcurrency("second", 10), TEST_LOCK_TIMINGS);
await expect(Promise.all([first, second])).resolves.toEqual(["first", "second"]);
expect(maxConcurrent).toBe(1);
expect(paths.has(HF_LOCK)).toBe(false);
});
it("withInstallLock reports progress while waiting instead of staying silent", async () => {
const paths = installFsMocks({ existing: new Set([CACHE_ROOT, HF_LOCK]) });
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
const { withInstallLock } = await import("./manager.js");
await withInstallLock(async () => "done", { ...TEST_LOCK_TIMINGS, waitNoticeMs: 20 });
expect(paths.has(HF_LOCK)).toBe(false);
expect(
warnSpy.mock.calls.some(([msg]) =>
String(msg).includes("Waiting for another hyperframes process"),
),
).toBe(true);
});
it("keeps the holder running when a heartbeat cannot touch the lock", async () => {
installFsMocks({
existing: new Set([CACHE_ROOT]),
touchError: Object.assign(new Error("EACCES"), { code: "EACCES" }),
});
const { withInstallLock } = await import("./manager.js");
await expect(
withInstallLock(async () => {
await new Promise((resolve) => setTimeout(resolve, 25));
return "done";
}, TEST_LOCK_TIMINGS),
).resolves.toBe("done");
});
it("warns and falls through when the hyperframes cache cannot be read", async () => {
installFsMocks({ existing: new Set([HF_CACHE, SYSTEM_CHROME]) });
installPuppeteerBrowsersMock({
packages/cli/src/browser/manager.ts
+41 -13
View File
@@ -1,6 +1,6 @@
// fallow-ignore-file code-duplication
import { execSync, spawnSync } from "node:child_process";
import { existsSync, mkdirSync, readdirSync, rmSync, statSync } from "node:fs";
import { existsSync, mkdirSync, readdirSync, rmSync, statSync, utimesSync } from "node:fs";
import { basename } from "node:path";
import { homedir } from "node:os";
import { join } from "node:path";
@@ -42,8 +42,19 @@ const PUPPETEER_CACHE_DIR = join(homedir(), ".cache", "puppeteer", "chrome-headl
// doubles as a zero-dependency cross-process mutex — no lockfile library needed.
const INSTALL_LOCK_DIR = join(CACHE_ROOT_DIR, ".chrome.install.lock");
const INSTALL_RECLAIM_LOCK_DIR = join(CACHE_ROOT_DIR, ".chrome.install.reclaim.lock");
const INSTALL_LOCK_TIMEOUT_MS = 120_000; // generous: a real download+extract can take a while
const INSTALL_LOCK_POLL_MS = 200;
const INSTALL_LOCK_TIMINGS = {
staleMs: 120_000,
pollMs: 200,
heartbeatMs: 15_000,
waitNoticeMs: 10_000,
};
interface InstallLockTimings {
staleMs: number;
pollMs: number;
heartbeatMs: number;
waitNoticeMs: number;
}
function isErrno(err: unknown, code: string): boolean {
return (err as NodeJS.ErrnoException).code === code;
@@ -80,43 +91,60 @@ function reclaimStaleInstallLock(timeoutMs: number): void {
}
}
// timeoutMs/pollMs are parameters (not just the module constants) so tests can
// exercise the reclaim-on-timeout branch with real but tiny waits instead of
// mocking Date.now()/setTimeout through the full ensureBrowser call graph.
function touchInstallLock(): void {
try {
const now = new Date();
utimesSync(INSTALL_LOCK_DIR, now, now);
} catch {
// ponytail: heartbeat is best-effort; stale-lock reclaim remains the fallback.
}
}
export async function withInstallLock<T>(
fn: () => Promise<T>,
timeoutMs = INSTALL_LOCK_TIMEOUT_MS,
pollMs = INSTALL_LOCK_POLL_MS,
timings: InstallLockTimings = INSTALL_LOCK_TIMINGS,
): Promise<T> {
// recursive:false below needs the parent to already exist (unlike `mkdir -p`).
// Keep lock dirs outside CACHE_DIR so force-clearing the Chrome cache cannot
// delete another installer's in-flight lock.
if (!existsSync(CACHE_ROOT_DIR)) mkdirSync(CACHE_ROOT_DIR, { recursive: true });
let deadline = Date.now() + timeoutMs;
let deadline = Date.now() + timings.staleMs;
const waitStart = Date.now();
let lastNoticeMs = 0;
for (;;) {
if (existsSync(INSTALL_RECLAIM_LOCK_DIR)) {
await sleep(pollMs);
await sleep(timings.pollMs);
continue;
}
if (tryAcquireDirLock(INSTALL_LOCK_DIR)) {
rmSync(INSTALL_RECLAIM_LOCK_DIR, { recursive: true, force: true });
break;
}
const waitedMs = Date.now() - waitStart;
if (waitedMs - lastNoticeMs >= timings.waitNoticeMs) {
lastNoticeMs = waitedMs;
console.warn(
`[browser] Waiting for another hyperframes process to finish installing chrome-headless-shell (${Math.round(waitedMs / 1000)}s elapsed)...`,
);
}
if (Date.now() > deadline) {
// The reclaim gate matters when multiple waiters cross the timeout at
// once: without it, waiter A can delete the stale lock and acquire a
// fresh one, then waiter B (whose old deadline also expired) can delete
// A's fresh lock. The gate serializes reclaimers, and the mtime re-check
// after the gate prevents deleting a fresh lock another waiter just won.
reclaimStaleInstallLock(timeoutMs);
deadline = Date.now() + timeoutMs;
reclaimStaleInstallLock(timings.staleMs);
deadline = Date.now() + timings.staleMs;
continue;
}
await sleep(pollMs);
await sleep(timings.pollMs);
}
const heartbeat = setInterval(touchInstallLock, timings.heartbeatMs);
if (typeof heartbeat.unref === "function") heartbeat.unref();
try {
return await fn();
} finally {
clearInterval(heartbeat);
rmSync(INSTALL_LOCK_DIR, { recursive: true, force: true });
}
}