diff --git a/AGENTS.md b/AGENTS.md index a89137cb8..b27fdc337 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -147,25 +147,24 @@ if (result.success) { ### Error Handling -```typescript -// Custom error classes extend Error -export class SentryApiError extends Error { - readonly status: number; - constructor(message: string, status: number) { - super(message); - this.name = "SentryApiError"; - this.status = status; - } -} +All CLI errors extend the `CliError` base class from `src/lib/errors.ts`: -// In commands: catch and write to stderr -try { - // ... -} catch (error) { - const message = error instanceof Error ? error.message : String(error); - process.stderr.write(`Error: ${message}\n`); - process.exitCode = 1; -} +```typescript +// Error hierarchy in src/lib/errors.ts +CliError (base) +├── ApiError (HTTP/API failures - status, detail, endpoint) +├── AuthError (authentication - reason: 'not_authenticated' | 'expired' | 'invalid') +├── ConfigError (configuration - suggestion?) +├── ValidationError (input validation - field?) +└── DeviceFlowError (OAuth flow - code) + +// Usage: throw specific error types +import { ApiError, AuthError } from "../lib/errors.js"; +throw new AuthError("not_authenticated"); +throw new ApiError("Request failed", 404, "Not found"); + +// In commands: let errors propagate to central handler +// The bin.ts entry point catches and formats all errors consistently ``` ### Async Config Functions diff --git a/packages/cli/src/bin.ts b/packages/cli/src/bin.ts index 21cf79105..55d67d113 100755 --- a/packages/cli/src/bin.ts +++ b/packages/cli/src/bin.ts @@ -2,5 +2,11 @@ import { run } from "@stricli/core"; import { app } from "./app.js"; import { buildContext } from "./context.js"; +import { formatError, getExitCode } from "./lib/errors.js"; -await run(app, process.argv.slice(2), buildContext(process)); +try { + await run(app, process.argv.slice(2), buildContext(process)); +} catch (error) { + process.stderr.write(`Error: ${formatError(error)}\n`); + process.exit(getExitCode(error)); +} diff --git a/packages/cli/src/commands/api.ts b/packages/cli/src/commands/api.ts index eacf5211d..63f2cef64 100644 --- a/packages/cli/src/commands/api.ts +++ b/packages/cli/src/commands/api.ts @@ -217,43 +217,36 @@ export const apiCommand = buildCommand({ endpoint: string ): Promise { const { process } = this; - const { stdout, stderr } = process; + const { stdout } = process; - try { - const body = - flags.field?.length > 0 ? parseFields(flags.field) : undefined; - const headers = - flags.header?.length > 0 ? parseHeaders(flags.header) : undefined; + const body = flags.field?.length > 0 ? parseFields(flags.field) : undefined; + const headers = + flags.header?.length > 0 ? parseHeaders(flags.header) : undefined; - const response = await rawApiRequest(endpoint, { - method: flags.method, - body, - headers, - }); + const response = await rawApiRequest(endpoint, { + method: flags.method, + body, + headers, + }); - // Silent mode - only set exit code - if (flags.silent) { - if (response.status >= 400) { - process.exitCode = 1; - } - return; + // Silent mode - only set exit code + if (flags.silent) { + if (response.status >= 400) { + process.exitCode = 1; } + return; + } - // Output headers if requested - if (flags.include) { - writeResponseHeaders(stdout, response.status, response.headers); - } + // Output headers if requested + if (flags.include) { + writeResponseHeaders(stdout, response.status, response.headers); + } - // Output body - writeResponseBody(stdout, response.body); + // Output body + writeResponseBody(stdout, response.body); - // Set exit code for error responses - if (response.status >= 400) { - process.exitCode = 1; - } - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - stderr.write(`Error: ${message}\n`); + // Set exit code for error responses + if (response.status >= 400) { process.exitCode = 1; } }, diff --git a/packages/cli/src/commands/auth/login.ts b/packages/cli/src/commands/auth/login.ts index 35660d8a3..e9cb02a72 100644 --- a/packages/cli/src/commands/auth/login.ts +++ b/packages/cli/src/commands/auth/login.ts @@ -8,6 +8,7 @@ import { isAuthenticated, setAuthToken, } from "../../lib/config.js"; +import { AuthError } from "../../lib/errors.js"; import { completeOAuthFlow, performDeviceFlow } from "../../lib/oauth.js"; import { generateQRCode } from "../../lib/qrcode.js"; @@ -68,7 +69,8 @@ export const loginCommand = buildCommand({ } catch { // Token is invalid - clear it and throw await clearAuth(); - throw new Error( + throw new AuthError( + "invalid", "Invalid API token. Please check your token and try again." ); } @@ -81,56 +83,51 @@ export const loginCommand = buildCommand({ // Device Flow OAuth process.stdout.write("Starting authentication...\n\n"); - try { - const tokenResponse = await performDeviceFlow( - { - onUserCode: async ( - userCode, - verificationUri, - verificationUriComplete - ) => { - const browserOpened = await openBrowser(verificationUriComplete); - if (browserOpened) { - process.stdout.write("Opening browser...\n\n"); - } + const tokenResponse = await performDeviceFlow( + { + onUserCode: async ( + userCode, + verificationUri, + verificationUriComplete + ) => { + const browserOpened = await openBrowser(verificationUriComplete); + if (browserOpened) { + process.stdout.write("Opening browser...\n\n"); + } - if (flags.qr) { - process.stdout.write( - "Scan this QR code or visit the URL below:\n\n" - ); - const qr = await generateQRCode(verificationUriComplete); - process.stdout.write(qr); - process.stdout.write("\n"); - } + if (flags.qr) { + process.stdout.write( + "Scan this QR code or visit the URL below:\n\n" + ); + const qr = await generateQRCode(verificationUriComplete); + process.stdout.write(qr); + process.stdout.write("\n"); + } - process.stdout.write(`URL: ${verificationUri}\n`); - process.stdout.write(`Code: ${userCode}\n\n`); - process.stdout.write("Waiting for authorization...\n"); - }, - onPolling: () => { - // Could add a spinner or dots here - process.stdout.write("."); - }, + process.stdout.write(`URL: ${verificationUri}\n`); + process.stdout.write(`Code: ${userCode}\n\n`); + process.stdout.write("Waiting for authorization...\n"); }, - flags.timeout * 1000 - ); + onPolling: () => { + // Could add a spinner or dots here + process.stdout.write("."); + }, + }, + flags.timeout * 1000 + ); - // Clear the polling dots - process.stdout.write("\n\n"); + // Clear the polling dots + process.stdout.write("\n\n"); - // Store the token - await completeOAuthFlow(tokenResponse); + // Store the token + await completeOAuthFlow(tokenResponse); - process.stdout.write("✓ Authentication successful!\n"); - process.stdout.write(` Config saved to: ${getConfigPath()}\n`); + process.stdout.write("✓ Authentication successful!\n"); + process.stdout.write(` Config saved to: ${getConfigPath()}\n`); - if (tokenResponse.expires_in) { - const hours = Math.round(tokenResponse.expires_in / 3600); - process.stdout.write(` Token expires in: ${hours} hours\n`); - } - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new Error(`Authentication failed: ${message}`); + if (tokenResponse.expires_in) { + const hours = Math.round(tokenResponse.expires_in / 3600); + process.stdout.write(` Token expires in: ${hours} hours\n`); } }, }); diff --git a/packages/cli/src/commands/auth/status.ts b/packages/cli/src/commands/auth/status.ts index 0739e8f2e..1ab7ead5e 100644 --- a/packages/cli/src/commands/auth/status.ts +++ b/packages/cli/src/commands/auth/status.ts @@ -14,6 +14,7 @@ import { isAuthenticated, readConfig, } from "../../lib/config.js"; +import { AuthError } from "../../lib/errors.js"; import { formatExpiration, maskToken } from "../../lib/formatters/human.js"; import type { SentryConfig, Writer } from "../../types/index.js"; @@ -66,7 +67,10 @@ async function writeDefaults(stdout: Writer): Promise { /** * Verify credentials by fetching organizations */ -async function verifyCredentials(stdout: Writer): Promise { +async function verifyCredentials( + stdout: Writer, + stderr: Writer +): Promise { stdout.write("\nVerifying credentials...\n"); try { @@ -84,7 +88,7 @@ async function verifyCredentials(stdout: Writer): Promise { } } catch (error) { const message = error instanceof Error ? error.message : String(error); - stdout.write(`\n✗ Could not verify credentials: ${message}\n`); + stderr.write(`\n✗ Could not verify credentials: ${message}\n`); } } @@ -106,7 +110,7 @@ export const statusCommand = buildCommand({ }, async func(this: SentryContext, flags: StatusFlags): Promise { const { process } = this; - const { stdout } = process; + const { stdout, stderr } = process; const config = await readConfig(); const authenticated = await isAuthenticated(); @@ -114,15 +118,13 @@ export const statusCommand = buildCommand({ stdout.write(`Config file: ${getConfigPath()}\n\n`); if (!authenticated) { - throw new Error( - "Not authenticated. Run 'sentry auth login' to authenticate." - ); + throw new AuthError("not_authenticated"); } stdout.write("Status: Authenticated ✓\n\n"); writeTokenInfo(stdout, config, flags.showToken); await writeDefaults(stdout); - await verifyCredentials(stdout); + await verifyCredentials(stdout, stderr); }, }); diff --git a/packages/cli/src/lib/api-client.ts b/packages/cli/src/lib/api-client.ts index f11be60ec..74d1596a5 100644 --- a/packages/cli/src/lib/api-client.ts +++ b/packages/cli/src/lib/api-client.ts @@ -13,6 +13,7 @@ import type { SentryProject, } from "../types/index.js"; import { getAuthToken } from "./config.js"; +import { ApiError, AuthError } from "./errors.js"; const DEFAULT_SENTRY_URL = "https://sentry.io"; @@ -50,22 +51,6 @@ function normalizePath(endpoint: string): string { */ const SHORT_ID_PATTERN = /[a-zA-Z]/; -// ───────────────────────────────────────────────────────────────────────────── -// Error Handling -// ───────────────────────────────────────────────────────────────────────────── - -export class SentryApiError extends Error { - readonly status: number; - readonly detail?: string; - - constructor(message: string, status: number, detail?: string) { - super(message); - this.name = "SentryApiError"; - this.status = status; - this.detail = detail; - } -} - // ───────────────────────────────────────────────────────────────────────────── // Request Helpers // ───────────────────────────────────────────────────────────────────────────── @@ -79,16 +64,14 @@ type ApiRequestOptions = { /** * Create a configured ky instance with retry, timeout, and authentication. * - * @throws {SentryApiError} When not authenticated (status 401) + * @throws {AuthError} When not authenticated + * @throws {ApiError} When API request fails */ async function createApiClient(): Promise { const token = await getAuthToken(); if (!token) { - throw new SentryApiError( - "Not authenticated. Run 'sentry auth login' first.", - 401 - ); + throw new AuthError("not_authenticated"); } return ky.create({ @@ -117,7 +100,7 @@ async function createApiClient(): Promise { } catch { detail = text; } - throw new SentryApiError( + throw new ApiError( `API request failed: ${response.status} ${response.statusText}`, response.status, detail @@ -140,7 +123,7 @@ function buildSearchParams( params?: Record ): URLSearchParams | undefined { if (!params) { - return undefined; + return; } const searchParams = new URLSearchParams(); @@ -163,7 +146,8 @@ function buildSearchParams( * @param endpoint - API endpoint path (e.g., "/organizations/") * @param options - Request options including method, body, and query params * @returns Parsed JSON response - * @throws {SentryApiError} On authentication failure or API errors + * @throws {AuthError} When not authenticated + * @throws {ApiError} On API errors */ export async function apiRequest( endpoint: string, @@ -189,7 +173,7 @@ export async function apiRequest( * @param endpoint - API endpoint path (e.g., "/organizations/") * @param options - Request options including method, body, params, and custom headers * @returns Response status, headers, and parsed body - * @throws {SentryApiError} Only on authentication failure (not on API errors) + * @throws {AuthError} Only on authentication failure (not on API errors) */ export async function rawApiRequest( endpoint: string, diff --git a/packages/cli/src/lib/errors.ts b/packages/cli/src/lib/errors.ts new file mode 100644 index 000000000..5badc9a29 --- /dev/null +++ b/packages/cli/src/lib/errors.ts @@ -0,0 +1,201 @@ +/** + * CLI Error Hierarchy + * + * Unified error classes for consistent error handling across the CLI. + */ + +// ───────────────────────────────────────────────────────────────────────────── +// Base Error +// ───────────────────────────────────────────────────────────────────────────── + +/** + * Base class for all CLI errors. + * + * @param message - Error message for display + * @param exitCode - Process exit code (default: 1) + */ +export class CliError extends Error { + readonly exitCode: number; + + constructor(message: string, exitCode = 1) { + super(message); + this.name = "CliError"; + this.exitCode = exitCode; + } + + /** + * Format error for user display. Override in subclasses to add details. + */ + format(): string { + return this.message; + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// API Errors +// ───────────────────────────────────────────────────────────────────────────── + +/** + * API request errors from Sentry. + * + * @param message - Error summary + * @param status - HTTP status code + * @param detail - Detailed error message from API response + * @param endpoint - API endpoint that failed + */ +export class ApiError extends CliError { + readonly status: number; + readonly detail?: string; + readonly endpoint?: string; + + constructor( + message: string, + status: number, + detail?: string, + endpoint?: string + ) { + super(message); + this.name = "ApiError"; + this.status = status; + this.detail = detail; + this.endpoint = endpoint; + } + + override format(): string { + let msg = this.message; + if (this.detail && this.detail !== this.message) { + msg += `\n ${this.detail}`; + } + return msg; + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Authentication Errors +// ───────────────────────────────────────────────────────────────────────────── + +export type AuthErrorReason = "not_authenticated" | "expired" | "invalid"; + +/** + * Authentication errors. + * + * @param reason - Type of auth failure + * @param message - Custom message (uses default if not provided) + */ +export class AuthError extends CliError { + readonly reason: AuthErrorReason; + + constructor(reason: AuthErrorReason, message?: string) { + const defaultMessages: Record = { + not_authenticated: "Not authenticated. Run 'sentry auth login' first.", + expired: + "Authentication expired. Run 'sentry auth login' to re-authenticate.", + invalid: "Invalid authentication token.", + }; + super(message ?? defaultMessages[reason]); + this.name = "AuthError"; + this.reason = reason; + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Configuration Errors +// ───────────────────────────────────────────────────────────────────────────── + +/** + * Configuration or DSN errors. + * + * @param message - Error description + * @param suggestion - Helpful hint for resolving the error + */ +export class ConfigError extends CliError { + readonly suggestion?: string; + + constructor(message: string, suggestion?: string) { + super(message); + this.name = "ConfigError"; + this.suggestion = suggestion; + } + + override format(): string { + let msg = this.message; + if (this.suggestion) { + msg += `\n\nSuggestion: ${this.suggestion}`; + } + return msg; + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Validation Errors +// ───────────────────────────────────────────────────────────────────────────── + +/** + * Input validation errors. + * + * @param message - Validation failure description + * @param field - Name of the invalid field + */ +export class ValidationError extends CliError { + readonly field?: string; + + constructor(message: string, field?: string) { + super(message); + this.name = "ValidationError"; + this.field = field; + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// OAuth Device Flow Errors +// ───────────────────────────────────────────────────────────────────────────── + +/** + * OAuth device flow errors (RFC 8628). + * + * @param code - OAuth error code (e.g., "authorization_pending", "slow_down") + * @param description - Human-readable error description + */ +export class DeviceFlowError extends CliError { + readonly code: string; + + constructor(code: string, description?: string) { + super(description ?? code); + this.name = "DeviceFlowError"; + this.code = code; + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Error Utilities +// ───────────────────────────────────────────────────────────────────────────── + +/** + * Format any error for user display. + * Uses CliError.format() for CLI errors, message for standard errors. + * + * @param error - Any thrown value + * @returns Formatted error string + */ +export function formatError(error: unknown): string { + if (error instanceof CliError) { + return error.format(); + } + if (error instanceof Error) { + return error.message; + } + return String(error); +} + +/** + * Get process exit code for an error. + * + * @param error - Any thrown value + * @returns Exit code (from CliError.exitCode or 1 for other errors) + */ +export function getExitCode(error: unknown): number { + if (error instanceof CliError) { + return error.exitCode; + } + return 1; +} diff --git a/packages/cli/src/lib/oauth.ts b/packages/cli/src/lib/oauth.ts index f00f817ea..f6255975b 100644 --- a/packages/cli/src/lib/oauth.ts +++ b/packages/cli/src/lib/oauth.ts @@ -11,6 +11,7 @@ import type { TokenResponse, } from "../types/index.js"; import { setAuthToken } from "./config.js"; +import { DeviceFlowError } from "./errors.js"; // ───────────────────────────────────────────────────────────────────────────── // Configuration @@ -142,19 +143,6 @@ async function pollForToken(deviceCode: string): Promise { throw new DeviceFlowError(data.error, data.error_description); } -/** - * Custom error class for device flow errors - */ -class DeviceFlowError extends Error { - readonly code: string; - - constructor(code: string, description?: string) { - super(description ?? code); - this.name = "DeviceFlowError"; - this.code = code; - } -} - type PollResult = | { status: "success"; token: TokenResponse } | { status: "pending" }