Skip to content

refactor(init): route wizard errors through framework error pipeline #678

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.

Sign up for GitHub

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

Merged
betegon merged 7 commits into from
Apr 7, 2026
+65 −50
Merged

refactor(init): route wizard errors through framework error pipeline #678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d61d7f0
refactor(init): route wizard errors through framework error pipeline
betegon Apr 7, 2026
f742575
fix: lint — import ordering, line length, noUselessElse
betegon Apr 7, 2026
dfe1666
fix(init): use valid placeholder URLs in dry-run mode
betegon Apr 7, 2026
a354a34
Revert "fix(init): use valid placeholder URLs in dry-run mode"
betegon Apr 7, 2026
6fe3daf
fix: update tests for WizardError throws + add 10s timeout guard
betegon Apr 7, 2026
e1984f1
fix: re-throw WizardError without double-displaying
betegon Apr 7, 2026
ad41990
Merge branch 'main' into refactor/wizard-error-pipeline
betegon Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
getExitCode,
OutputError,
stringifyUnknown,
WizardError,
} from "./lib/errors.js";
import { error as errorColor, warning } from "./lib/formatters/colors.js";
import { isRouteMap, type RouteMap } from "./lib/introspect.js";
Expand Down Expand Up @@ -324,6 +325,11 @@ const customText: ApplicationText = {
Sentry.captureException(exc);

if (exc instanceof CliError) {
// WizardError with rendered=true: clack already displayed the error.
// Return empty string to avoid double output, exit code flows through.
if (exc instanceof WizardError && exc.rendered) {
return "";
}
const prefix = ansiColor ? errorColor("Error:") : "Error:";
return `${prefix} ${exc.format()}`;
}
Expand Down
15 changes: 15 additions & 0 deletions src/lib/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,21 @@ export class TimeoutError extends CliError {
}
}

/**
* Error thrown by the init wizard when it has already displayed
* the error via clack UI. The `rendered` flag tells the framework
* error handler to skip its own formatting.
*/
export class WizardError extends CliError {
readonly rendered: boolean;

constructor(message: string, options?: { rendered?: boolean }) {
super(message);
this.name = "WizardError";
this.rendered = options?.rendered ?? true;
}
}

// Error Utilities

