mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 12:54:29 +00:00
fix(cli): keep doctor resilient to a corrupt browser cache (#1822)
* fix(cli): keep doctor resilient to a corrupt browser cache A partial or corrupt browser cache (a stub file where a version directory is expected, a missing executable, or malformed metadata) makes getInstalledBrowsers throw ENOTDIR. That throw propagated up through findBrowser -> checkChrome -> runEnvironmentChecks, and since doctor.run calls runEnvironmentChecks before any try/catch or the --json output, the command crashed with exit 1. doctor --json is documented to exit 0 even when checks fail, so it must report a corrupt cache as "Chrome not found", not crash on it. - checkChrome now catches any error from findBrowser and converts it to the existing ok:false "Chrome not found" outcome with the browser ensure hint, so runEnvironmentChecks never throws for a missing or corrupt browser. - findFromCache treats a throwing getInstalledBrowsers as "no cached browser", letting resolution fall through to system/download instead of crashing every caller (render included), not just doctor. A healthy browser still reports ok:true. Adds a preflight test asserting an ok:false Chrome outcome when discovery throws, instead of propagating. * fix(cli): warn on corrupt browser cache fallback
This commit is contained in:
@@ -73,13 +73,16 @@ function installFsMocks({ existing, dirs }: FsMockOptions) {
|
||||
function installPuppeteerBrowsersMock(
|
||||
opts: {
|
||||
installedInHfCache?: Array<{ browser: string; executablePath: string }>;
|
||||
installedInHfCacheError?: Error;
|
||||
installResult?: { executablePath: string };
|
||||
} = {},
|
||||
) {
|
||||
vi.doMock("@puppeteer/browsers", () => ({
|
||||
Browser: { CHROMEHEADLESSSHELL: "chrome-headless-shell" },
|
||||
detectBrowserPlatform: () => "linux",
|
||||
getInstalledBrowsers: vi.fn().mockResolvedValue(opts.installedInHfCache ?? []),
|
||||
getInstalledBrowsers: opts.installedInHfCacheError
|
||||
? vi.fn().mockRejectedValue(opts.installedInHfCacheError)
|
||||
: vi.fn().mockResolvedValue(opts.installedInHfCache ?? []),
|
||||
install: vi.fn().mockResolvedValue(opts.installResult ?? { executablePath: HF_BINARY }),
|
||||
}));
|
||||
}
|
||||
@@ -144,6 +147,24 @@ describe("findBrowser — cache resolution", () => {
|
||||
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("Cached binary missing"));
|
||||
});
|
||||
|
||||
it("warns and falls through when the hyperframes cache cannot be read", async () => {
|
||||
installFsMocks({ existing: new Set([HF_CACHE, SYSTEM_CHROME]) });
|
||||
installPuppeteerBrowsersMock({
|
||||
installedInHfCacheError: Object.assign(new Error("ENOTDIR: not a directory"), {
|
||||
code: "ENOTDIR",
|
||||
}),
|
||||
});
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
|
||||
const { findBrowser, _resetSystemFallbackWarnForTests } = await import("./manager.js");
|
||||
_resetSystemFallbackWarnForTests();
|
||||
const result = await findBrowser();
|
||||
|
||||
expect(result).toEqual({ executablePath: SYSTEM_CHROME, source: "system" });
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("Browser cache read failed (ENOTDIR)");
|
||||
expect(warnSpy.mock.calls[0]?.[0]).toContain("Falling back to system Chrome");
|
||||
});
|
||||
|
||||
it("falls back to the puppeteer-managed cache when hyperframes cache is empty", async () => {
|
||||
// Empty hyperframes cache, populated puppeteer cache — the regression
|
||||
// scenario from the hf#677 spike.
|
||||
|
||||
@@ -105,7 +105,21 @@ async function findFromCache(): Promise<CacheLookupResult> {
|
||||
// no puppeteer-cache binary exists.
|
||||
if (existsSync(CACHE_DIR)) {
|
||||
const { Browser, getInstalledBrowsers } = await loadPuppeteerBrowsers();
|
||||
const installed = await getInstalledBrowsers({ cacheDir: CACHE_DIR });
|
||||
// A corrupt cache (stub file where a browser dir is expected, malformed
|
||||
// metadata) makes getInstalledBrowsers throw. Treat that as "no cached
|
||||
// browser" so resolution falls through to system/download instead of
|
||||
// crashing every caller.
|
||||
let installed: Awaited<ReturnType<typeof getInstalledBrowsers>>;
|
||||
try {
|
||||
installed = await getInstalledBrowsers({ cacheDir: CACHE_DIR });
|
||||
} catch (err) {
|
||||
const code = (err as NodeJS.ErrnoException | undefined)?.code;
|
||||
const suffix = code ? ` (${code})` : "";
|
||||
console.warn(
|
||||
`[hyperframes] Browser cache read failed${suffix}: ${normalizeErrorMessage(err)}. Falling back to system Chrome or a fresh download.`,
|
||||
);
|
||||
installed = [];
|
||||
}
|
||||
const match = installed.find((b) => b.browser === Browser.CHROMEHEADLESSSHELL);
|
||||
if (match && existsSync(match.executablePath)) {
|
||||
return { result: { executablePath: match.executablePath, source: "cache" } };
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// fallow-ignore-file code-duplication
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { parseToolVersion, runEnvironmentChecks } from "./preflight.js";
|
||||
import * as manager from "./manager.js";
|
||||
|
||||
describe("runEnvironmentChecks", () => {
|
||||
const originalFfmpegPath = process.env.HYPERFRAMES_FFMPEG_PATH;
|
||||
@@ -67,6 +68,27 @@ describe("runEnvironmentChecks", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("reports Chrome as not found (no throw) when browser discovery throws on a corrupt cache", async () => {
|
||||
const spy = vi.spyOn(manager, "findBrowser").mockRejectedValue(
|
||||
Object.assign(new Error("ENOTDIR: not a directory, scandir 'chrome-headless-shell'"), {
|
||||
code: "ENOTDIR",
|
||||
}),
|
||||
);
|
||||
|
||||
try {
|
||||
const result = await runEnvironmentChecks({ includeBrowser: true });
|
||||
|
||||
expect(result.outcomes.find((outcome) => outcome.name === "Chrome")).toMatchObject({
|
||||
ok: false,
|
||||
title: "Chrome not found",
|
||||
hint: "Run: npx hyperframes browser ensure",
|
||||
});
|
||||
expect(result.browser).toBeUndefined();
|
||||
} finally {
|
||||
spy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("reports an explicit missing browser path before render starts", async () => {
|
||||
const result = await runEnvironmentChecks({
|
||||
includeBrowser: true,
|
||||
|
||||
@@ -139,7 +139,17 @@ async function checkChrome(browserPath?: string): Promise<EnvironmentCheckOutcom
|
||||
};
|
||||
}
|
||||
|
||||
const info = await findBrowser();
|
||||
// A corrupt/partial browser cache (stub files where a version dir is
|
||||
// expected, missing executable, malformed metadata) makes findBrowser throw.
|
||||
// That is the exact condition this check exists to report, so treat any
|
||||
// failure as "Chrome not found" rather than letting it crash the caller
|
||||
// (notably `doctor`, which is documented to exit 0 even when checks fail).
|
||||
let info: Awaited<ReturnType<typeof findBrowser>>;
|
||||
try {
|
||||
info = await findBrowser();
|
||||
} catch {
|
||||
info = undefined;
|
||||
}
|
||||
if (info) {
|
||||
return {
|
||||
name: "Chrome",
|
||||
|
||||
Reference in New Issue
Block a user