diff --git a/src/app.ts b/src/app.ts index 2089d77f1..e12e5401b 100644 --- a/src/app.ts +++ b/src/app.ts @@ -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"; @@ -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()}`; } diff --git a/src/lib/errors.ts b/src/lib/errors.ts index 29c008c97..e866ea754 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -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 /** diff --git a/src/lib/init/wizard-runner.ts b/src/lib/init/wizard-runner.ts index c0f49067a..f0f823975 100644 --- a/src/lib/init/wizard-runner.ts +++ b/src/lib/init/wizard-runner.ts @@ -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"; @@ -326,11 +327,10 @@ async function preamble( dryRun: boolean ): Promise { 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`); @@ -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 }; } @@ -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 }; @@ -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 { const { directory, yes, dryRun, features } = initialOptions; @@ -646,8 +646,7 @@ export async function runWizard(initialOptions: WizardOptions): Promise { spinState.running = false; log.error(errorMessage(err)); cancel("Setup failed"); - process.exitCode = 1; - return; + throw new WizardError(errorMessage(err)); } const stepPhases = new Map(); @@ -664,8 +663,7 @@ export async function runWizard(initialOptions: WizardOptions): Promise { 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( @@ -700,14 +698,17 @@ export async function runWizard(initialOptions: WizardOptions): Promise { 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); @@ -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( diff --git a/test/lib/init/wizard-runner.test.ts b/test/lib/init/wizard-runner.test.ts index 111afc1fc..3f64564c8 100644 --- a/test/lib/init/wizard-runner.test.ts +++ b/test/lib/init/wizard-runner.test.ts @@ -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 @@ -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" } }; @@ -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); }); }); @@ -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(); }); @@ -424,11 +426,10 @@ 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); }); }); @@ -436,11 +437,10 @@ describe("runWizard", () => { 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); }); }); @@ -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); }); }); @@ -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 () => { @@ -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 () => { @@ -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 () => { @@ -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 () => { @@ -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 () => { @@ -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); }); }); });