/**
Expand Down
47 changes: 24 additions & 23 deletions src/lib/init/wizard-runner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { createTeam, listTeams } from "../api-client.js";
import { formatBanner } from "../banner.js";
import { CLI_VERSION } from "../constants.js";
import { getAuthToken } from "../db/auth.js";
import { WizardError } from "../errors.js";
import { terminalLink } from "../formatters/colors.js";
import { getSentryBaseUrl } from "../sentry-urls.js";
import { slugify } from "../utils.js";
Expand Down Expand Up @@ -326,11 +327,10 @@ async function preamble(
dryRun: boolean
): Promise<boolean> {
if (!(yes || dryRun || process.stdin.isTTY)) {
process.stderr.write(
"Error: Interactive mode requires a terminal. Use --yes for non-interactive mode.\n"
throw new WizardError(
"Interactive mode requires a terminal. Use --yes for non-interactive mode.",
{ rendered: false }
);
process.exitCode = 1;
return false;
}

process.stderr.write(`\n${formatBanner()}\n\n`);
Expand Down Expand Up @@ -450,14 +450,14 @@ async function resolvePreSpinnerOptions(
}
log.error(errorMessage(err));
cancel("Setup failed.");
process.exitCode = 1;
return null;
throw new WizardError(errorMessage(err));
}
if (typeof orgResult !== "string") {
log.error(orgResult.error ?? "Failed to resolve organization.");
cancel("Setup failed.");
process.exitCode = 1;
return null;
throw new WizardError(
orgResult.error ?? "Failed to resolve organization."
);
}
opts = { ...opts, org: orgResult };
}
Expand Down Expand Up @@ -524,8 +524,7 @@ async function resolvePreSpinnerOptions(
`Create one at ${terminalLink(teamsUrl)} and run sentry init again.`
);
cancel("Setup failed.");
process.exitCode = 1;
return null;
throw new WizardError("No teams in your organization.");
}
} else if (teams.length === 1) {
opts = { ...opts, team: (teams[0] as SentryTeam).slug };
Comment on lines 524 to 530
Copy link
Copy Markdown

@sentry sentry bot Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: An outer try...catch block in resolvePreSpinnerOptions swallows a WizardError from a failed team creation, causing the wizard to continue with incomplete configuration.
Severity: HIGH

Suggested Fix

The outer catch block should re-throw the WizardError to ensure the wizard process is properly halted upon team resolution failure. This can be done by checking if (err instanceof WizardError) { throw err; } within the outer catch block.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/lib/init/wizard-runner.ts#L524-L530

Potential issue: In the `resolvePreSpinnerOptions` function, if an organization has no
teams, the wizard attempts to auto-create one. If this creation fails, a `WizardError`
is thrown. However, this error is caught by an outer `try...catch` block that does not
re-throw it. As a result, the function returns the `opts` object without the required
`team` property. The wizard then proceeds with this incomplete configuration,
potentially causing downstream failures or silent misconfiguration, despite logging an
error to the user. This behavior is a regression from the previous implementation which
would have halted execution.

Expand Down Expand Up @@ -565,6 +564,7 @@ async function resolvePreSpinnerOptions(
return opts;
}

// biome-ignore lint/complexity/noExcessiveCognitiveComplexity: sequential wizard orchestration with error handling branches
export async function runWizard(initialOptions: WizardOptions): Promise<void> {
const { directory, yes, dryRun, features } = initialOptions;

Expand Down Expand Up @@ -646,8 +646,7 @@ export async function runWizard(initialOptions: WizardOptions): Promise<void> {
spinState.running = false;
log.error(errorMessage(err));
cancel("Setup failed");
process.exitCode = 1;
return;
throw new WizardError(errorMessage(err));
}

const stepPhases = new Map<string, number>();
Expand All @@ -664,8 +663,7 @@ export async function runWizard(initialOptions: WizardOptions): Promise<void> {
spinState.running = false;
log.error(`No suspend payload found for step "${stepId}"`);
cancel("Setup failed");
process.exitCode = 1;
return;
throw new WizardError(`No suspend payload found for step "${stepId}"`);
}

const resumeData = await handleSuspendedStep(
Expand Down Expand Up @@ -700,14 +698,17 @@ export async function runWizard(initialOptions: WizardOptions): Promise<void> {
process.exitCode = 0;
return;
}
// Already rendered by an inner throw — don't double-display
if (err instanceof WizardError) {
throw err;
}
if (spinState.running) {
spin.stop("Error", 1);
spinState.running = false;
}
log.error(errorMessage(err));
cancel("Setup failed");
process.exitCode = 1;
return;
throw new WizardError(errorMessage(err));
}

handleFinalResult(result, spin, spinState);
Expand All @@ -726,14 +727,14 @@ function handleFinalResult(
spinState.running = false;
}
formatError(result);
process.exitCode = 1;
} else {
if (spinState.running) {
spin.stop("Done");
spinState.running = false;
}
formatResult(result);
throw new WizardError("Workflow returned an error");
}

if (spinState.running) {
spin.stop("Done");
spinState.running = false;
}
formatResult(result);
}

function extractSuspendPayload(
Expand Down
47 changes: 20 additions & 27 deletions test/lib/init/wizard-runner.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import * as banner from "../../../src/lib/banner.js";
import * as auth from "../../../src/lib/db/auth.js";
// biome-ignore lint/performance/noNamespaceImport: spyOn requires object reference
import * as userDb from "../../../src/lib/db/user.js";
import { WizardError } from "../../../src/lib/errors.js";
// biome-ignore lint/performance/noNamespaceImport: spyOn requires object reference
import * as fmt from "../../../src/lib/init/formatters.js";
// biome-ignore lint/performance/noNamespaceImport: spyOn requires object reference
Expand Down Expand Up @@ -259,7 +260,11 @@ afterEach(() => {

// ── Tests ───────────────────────────────────────────────────────────────────

describe("runWizard", () => {
// Guard against tests that accidentally wait for interactive input.
// If a test hangs for 10s it's almost certainly blocked on stdin, not slow I/O.
const TEST_TIMEOUT_MS = 10_000;

describe("runWizard", { timeout: TEST_TIMEOUT_MS }, () => {
describe("success path", () => {
test("calls formatResult when workflow completes successfully", async () => {
mockStartResult = { status: "success", result: { platform: "React" } };
Expand All @@ -273,23 +278,21 @@ describe("runWizard", () => {
});

describe("TTY check", () => {
test("writes error to stderr when not TTY and not --yes", async () => {
test("throws WizardError when not TTY and not --yes", async () => {
const origIsTTY = process.stdin.isTTY;
Object.defineProperty(process.stdin, "isTTY", {
value: false,
configurable: true,
});

await runWizard(makeOptions({ yes: false }));
await expect(runWizard(makeOptions({ yes: false }))).rejects.toThrow(
WizardError
);

Object.defineProperty(process.stdin, "isTTY", {
value: origIsTTY,
configurable: true,
});

const written = stderrSpy.mock.calls.map((c) => String(c[0])).join("");
expect(written).toContain("Interactive mode requires a terminal");
expect(process.exitCode).toBe(1);
});
});

Expand Down Expand Up @@ -404,12 +407,11 @@ describe("runWizard", () => {
// Advance past the timeout
jest.advanceTimersByTime(API_TIMEOUT_MS);

await promise;
await expect(promise).rejects.toThrow(WizardError);

expect(logErrorSpy).toHaveBeenCalled();
const errorMsg: string = logErrorSpy.mock.calls[0][0];
expect(errorMsg).toContain("timed out");
expect(process.exitCode).toBe(1);

jest.useRealTimers();
});
Expand All @@ -424,23 +426,21 @@ describe("runWizard", () => {
};
getWorkflowSpy.mockReturnValue(mockWorkflow as any);

await runWizard(makeOptions());
await expect(runWizard(makeOptions())).rejects.toThrow(WizardError);

expect(logErrorSpy).toHaveBeenCalledWith("Connection refused");
expect(cancelSpy).toHaveBeenCalledWith("Setup failed");
expect(process.exitCode).toBe(1);
});
});

describe("workflow failure", () => {
test("calls formatError when status is failed", async () => {
mockStartResult = { status: "failed", error: "workflow exploded" };

await runWizard(makeOptions());
await expect(runWizard(makeOptions())).rejects.toThrow(WizardError);

expect(formatErrorSpy).toHaveBeenCalled();
expect(formatResultSpy).not.toHaveBeenCalled();
expect(process.exitCode).toBe(1);
});
});

Expand All @@ -451,10 +451,9 @@ describe("runWizard", () => {
result: { exitCode: 10 },
};

await runWizard(makeOptions());
await expect(runWizard(makeOptions())).rejects.toThrow(WizardError);

expect(formatErrorSpy).toHaveBeenCalled();
expect(process.exitCode).toBe(1);
});
});

Expand Down Expand Up @@ -734,12 +733,11 @@ describe("runWizard", () => {
},
};

await runWizard(makeOptions());
await expect(runWizard(makeOptions())).rejects.toThrow(WizardError);

expect(logErrorSpy).toHaveBeenCalled();
const errorMsg: string = logErrorSpy.mock.calls[0][0];
expect(errorMsg).toContain("alien");
expect(process.exitCode).toBe(1);
});

test("handles missing suspend payload", async () => {
Expand All @@ -749,12 +747,11 @@ describe("runWizard", () => {
steps: {},
};

await runWizard(makeOptions());
await expect(runWizard(makeOptions())).rejects.toThrow(WizardError);

expect(logErrorSpy).toHaveBeenCalled();
const errorMsg: string = logErrorSpy.mock.calls[0][0];
expect(errorMsg).toContain("No suspend payload");
expect(process.exitCode).toBe(1);
});

test("non-WizardCancelledError in catch triggers log.error + cancel", async () => {
Expand All @@ -775,11 +772,10 @@ describe("runWizard", () => {
},
};

await runWizard(makeOptions());
await expect(runWizard(makeOptions())).rejects.toThrow(WizardError);

expect(logErrorSpy).toHaveBeenCalledWith("string error");
expect(cancelSpy).toHaveBeenCalledWith("Setup failed");
expect(process.exitCode).toBe(1);
});

test("falls back to result.suspendPayload when step payload missing", async () => {
Expand Down Expand Up @@ -880,12 +876,11 @@ describe("runWizard", () => {
};
getWorkflowSpy.mockReturnValue(badWorkflow as any);

await runWizard(makeOptions());
await expect(runWizard(makeOptions())).rejects.toThrow(WizardError);

expect(logErrorSpy).toHaveBeenCalledWith(
"Invalid workflow response: expected object"
);
expect(process.exitCode).toBe(1);
});

test("rejects response with invalid status", async () => {
Expand All @@ -900,12 +895,11 @@ describe("runWizard", () => {
};
getWorkflowSpy.mockReturnValue(badWorkflow as any);

await runWizard(makeOptions());
await expect(runWizard(makeOptions())).rejects.toThrow(WizardError);

expect(logErrorSpy).toHaveBeenCalledWith(
"Unexpected workflow status: banana"
);
expect(process.exitCode).toBe(1);
});

test("rejects null response from startAsync", async () => {
Expand All @@ -918,12 +912,11 @@ describe("runWizard", () => {
};
getWorkflowSpy.mockReturnValue(badWorkflow as any);

await runWizard(makeOptions());
await expect(runWizard(makeOptions())).rejects.toThrow(WizardError);

expect(logErrorSpy).toHaveBeenCalledWith(
"Invalid workflow response: expected object"
);
expect(process.exitCode).toBe(1);
});
});
});
Expand Down
Loading