mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(cli): lint sets process.exitCode instead of process.exit() to flush stdout
`hyperframes lint --json` wrote the JSON payload with console.log() and then immediately called process.exit(). process.exit() terminates the process before Node flushes an asynchronously-buffered stdout, which is what a non-TTY (piped) stdout is — so `hyperframes lint --json | tee`, `> out.json`, or any agent/CI capture silently lost the entire payload on Windows (reported on 0.7.31 non-TTY). The same console.log-then-exit pattern was on all four exit sites (both --json branches and the human-readable + thrown-error paths), so any of them could truncate. Fix: set process.exitCode and return, letting run() unwind so Node drains stdout before exiting with the code. This is exactly the pattern the other commands (publish/transcribe/upgrade/play/present) already use; lint was the outlier still calling process.exit() after writing. Test: new lint.test.ts drives the command's run() with mocked lintProject/resolveProject and a process.exit spy that throws if called. Covers the --json-with-errors, --json-clean, --json-thrown, and human-readable paths — each asserts process.exit is never called and the correct exitCode is set. Fails against the pre-fix code (the spy throws on the first process.exit).
This commit is contained in:
@@ -0,0 +1,90 @@
|
||||
// Regression: `lint --json` used process.exit() right after console.log(JSON).
|
||||
// process.exit() terminates before Node flushes an async (non-TTY / piped)
|
||||
// stdout, so piping `hyperframes lint --json` on Windows silently lost the whole
|
||||
// payload. The fix sets process.exitCode + returns so stdout drains first. These
|
||||
// tests lock that in: run() must NEVER call process.exit(), and must set the
|
||||
// right exitCode, for the success, error-findings, and thrown-error paths.
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const lintProjectMock = vi.fn();
|
||||
|
||||
vi.mock("../utils/project.js", () => ({
|
||||
resolveProject: (dir?: string) => ({ dir: dir ?? "/proj", name: "proj" }),
|
||||
}));
|
||||
vi.mock("../utils/lintProject.js", () => ({
|
||||
lintProject: (...args: unknown[]) => lintProjectMock(...args),
|
||||
}));
|
||||
// withMeta just annotates the object; identity keeps the assertions simple.
|
||||
vi.mock("../utils/updateCheck.js", () => ({ withMeta: (o: unknown) => o }));
|
||||
|
||||
import lintCommand from "./lint.js";
|
||||
|
||||
function run(args: Record<string, unknown>): Promise<unknown> {
|
||||
// citty's CommandDef.run receives a context whose `args` we control.
|
||||
return (lintCommand.run as (ctx: { args: Record<string, unknown> }) => Promise<unknown>)({
|
||||
args,
|
||||
});
|
||||
}
|
||||
|
||||
describe("lint command exit handling", () => {
|
||||
const origExitCode = process.exitCode;
|
||||
|
||||
beforeEach(() => {
|
||||
process.exitCode = undefined;
|
||||
vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
// If run() ever calls process.exit, fail loudly (that's the bug).
|
||||
vi.spyOn(process, "exit").mockImplementation(((code?: number) => {
|
||||
throw new Error(`process.exit(${code}) called — truncates piped stdout`);
|
||||
}) as never);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
process.exitCode = origExitCode;
|
||||
});
|
||||
|
||||
it("--json with errors sets exitCode 1 and does NOT call process.exit", async () => {
|
||||
lintProjectMock.mockResolvedValue({
|
||||
results: [{ result: { findings: [{ severity: "error" }] }, file: "index.html" }],
|
||||
totalErrors: 1,
|
||||
totalWarnings: 0,
|
||||
totalInfos: 0,
|
||||
});
|
||||
await run({ json: true, verbose: false });
|
||||
expect(vi.mocked(process.exit)).not.toHaveBeenCalled();
|
||||
expect(process.exitCode).toBe(1);
|
||||
});
|
||||
|
||||
it("--json when clean sets exitCode 0 and does NOT call process.exit", async () => {
|
||||
lintProjectMock.mockResolvedValue({
|
||||
results: [{ result: { findings: [] }, file: "index.html" }],
|
||||
totalErrors: 0,
|
||||
totalWarnings: 0,
|
||||
totalInfos: 0,
|
||||
});
|
||||
await run({ json: true, verbose: false });
|
||||
expect(vi.mocked(process.exit)).not.toHaveBeenCalled();
|
||||
expect(process.exitCode).toBe(0);
|
||||
});
|
||||
|
||||
it("--json on a thrown error sets exitCode 1 and does NOT call process.exit", async () => {
|
||||
lintProjectMock.mockRejectedValue(new Error("boom"));
|
||||
await run({ json: true, verbose: false });
|
||||
expect(vi.mocked(process.exit)).not.toHaveBeenCalled();
|
||||
expect(process.exitCode).toBe(1);
|
||||
});
|
||||
|
||||
it("human-readable path with errors sets exitCode 1 without process.exit", async () => {
|
||||
lintProjectMock.mockResolvedValue({
|
||||
results: [{ result: { findings: [{ severity: "error" }] }, file: "index.html" }],
|
||||
totalErrors: 1,
|
||||
totalWarnings: 0,
|
||||
totalInfos: 0,
|
||||
});
|
||||
await run({ json: false, verbose: false });
|
||||
expect(vi.mocked(process.exit)).not.toHaveBeenCalled();
|
||||
expect(process.exitCode).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -36,6 +36,12 @@ export default defineCommand({
|
||||
},
|
||||
},
|
||||
async run({ args }) {
|
||||
// Set process.exitCode + return instead of process.exit(): process.exit()
|
||||
// terminates before Node flushes an async (non-TTY / piped) stdout, so
|
||||
// `hyperframes lint --json | ...` on Windows silently loses the entire JSON
|
||||
// payload written just above the exit. Letting run() return drains stdout
|
||||
// first, then Node exits with the set code — the pattern the other commands
|
||||
// (publish/transcribe/upgrade/play/present) already use.
|
||||
try {
|
||||
const project = resolveProject(args.dir);
|
||||
const lintResult = await lintProject(project.dir);
|
||||
@@ -51,7 +57,8 @@ export default defineCommand({
|
||||
filesScanned: lintResult.results.length,
|
||||
};
|
||||
console.log(JSON.stringify(withMeta(combined), null, 2));
|
||||
process.exit(combined.ok ? 0 : 1);
|
||||
process.exitCode = combined.ok ? 0 : 1;
|
||||
return;
|
||||
}
|
||||
|
||||
const fileCount = lintResult.results.length;
|
||||
@@ -72,7 +79,8 @@ export default defineCommand({
|
||||
});
|
||||
for (const line of lines) console.log(line);
|
||||
|
||||
process.exit(lintResult.totalErrors > 0 ? 1 : 0);
|
||||
process.exitCode = lintResult.totalErrors > 0 ? 1 : 0;
|
||||
return;
|
||||
} catch (err: unknown) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
if (args.json) {
|
||||
@@ -91,10 +99,11 @@ export default defineCommand({
|
||||
2,
|
||||
),
|
||||
);
|
||||
process.exit(1);
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
console.error(message);
|
||||
process.exit(1);
|
||||
process.exitCode = 1;
|
||||
}
|
||||
},
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user