-
Notifications
You must be signed in to change notification settings - Fork 2
fix(cli): surface verbose error details #416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@gh-symphony/cli": patch | ||
| --- | ||
|
|
||
| Fix issue #396 so `-v` / `--verbose` surfaces stack traces and error cause chains for top-level CLI and orchestrator failures, including daemon startup diagnostics. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -686,10 +686,11 @@ const handler = async ( | |
| const runtimeRoot = resolveRuntimeRoot(options.configDir); | ||
| const projectId = projectConfig.projectId; | ||
| let logLevel: OrchestratorLogLevel; | ||
| const requestedLogLevel = | ||
| parsed.logLevel ?? | ||
| (options.verbose ? "verbose" : process.env.SYMPHONY_LOG_LEVEL); | ||
| try { | ||
| logLevel = resolveOrchestratorLogLevel( | ||
| parsed.logLevel ?? process.env.SYMPHONY_LOG_LEVEL | ||
| ); | ||
| logLevel = resolveOrchestratorLogLevel(requestedLogLevel); | ||
| } catch (error) { | ||
| process.stderr.write( | ||
| `${error instanceof Error ? error.message : "Unsupported log level"}\n` | ||
|
|
@@ -709,7 +710,7 @@ const handler = async ( | |
| await startDaemon( | ||
| options, | ||
| projectId, | ||
| parsed.logLevel, | ||
| parsed.logLevel ?? (options.verbose ? "verbose" : undefined), | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 검증 결과: 데몬 경로의 verbose가 자식 프로세스 top-level catch까지 전달되지 않습니다 (Codex P2 확인).
로컬에서 확인: 따라서 범위는 좁습니다(서비스 레벨 verbose 로깅은 정상 전달되며, 영향은 데몬의 top-level uncaught 에러에 한정). 선택적/비차단 수정 제안: verbose 요청 시 자식 args에 Generated by Claude Code
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||
| parsed.httpPort, | ||
| parsed.webPort, | ||
| parsed.assignedOnly === true | ||
|
|
@@ -1080,6 +1081,7 @@ async function startDaemon( | |
| process.argv[1]!, | ||
| "repo", | ||
| "start", | ||
| ...(options.verbose ? ["--verbose"] : []), | ||
| ...(assignedOnly ? ["--assigned-only"] : []), | ||
| ...(httpPort !== undefined ? ["--http", String(httpPort)] : []), | ||
| ...(webPort !== undefined ? ["--web", String(webPort)] : []), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { formatErrorForTerminal, hasVerboseFlag } from "./error-format.js"; | ||
|
|
||
| describe("terminal error formatting", () => { | ||
| it("keeps non-verbose errors to a single message line", () => { | ||
| const error = new Error("top-level failure", { | ||
| cause: new Error("root cause"), | ||
| }); | ||
|
|
||
| expect(formatErrorForTerminal(error)).toBe("top-level failure\n"); | ||
| }); | ||
|
|
||
| it("prints stacks and walks causes when verbose", () => { | ||
| const cause = new Error("root cause"); | ||
| cause.stack = "Error: root cause\n at root"; | ||
| const error = new Error("top-level failure", { cause }); | ||
| error.stack = "Error: top-level failure\n at top"; | ||
|
|
||
| expect(formatErrorForTerminal(error, { verbose: true })).toBe( | ||
| [ | ||
| "Error: top-level failure", | ||
| " at top", | ||
| "Caused by: Error: root cause", | ||
| " at root", | ||
| "", | ||
| ].join("\n") | ||
| ); | ||
| }); | ||
|
|
||
| it("detects circular cause chains before repeating the top-level error", () => { | ||
| const cause = new Error("root cause"); | ||
| cause.stack = "Error: root cause\n at root"; | ||
| const error = new Error("top-level failure", { cause }); | ||
| error.stack = "Error: top-level failure\n at top"; | ||
| cause.cause = error; | ||
|
|
||
| expect(formatErrorForTerminal(error, { verbose: true })).toBe( | ||
| [ | ||
| "Error: top-level failure", | ||
| " at top", | ||
| "Caused by: Error: root cause", | ||
| " at root", | ||
| "Caused by: [Circular cause]", | ||
| "", | ||
| ].join("\n") | ||
| ); | ||
| }); | ||
|
|
||
| it("detects both verbose flags", () => { | ||
| expect(hasVerboseFlag(["repo", "start", "--verbose"])).toBe(true); | ||
| expect(hasVerboseFlag(["-v", "repo", "start"])).toBe(true); | ||
| expect(hasVerboseFlag(["repo", "start"])).toBe(false); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| export type TerminalErrorFormatOptions = { | ||
| verbose?: boolean; | ||
| }; | ||
|
|
||
| export function hasVerboseFlag(argv: readonly string[]): boolean { | ||
| return argv.some((arg) => arg === "--verbose" || arg === "-v"); | ||
| } | ||
|
|
||
| export function formatErrorForTerminal( | ||
| error: unknown, | ||
| options: TerminalErrorFormatOptions = {} | ||
| ): string { | ||
| if (!options.verbose) { | ||
| return `${error instanceof Error ? error.message : "Unknown error"}\n`; | ||
| } | ||
|
|
||
| const lines = [formatSingleError(error)]; | ||
| const seenCauses = new Set<object>(); | ||
| if (typeof error === "object" && error !== null) { | ||
| seenCauses.add(error); | ||
| } | ||
| let cause = resolveCause(error); | ||
|
|
||
| while (cause !== undefined) { | ||
| if (typeof cause === "object" && cause !== null) { | ||
| if (seenCauses.has(cause)) { | ||
| lines.push("Caused by: [Circular cause]"); | ||
| break; | ||
| } | ||
| seenCauses.add(cause); | ||
| } | ||
|
|
||
| lines.push(`Caused by: ${formatSingleError(cause)}`); | ||
| cause = resolveCause(cause); | ||
| } | ||
|
|
||
| return `${lines.join("\n")}\n`; | ||
| } | ||
|
|
||
| function formatSingleError(error: unknown): string { | ||
| if (error instanceof Error) { | ||
| return error.stack ?? error.message; | ||
| } | ||
|
|
||
| return String(error); | ||
| } | ||
|
|
||
| function resolveCause(error: unknown): unknown { | ||
| return error instanceof Error ? error.cause : undefined; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the daemon path,
gh-symphony -v repo start --daemonconverts the parent verbose flag only into--log-level verbosefor the detached child. The child is another CLI process whose top-level catch enables stack/cause output only whenhasVerboseFlag(process.argv.slice(2))sees-v/--verbose, so uncaught startup failures in the daemon log still get the old single-line message even though the user requested verbose diagnostics. Pass the verbose flag through separately, or make the child diagnostic check recognize this propagated state.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in
5049ad9:startDaemonnow passes--verboseto the child CLI when the parent global verbose option is set, alongside--log-level verbose. Addedstart.test.tscoverage asserting the daemon child argv includes both flags.