Skip to content

Commit df7b32d

Browse files
betegonclaude
andauthored
fix(init): surface wizard exit code and command stderr in Sentry events (CLI-1JA) (#980)
## Summary Every event in CLI-1JA was identical in Sentry: \`WizardError: Workflow returned an error\` with no way to distinguish exit 11 (no files found), 20 (unknown platform), 30 (install failed), or 41 (codemod failed). The server already tags \`wizard.exit_code\` via \`bailWithTracking\`, but the CLI-side event never reflected it. ## Changes **\`wizard-runner.ts\`** — \`handleFinalResult\` now tags \`wizard.exit_code\` with the numeric workflow code when present, and uses \`result.result?.message ?? result.error\` as the \`WizardError\` message instead of the generic fallback. Priority order matches \`formatError\` (which already showed the right message in the terminal, just not in Sentry). **\`run-commands.ts\`** — adds an explicit \`addBreadcrumb\` with up to 500 chars of stderr when a spawned command exits non-zero. The automatic Sentry child-process breadcrumb only captured the exit code; pnpm/npm/xcodebuild failure reasons were invisible. ## Test Plan - New events in CLI-1JA should carry \`wizard.exit_code: 11/20/30/41\` so failures are immediately distinguishable - The exception message will be the actual bail reason (e.g. "Dependency installation failed after 5 attempts…") instead of "Workflow returned an error" - Command failure breadcrumbs will show the first 500 chars of stderr from pnpm/npm/xcodebuild 8 new unit tests cover both changes (\`wizard-runner-handle-final-result.mocked.test.ts\`, \`run-commands.mocked.test.ts\`). Fixes CLI-1JA --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent a80bb0f commit df7b32d

4 files changed

Lines changed: 267 additions & 2 deletions

File tree

src/lib/init/tools/run-commands.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { addBreadcrumb } from "@sentry/node-core/light";
12
import { DEFAULT_COMMAND_TIMEOUT_MS } from "../constants.js";
23
import type { RunCommandsPayload, ToolResult } from "../types.js";
34
import {
@@ -46,6 +47,15 @@ export async function runCommands(
4647
const result = await runSingleCommand(command, payload.cwd, timeoutMs);
4748
results.push(result);
4849
if (result.exitCode !== 0) {
50+
addBreadcrumb({
51+
level: "error",
52+
message: `Command failed: ${command.original}`,
53+
data: {
54+
exitCode: result.exitCode,
55+
stderr: result.stderr.slice(0, 500),
56+
cwd: payload.cwd,
57+
},
58+
});
4959
return {
5060
ok: false,
5161
error: `Command "${command.original}" failed with exit code ${result.exitCode}: ${result.stderr}`,

src/lib/init/wizard-runner.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -846,7 +846,7 @@ export async function runWizard(initialOptions: WizardOptions): Promise<void> {
846846
}
847847
}
848848

849-
function handleFinalResult(
849+
export function handleFinalResult(
850850
result: WorkflowRunResult,
851851
spin: SpinnerHandle,
852852
spinState: SpinState,
@@ -865,7 +865,13 @@ function handleFinalResult(
865865
const workflowCode = result.result?.exitCode;
866866
const exitCode = mapWorkflowExitCode(workflowCode);
867867
setTag("wizard.outcome", "errored");
868-
throw new WizardError("Workflow returned an error", { exitCode });
868+
if (workflowCode !== undefined) {
869+
setTag("wizard.exit_code", workflowCode);
870+
}
871+
throw new WizardError(
872+
result.result?.message ?? result.error ?? "Workflow returned an error",
873+
{ exitCode }
874+
);
869875
}
870876

871877
if (spinState.running) {
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/**
2+
* Unit tests for the run-commands tool.
3+
*
4+
* Kept as a mocked sibling file because mock.module() on @sentry/node-core/light
5+
* must precede all module imports to take effect.
6+
*/
7+
8+
import { beforeEach, describe, expect, mock, test } from "bun:test";
9+
import type { RunCommandsPayload } from "../../src/lib/init/types.js";
10+
11+
// ============================================================================
12+
// Mock Setup — must precede all imports of the module under test
13+
// ============================================================================
14+
15+
type Breadcrumb = {
16+
level: string;
17+
message: string;
18+
data: { exitCode: number; stderr: string; cwd: string };
19+
};
20+
21+
const breadcrumbs: Breadcrumb[] = [];
22+
23+
mock.module("@sentry/node-core/light", () => ({
24+
addBreadcrumb: (crumb: Breadcrumb) => breadcrumbs.push(crumb),
25+
}));
26+
27+
// Import AFTER mock setup
28+
import { runCommands } from "../../src/lib/init/tools/run-commands.js";
29+
30+
// ============================================================================
31+
// Helpers
32+
// ============================================================================
33+
34+
function makePayload(commands: string[], cwd = "/tmp"): RunCommandsPayload {
35+
return { type: "tool", operation: "run-commands", cwd, params: { commands } };
36+
}
37+
38+
// ============================================================================
39+
// Tests
40+
// ============================================================================
41+
42+
beforeEach(() => breadcrumbs.splice(0));
43+
44+
describe("runCommands breadcrumb on failure", () => {
45+
test("emits an error breadcrumb with stderr when a command exits non-zero", async () => {
46+
// `ls /nonexistent-sentry-test-path` exits 1 on macOS/Linux with a
47+
// "No such file" message on stderr — a reliable non-zero exit without
48+
// requiring shell built-ins.
49+
const result = await runCommands(
50+
makePayload(["ls /nonexistent-sentry-test-path-xyz"]),
51+
{ dryRun: false }
52+
);
53+
54+
expect(result.ok).toBe(false);
55+
expect(breadcrumbs).toHaveLength(1);
56+
57+
const crumb = breadcrumbs[0];
58+
expect(crumb.level).toBe("error");
59+
expect(crumb.message).toContain("ls");
60+
expect(crumb.data.exitCode).not.toBe(0);
61+
expect(typeof crumb.data.stderr).toBe("string");
62+
expect(crumb.data.cwd).toBe("/tmp");
63+
});
64+
65+
test("does not emit a breadcrumb when the command succeeds", async () => {
66+
const result = await runCommands(makePayload(["echo hello"]), {
67+
dryRun: false,
68+
});
69+
70+
expect(result.ok).toBe(true);
71+
expect(breadcrumbs).toHaveLength(0);
72+
});
73+
74+
test("does not emit a breadcrumb in dry-run mode", async () => {
75+
const result = await runCommands(
76+
makePayload(["ls /nonexistent-sentry-test-path-xyz"]),
77+
{ dryRun: true }
78+
);
79+
80+
expect(result.ok).toBe(true);
81+
expect(breadcrumbs).toHaveLength(0);
82+
});
83+
});
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
/**
2+
* Unit tests for handleFinalResult in wizard-runner.ts.
3+
*
4+
* Kept as a mocked sibling file because mock.module() on @sentry/node-core/light
5+
* would pollute the module graph for other wizard-runner tests that may not
6+
* want Sentry calls mocked.
7+
*/
8+
9+
import { beforeEach, describe, expect, mock, test } from "bun:test";
10+
import type {
11+
WizardOutput,
12+
WorkflowRunResult,
13+
} from "../../src/lib/init/types.js";
14+
15+
// ============================================================================
16+
// Mock Setup — must precede all imports of the module under test
17+
// ============================================================================
18+
19+
const tags: Record<string, unknown> = {};
20+
21+
mock.module("@sentry/node-core/light", () => ({
22+
addBreadcrumb: () => null,
23+
captureException: () => null,
24+
getTraceData: () => ({}),
25+
setTag: (key: string, value: unknown) => {
26+
tags[key] = value;
27+
},
28+
}));
29+
30+
import { WizardError } from "../../src/lib/errors.js";
31+
// Import AFTER mock setup so the mocked module is used
32+
import { handleFinalResult } from "../../src/lib/init/wizard-runner.js";
33+
34+
// ============================================================================
35+
// Test helpers
36+
// ============================================================================
37+
38+
function makeSpinnerHandle() {
39+
return { start: () => null, stop: () => null };
40+
}
41+
42+
function makeSpinState(running = false) {
43+
return { running };
44+
}
45+
46+
/** Minimal WizardUI stub — only the methods formatError touches. */
47+
function makeUI() {
48+
const noop = () => null;
49+
return {
50+
log: { error: noop, warn: noop, info: noop, message: noop },
51+
cancel: noop,
52+
feedback: noop,
53+
summary: noop,
54+
outro: noop,
55+
intro: noop,
56+
setStep: noop,
57+
markFilesAnalyzed: noop,
58+
} as any;
59+
}
60+
61+
function makeBailResult(
62+
partial: Partial<WizardOutput> = {}
63+
): WorkflowRunResult {
64+
return {
65+
status: "success",
66+
result: { exitCode: 30, ...partial },
67+
};
68+
}
69+
70+
// ============================================================================
71+
// Tests
72+
// ============================================================================
73+
74+
beforeEach(() => {
75+
for (const key of Object.keys(tags)) {
76+
delete tags[key];
77+
}
78+
});
79+
80+
describe("handleFinalResult", () => {
81+
describe("WizardError message", () => {
82+
test("uses bail message from result.result.message when present", () => {
83+
const result = makeBailResult({
84+
message:
85+
"Dependency installation failed after 5 attempts: pnpm exited with code 1",
86+
});
87+
88+
expect(() =>
89+
handleFinalResult(
90+
result,
91+
makeSpinnerHandle(),
92+
makeSpinState(),
93+
makeUI()
94+
)
95+
).toThrow(
96+
"Dependency installation failed after 5 attempts: pnpm exited with code 1"
97+
);
98+
});
99+
100+
test("falls back to generic message when result.result.message is absent", () => {
101+
const result = makeBailResult({ message: undefined });
102+
103+
expect(() =>
104+
handleFinalResult(
105+
result,
106+
makeSpinnerHandle(),
107+
makeSpinState(),
108+
makeUI()
109+
)
110+
).toThrow("Workflow returned an error");
111+
});
112+
});
113+
114+
describe("wizard.exit_code tag", () => {
115+
test("tags wizard.exit_code with the workflow exit code", () => {
116+
const result = makeBailResult({ exitCode: 11 });
117+
118+
expect(() =>
119+
handleFinalResult(
120+
result,
121+
makeSpinnerHandle(),
122+
makeSpinState(),
123+
makeUI()
124+
)
125+
).toThrow(WizardError);
126+
127+
expect(tags["wizard.exit_code"]).toBe(11);
128+
});
129+
130+
test("does not set wizard.exit_code when exitCode is absent", () => {
131+
const result: WorkflowRunResult = {
132+
status: "failed",
133+
error: "network error",
134+
};
135+
136+
expect(() =>
137+
handleFinalResult(
138+
result,
139+
makeSpinnerHandle(),
140+
makeSpinState(),
141+
makeUI()
142+
)
143+
).toThrow(WizardError);
144+
145+
expect(tags["wizard.exit_code"]).toBeUndefined();
146+
});
147+
});
148+
149+
describe("WizardError message — result.error fallback", () => {
150+
test("uses result.error when result.result is absent (plain workflow failure)", () => {
151+
const result: WorkflowRunResult = {
152+
status: "failed",
153+
error: "upstream network timeout",
154+
};
155+
156+
expect(() =>
157+
handleFinalResult(
158+
result,
159+
makeSpinnerHandle(),
160+
makeSpinState(),
161+
makeUI()
162+
)
163+
).toThrow("upstream network timeout");
164+
});
165+
});
166+
});

0 commit comments

Comments
 (0)