Skip to content

Commit 0594274

Browse files
author
root
committed
Improve CLI startup failure log surfacing
1 parent 4b3073a commit 0594274

7 files changed

Lines changed: 410 additions & 18 deletions

File tree

packages/cli/src/bin.test.ts

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,66 @@ describe("main", () => {
212212
}
213213
});
214214

215+
it("prints only the recent tail for logs command", async () => {
216+
const logDir = mkdtempSync(join(tmpdir(), "cs-cli-logs-tail-"));
217+
const outFile = join(logDir, "server.out.log");
218+
const errFile = join(logDir, "server.err.log");
219+
const outLines = Array.from({ length: 45 }, (_, index) => `out line ${index + 1}`);
220+
const errLines = ["err line 1", "err line 2"];
221+
writeFileSync(outFile, `${outLines.join("\n")}\n`, "utf-8");
222+
writeFileSync(errFile, `${errLines.join("\n")}\n`, "utf-8");
223+
getServerStatus.mockResolvedValue({
224+
status: "running",
225+
pid: 424242,
226+
host: "127.0.0.1",
227+
port: 4187,
228+
restartCount: 2,
229+
outFile,
230+
errFile,
231+
startedAt: 1000,
232+
});
233+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
234+
235+
try {
236+
await main(["logs"]);
237+
expect(logSpy).toHaveBeenCalledWith(
238+
`${outLines.slice(-40).join("\n")}\n${errLines.join("\n")}`
239+
);
240+
} finally {
241+
if (existsSync(logDir)) {
242+
rmSync(logDir, { recursive: true, force: true });
243+
}
244+
}
245+
});
246+
247+
it("prints only the requested error log tail for logs command", async () => {
248+
const logDir = mkdtempSync(join(tmpdir(), "cs-cli-logs-errors-"));
249+
const outFile = join(logDir, "server.out.log");
250+
const errFile = join(logDir, "server.err.log");
251+
writeFileSync(outFile, "out line 1\nout line 2\n", "utf-8");
252+
writeFileSync(errFile, "err line 1\nerr line 2\n", "utf-8");
253+
getServerStatus.mockResolvedValue({
254+
status: "running",
255+
pid: 424242,
256+
host: "127.0.0.1",
257+
port: 4187,
258+
restartCount: 2,
259+
outFile,
260+
errFile,
261+
startedAt: 1000,
262+
});
263+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
264+
265+
try {
266+
await main(["logs", "--errors-only", "--tail", "1"]);
267+
expect(logSpy).toHaveBeenCalledWith("err line 2");
268+
} finally {
269+
if (existsSync(logDir)) {
270+
rmSync(logDir, { recursive: true, force: true });
271+
}
272+
}
273+
});
274+
215275
it("prints stop output for stop command", async () => {
216276
stopRunningServer.mockResolvedValue(true);
217277
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {});
@@ -383,6 +443,18 @@ describe("main", () => {
383443
expect(openBrowser).toHaveBeenCalledWith("http://127.0.0.1:4190");
384444
});
385445

446+
it("rethrows background startup failures for open and does not open the browser", async () => {
447+
const startupError = new Error(
448+
"Coder Studio failed to start in background: the managed process entered the errored state.\n\nRecent error log excerpt (/tmp/server.err.log):\nError: listen EADDRINUSE: address already in use 127.0.0.1:4187\n\nRun `coder-studio logs` for details or `coder-studio serve --foreground` for interactive debugging."
449+
);
450+
startManagedServer.mockRejectedValueOnce(startupError);
451+
452+
await expect(main(["open"])).rejects.toThrow(
453+
"Error: listen EADDRINUSE: address already in use 127.0.0.1:4187"
454+
);
455+
expect(openBrowser).not.toHaveBeenCalled();
456+
});
457+
386458
it("does not restart in non-interactive mode and prints a clear message", async () => {
387459
getServerStatus.mockResolvedValue({
388460
status: "running",
@@ -512,6 +584,20 @@ describe("parseArgs", () => {
512584
});
513585
});
514586

587+
it("parses logs command with tail count", () => {
588+
expect(parseArgs(["logs", "--tail", "100"])).toEqual({
589+
command: "logs",
590+
tail: 100,
591+
});
592+
});
593+
594+
it("parses logs command with errors-only flag", () => {
595+
expect(parseArgs(["logs", "--errors-only"])).toEqual({
596+
command: "logs",
597+
errorsOnly: true,
598+
});
599+
});
600+
515601
it("parses open command", () => {
516602
expect(parseArgs(["open"])).toEqual({
517603
command: "open",
@@ -633,6 +719,26 @@ describe("parseArgs", () => {
633719
expect(() => parseArgs(["logs", "--port", "4186"])).toThrow("Unknown option: --port");
634720
});
635721

722+
it("rejects logs tail with a non-numeric value", () => {
723+
expect(() => parseArgs(["logs", "--tail", "nope"])).toThrow("Invalid tail number");
724+
});
725+
726+
it("rejects logs tail with zero", () => {
727+
expect(() => parseArgs(["logs", "--tail", "0"])).toThrow("Invalid tail number");
728+
});
729+
730+
it("rejects logs tail with a negative value", () => {
731+
expect(() => parseArgs(["logs", "--tail", "-1"])).toThrow("Invalid tail number");
732+
});
733+
734+
it("rejects logs tail with a decimal value", () => {
735+
expect(() => parseArgs(["logs", "--tail", "1.5"])).toThrow("Invalid tail number");
736+
});
737+
738+
it("rejects logs tail with trailing garbage", () => {
739+
expect(() => parseArgs(["logs", "--tail", "10junk"])).toThrow("Invalid tail number");
740+
});
741+
636742
it("rejects stop-time data-dir overrides", () => {
637743
expect(() => parseArgs(["stop", "--data-dir", "/tmp/cs-data"])).toThrow(
638744
"Unknown option: --data-dir"

packages/cli/src/bin.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { fileURLToPath } from "url";
44
import { clearAuthBlockByIp, listAuthBlocks } from "./auth-control.js";
55
import { openBrowser } from "./browser.js";
66
import { type CliConfig, readCliConfig, writeCliConfig } from "./config-store.js";
7+
import { readLogExcerpt } from "./log-excerpt.js";
78
import { assertSupportedNodeVersion } from "./node-version.js";
89
import { parseArgs } from "./parse-args.js";
910
import { startManagedServer } from "./pm2-control.js";
@@ -13,6 +14,7 @@ import { startServer } from "./server-runner.js";
1314
import { getBrowserUrl, getListenIp, getListenUrl } from "./server-url.js";
1415

1516
const MANAGED_SERVER_WAIT_MS = 5000;
17+
const DEFAULT_LOG_TAIL_LINES = 40;
1618

1719
function formatConfig(config: CliConfig | null): string {
1820
return JSON.stringify(config ?? {}, null, 2);
@@ -38,15 +40,18 @@ function formatStatus(status: ServerStatus): string {
3840
].join("\n");
3941
}
4042

41-
function showLogs(status: ServerStatus): void {
42-
const contents = [status.outFile, status.errFile]
43+
function showLogs(
44+
status: ServerStatus,
45+
{
46+
tail = DEFAULT_LOG_TAIL_LINES,
47+
errorsOnly = false,
48+
}: { tail?: number; errorsOnly?: boolean } = {}
49+
): void {
50+
const paths = errorsOnly ? [status.errFile] : [status.outFile, status.errFile];
51+
const contents = paths
4352
.filter((path, index, paths) => paths.indexOf(path) === index)
4453
.flatMap((path) => {
45-
if (!existsSync(path)) {
46-
return [];
47-
}
48-
49-
const content = readFileSync(path, "utf-8").trim();
54+
const content = readLogExcerpt(path, { maxLines: tail, maxChars: null });
5055
return content ? [content] : [];
5156
});
5257

@@ -303,7 +308,7 @@ export async function main(argv = process.argv.slice(2)): Promise<void> {
303308
}
304309

305310
if (args.command === "logs") {
306-
showLogs(await getServerStatus());
311+
showLogs(await getServerStatus(), { tail: args.tail, errorsOnly: args.errorsOnly });
307312
return;
308313
}
309314

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { existsSync, mkdtempSync, rmSync, writeFileSync } from "fs";
2+
import { tmpdir } from "os";
3+
import { join } from "path";
4+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
5+
import { getFileSize, readLogExcerpt } from "./log-excerpt";
6+
7+
describe("log-excerpt", () => {
8+
let testDir: string;
9+
10+
beforeEach(() => {
11+
testDir = mkdtempSync(join(tmpdir(), "cs-log-excerpt-"));
12+
});
13+
14+
afterEach(() => {
15+
if (existsSync(testDir)) {
16+
rmSync(testDir, { recursive: true, force: true });
17+
}
18+
});
19+
20+
it("returns only the newly appended log lines after the provided offset", () => {
21+
const logPath = join(testDir, "server.err.log");
22+
writeFileSync(logPath, "stale line 1\nstale line 2\n");
23+
24+
const startOffset = getFileSize(logPath);
25+
writeFileSync(logPath, "fresh line 1\nfresh line 2\n", { flag: "a" });
26+
27+
expect(readLogExcerpt(logPath, { startOffset })).toBe("fresh line 1\nfresh line 2");
28+
});
29+
30+
it("returns the recent tail when the new content exceeds configured limits", () => {
31+
const logPath = join(testDir, "server.err.log");
32+
writeFileSync(logPath, "line 1\nline 2\nline 3\nline 4\n");
33+
34+
expect(
35+
readLogExcerpt(logPath, {
36+
startOffset: 0,
37+
maxBytes: 64,
38+
maxLines: 2,
39+
maxChars: 8,
40+
})
41+
).toBe("…\nline 4");
42+
});
43+
44+
it("reads the replacement file when the previous offset is beyond the new size", () => {
45+
const logPath = join(testDir, "server.err.log");
46+
writeFileSync(logPath, "stale line 1\nstale line 2\n");
47+
48+
const startOffset = getFileSize(logPath);
49+
writeFileSync(logPath, "fresh replacement line\n", "utf-8");
50+
51+
expect(readLogExcerpt(logPath, { startOffset })).toBe("fresh replacement line");
52+
});
53+
54+
it("drops a partial leading line when bounded tail reads start mid-line", () => {
55+
const logPath = join(testDir, "server.err.log");
56+
writeFileSync(logPath, "very long first line\nsecond line\nthird line\n", "utf-8");
57+
58+
expect(readLogExcerpt(logPath, { maxBytes: 30, maxChars: null })).toBe(
59+
"second line\nthird line"
60+
);
61+
});
62+
});

packages/cli/src/log-excerpt.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { closeSync, existsSync, openSync, readSync, statSync } from "fs";
2+
3+
const DEFAULT_MAX_LINES = 40;
4+
const DEFAULT_MAX_CHARS = 4000;
5+
const DEFAULT_MAX_BYTES = 16 * 1024;
6+
7+
export interface LogExcerptOptions {
8+
startOffset?: number;
9+
maxBytes?: number;
10+
maxLines?: number;
11+
maxChars?: number | null;
12+
}
13+
14+
export const getFileSize = (path: string): number => {
15+
if (!existsSync(path)) {
16+
return 0;
17+
}
18+
19+
try {
20+
return statSync(path).size;
21+
} catch {
22+
return 0;
23+
}
24+
};
25+
26+
export const readLogExcerpt = (
27+
path: string,
28+
{
29+
startOffset = 0,
30+
maxBytes = DEFAULT_MAX_BYTES,
31+
maxLines = DEFAULT_MAX_LINES,
32+
maxChars = DEFAULT_MAX_CHARS,
33+
}: LogExcerptOptions = {}
34+
): string | null => {
35+
if (!existsSync(path)) {
36+
return null;
37+
}
38+
39+
const fileSize = getFileSize(path);
40+
const safeOffset = startOffset > fileSize ? 0 : Math.max(0, startOffset);
41+
if (fileSize === safeOffset) {
42+
return null;
43+
}
44+
45+
const bytesToRead = Math.min(fileSize - safeOffset, maxBytes);
46+
const readStart = fileSize - bytesToRead;
47+
const buffer = Buffer.allocUnsafe(bytesToRead);
48+
const fd = openSync(path, "r");
49+
let content = "";
50+
let startsMidLine = false;
51+
52+
try {
53+
const bytesRead = readSync(fd, buffer, 0, bytesToRead, readStart);
54+
if (bytesRead === 0) {
55+
return null;
56+
}
57+
58+
if (readStart > safeOffset && readStart > 0) {
59+
const previousByte = Buffer.alloc(1);
60+
const previousBytesRead = readSync(fd, previousByte, 0, 1, readStart - 1);
61+
startsMidLine = previousBytesRead === 1 && previousByte[0] !== 0x0a;
62+
}
63+
64+
content = buffer.toString("utf-8", 0, bytesRead).trimEnd();
65+
} finally {
66+
closeSync(fd);
67+
}
68+
69+
if (startsMidLine) {
70+
const firstNewlineIndex = content.indexOf("\n");
71+
if (firstNewlineIndex !== -1) {
72+
content = content.slice(firstNewlineIndex + 1);
73+
}
74+
}
75+
76+
if (content.length === 0) {
77+
return null;
78+
}
79+
80+
const lines = content
81+
.split(/\r?\n/u)
82+
.map((line) => line.trimEnd())
83+
.filter((line) => line.length > 0);
84+
if (lines.length === 0) {
85+
return null;
86+
}
87+
88+
const excerpt = lines.slice(-maxLines).join("\n");
89+
if (maxChars === null || excerpt.length <= maxChars) {
90+
return excerpt;
91+
}
92+
93+
return `…${excerpt.slice(-maxChars + 1)}`;
94+
};

0 commit comments

Comments
 (0)