diff --git a/docs/src/content/docs/commands/api.md b/docs/src/content/docs/commands/api.md index 3fe0aa92..2330669c 100644 --- a/docs/src/content/docs/commands/api.md +++ b/docs/src/content/docs/commands/api.md @@ -85,17 +85,20 @@ sentry api /organizations/ \ --header "X-Custom-Header:value" ``` -### Show Response Headers +### Verbose Mode ```bash -sentry api /organizations/ --include +sentry api /organizations/ --verbose ``` -``` -HTTP/2 200 -content-type: application/json -x-sentry-rate-limit-remaining: 95 +Request and response metadata is logged to stderr: +``` +> GET /api/0/organizations/ +> +< HTTP 200 +< content-type: application/json +< [{"slug": "my-org", ...}] ``` diff --git a/plugins/sentry-cli/skills/sentry-cli/SKILL.md b/plugins/sentry-cli/skills/sentry-cli/SKILL.md index 4c3b4b6d..c8b3d215 100644 --- a/plugins/sentry-cli/skills/sentry-cli/SKILL.md +++ b/plugins/sentry-cli/skills/sentry-cli/SKILL.md @@ -163,6 +163,7 @@ Create a new project **Flags:** - `-t, --team - Team to create the project under` +- `-n, --dry-run - Validate inputs and show what would be created without creating it` - `--json - Output as JSON` - `--fields - Comma-separated fields to include in JSON output (dot.notation supported)` @@ -395,9 +396,11 @@ Make an authenticated API request - `-f, --raw-field ... - Add a string parameter without JSON parsing` - `-H, --header ... - Add a HTTP request header in key:value format` - `--input - The file to use as body for the HTTP request (use "-" to read from standard input)` -- `-i, --include - Include HTTP response status line and headers in the output` - `--silent - Do not print the response body` - `--verbose - Include full HTTP request and response in the output` +- `-n, --dry-run - Show the resolved request without sending it` +- `--json - Output as JSON` +- `--fields - Comma-separated fields to include in JSON output (dot.notation supported)` **Examples:** @@ -436,7 +439,7 @@ sentry api /projects/my-org/my-project/ \ sentry api /organizations/ \ --header "X-Custom-Header:value" -sentry api /organizations/ --include +sentry api /organizations/ --verbose # Get all issues (automatically follows pagination) sentry api /projects/my-org/my-project/issues/ --paginate diff --git a/src/commands/api.ts b/src/commands/api.ts index ecb26171..43867168 100644 --- a/src/commands/api.ts +++ b/src/commands/api.ts @@ -6,10 +6,12 @@ */ import type { SentryContext } from "../context.js"; -import { rawApiRequest } from "../lib/api-client.js"; +import { buildSearchParams, rawApiRequest } from "../lib/api-client.js"; import { buildCommand } from "../lib/command.js"; -import { ValidationError } from "../lib/errors.js"; +import { OutputError, ValidationError } from "../lib/errors.js"; import { validateEndpoint } from "../lib/input-validation.js"; +import { logger } from "../lib/logger.js"; +import { getDefaultSdkConfig } from "../lib/sentry-client.js"; import type { Writer } from "../types/index.js"; type HttpMethod = "GET" | "POST" | "PUT" | "DELETE" | "PATCH"; @@ -21,9 +23,13 @@ type ApiFlags = { readonly "raw-field"?: string[]; readonly header?: string[]; readonly input?: string; - readonly include: boolean; readonly silent: boolean; readonly verbose: boolean; + readonly "dry-run": boolean; + /** Injected by buildCommand via output config */ + readonly json: boolean; + /** Injected by buildCommand via output config */ + readonly fields?: string[]; }; // Request Parsing @@ -842,105 +848,65 @@ export function buildBodyFromFields( // Response Output /** - * Write response headers to stdout (standard format) + * Format a raw response body value for human-readable output. + * Objects are pretty-printed as JSON, strings pass through, null/undefined → empty. * @internal Exported for testing */ -export function writeResponseHeaders( - stdout: Writer, - status: number, - headers: Headers -): void { - stdout.write(`HTTP ${status}\n`); - headers.forEach((value, key) => { - stdout.write(`${key}: ${value}\n`); - }); - stdout.write("\n"); -} - -/** - * Write response body to stdout - * @internal Exported for testing - */ -export function writeResponseBody(stdout: Writer, body: unknown): void { - if (body === null || body === undefined) { - return; +export function formatApiResponse(data: unknown): string { + if (data === null || data === undefined) { + return ""; } - - if (typeof body === "object") { - stdout.write(`${JSON.stringify(body, null, 2)}\n`); - } else { - stdout.write(`${String(body)}\n`); + if (typeof data === "object") { + return JSON.stringify(data, null, 2); } + return String(data); } /** - * Write verbose request output (curl-style format) + * Resolve the full URL that rawApiRequest would use for a request. + * + * Mirrors the URL construction in rawApiRequest: + * `${baseUrl}/api/0/${endpoint}?${queryString}` * @internal Exported for testing */ -export function writeVerboseRequest( - stdout: Writer, - method: string, +export function resolveRequestUrl( endpoint: string, - headers: Record | undefined -): void { - stdout.write(`> ${method} /api/0/${endpoint}\n`); - if (headers) { - for (const [key, value] of Object.entries(headers)) { - stdout.write(`> ${key}: ${value}\n`); - } - } - stdout.write(">\n"); + params?: Record +): string { + // Use getDefaultSdkConfig().baseUrl — same as rawApiRequest — to ensure + // trailing slashes are stripped and the URL matches what would be sent. + const { baseUrl } = getDefaultSdkConfig(); + const normalizedEndpoint = endpoint.startsWith("/") + ? endpoint.slice(1) + : endpoint; + const searchParams = buildSearchParams(params); + const queryString = searchParams ? `?${searchParams.toString()}` : ""; + return `${baseUrl}/api/0/${normalizedEndpoint}${queryString}`; } /** - * Write verbose response output (curl-style format) - * @internal Exported for testing - */ -export function writeVerboseResponse( - stdout: Writer, - status: number, - headers: Headers -): void { - stdout.write(`< HTTP ${status}\n`); - headers.forEach((value, key) => { - stdout.write(`< ${key}: ${value}\n`); - }); - stdout.write("<\n"); -} - -/** - * Handle response output based on flags + * Resolve effective request headers, mirroring rawApiRequest logic. + * + * Auto-adds Content-Type: application/json for non-string object bodies + * when no Content-Type was explicitly provided. + * * @internal Exported for testing */ -export function handleResponse( - stdout: Writer, - response: { status: number; headers: Headers; body: unknown }, - flags: { silent: boolean; verbose: boolean; include: boolean } -): void { - const isError = response.status >= 400; - - // Silent mode - only set exit code - if (flags.silent) { - if (isError) { - process.exit(1); - } - return; - } - - // Output headers (verbose or include mode) - if (flags.verbose) { - writeVerboseResponse(stdout, response.status, response.headers); - } else if (flags.include) { - writeResponseHeaders(stdout, response.status, response.headers); - } - - // Output body - writeResponseBody(stdout, response.body); - - // Exit with error code for error responses - if (isError) { - process.exit(1); +export function resolveEffectiveHeaders( + customHeaders: Record | undefined, + body: unknown +): Record { + // Mirror rawApiRequest exactly: auto-add Content-Type for any non-string, + // non-undefined body when no Content-Type was explicitly provided. + const isStringBody = typeof body === "string"; + const headers = { ...(customHeaders ?? {}) }; + const hasContentType = Object.keys(headers).some( + (k) => k.toLowerCase() === "content-type" + ); + if (!(isStringBody || hasContentType) && body !== undefined) { + headers["Content-Type"] = "application/json"; } + return headers; } /** @@ -1065,7 +1031,34 @@ export async function resolveBody( // Command Definition +const log = logger.withTag("api"); + +/** Log outgoing request details in `> ` curl-verbose style. */ +function logRequest( + method: string, + endpoint: string, + headers: Record | undefined +): void { + log.debug(`> ${method} /api/0/${endpoint}`); + if (headers) { + for (const [key, value] of Object.entries(headers)) { + log.debug(`> ${key}: ${value}`); + } + } + log.debug(">"); +} + +/** Log incoming response details in `< ` curl-verbose style. */ +function logResponse(response: { status: number; headers: Headers }): void { + log.debug(`< HTTP ${response.status}`); + response.headers.forEach((value, key) => { + log.debug(`< ${key}: ${value}`); + }); + log.debug("<"); +} + export const apiCommand = buildCommand({ + output: { json: true, human: formatApiResponse }, docs: { brief: "Make an authenticated API request", fullDescription: @@ -1143,11 +1136,6 @@ export const apiCommand = buildCommand({ optional: true, placeholder: "file", }, - include: { - kind: "boolean", - brief: "Include HTTP response status line and headers in the output", - default: false, - }, silent: { kind: "boolean", brief: "Do not print the response body", @@ -1158,6 +1146,11 @@ export const apiCommand = buildCommand({ brief: "Include full HTTP request and response in the output", default: false, }, + "dry-run": { + kind: "boolean", + brief: "Show the resolved request without sending it", + default: false, + }, }, aliases: { X: "method", @@ -1165,20 +1158,13 @@ export const apiCommand = buildCommand({ F: "field", f: "raw-field", H: "header", - i: "include", + n: "dry-run", }, }, - async func( - this: SentryContext, - flags: ApiFlags, - endpoint: string - ): Promise { - const { stdout, stderr, stdin } = this; - - // Normalize endpoint to ensure trailing slash (Sentry API requirement) - const normalizedEndpoint = normalizeEndpoint(endpoint); + async func(this: SentryContext, flags: ApiFlags, endpoint: string) { + const { stderr, stdin } = this; - // Resolve body and query params from flags (--data, --input, or fields) + const normalizedEndpoint = normalizeEndpoint(endpoint); const { body, params } = await resolveBody(flags, stdin, stderr); const headers = @@ -1186,9 +1172,22 @@ export const apiCommand = buildCommand({ ? parseHeaders(flags.header) : undefined; - // Verbose mode: show request details (unless silent) - if (flags.verbose && !flags.silent) { - writeVerboseRequest(stdout, flags.method, normalizedEndpoint, headers); + // Dry-run mode: preview the request that would be sent + if (flags["dry-run"]) { + return { + data: { + method: flags.method, + url: resolveRequestUrl(normalizedEndpoint, params), + headers: resolveEffectiveHeaders(headers, body), + body: body ?? null, + }, + }; + } + + const verbose = flags.verbose && !flags.silent; + + if (verbose) { + logRequest(flags.method, normalizedEndpoint, headers); } const response = await rawApiRequest(normalizedEndpoint, { @@ -1198,6 +1197,25 @@ export const apiCommand = buildCommand({ headers, }); - handleResponse(stdout, response, flags); + const isError = response.status >= 400; + + if (verbose) { + logResponse(response); + } + + // Silent mode — no output, just exit code + if (flags.silent) { + if (isError) { + throw new OutputError(null); + } + return; + } + + // Always return raw body — --fields filters it directly + if (isError) { + throw new OutputError(response.body); + } + + return { data: response.body }; }, }); diff --git a/src/commands/project/create.ts b/src/commands/project/create.ts index ec7c9dbd..19632ec6 100644 --- a/src/commands/project/create.ts +++ b/src/commands/project/create.ts @@ -60,6 +60,7 @@ const USAGE_HINT = "sentry project create / "; type CreateFlags = { readonly team?: string; + readonly "dry-run": boolean; readonly json: boolean; readonly fields?: string[]; }; @@ -308,8 +309,14 @@ export const createCommand = buildCommand({ brief: "Team to create the project under", optional: true, }, + "dry-run": { + kind: "boolean", + brief: + "Validate inputs and show what would be created without creating it", + default: false, + }, }, - aliases: { t: "team" }, + aliases: { t: "team", n: "dry-run" }, }, async func( this: SentryContext, @@ -379,8 +386,28 @@ export const createCommand = buildCommand({ detectedFrom: resolved.detectedFrom, usageHint: USAGE_HINT, autoCreateSlug: slugify(name), + dryRun: flags["dry-run"], }); + const expectedSlug = slugify(name); + + // Dry-run mode: show what would be created without creating it + if (flags["dry-run"]) { + const result: ProjectCreatedResult = { + project: { id: "", slug: expectedSlug, name, platform }, + orgSlug, + teamSlug: team.slug, + teamSource: team.source, + requestedPlatform: platform, + dsn: null, + url: "", + slugDiverged: false, + expectedSlug, + dryRun: true, + }; + return { data: result }; + } + // Create the project const project = await createProjectWithErrors({ orgSlug, @@ -393,8 +420,6 @@ export const createCommand = buildCommand({ // Fetch DSN (best-effort) const dsn = await tryGetPrimaryDsn(orgSlug, project.slug); - const expectedSlug = slugify(name); - const result: ProjectCreatedResult = { project, orgSlug, diff --git a/src/lib/command.ts b/src/lib/command.ts index cb759019..c4707ba9 100644 --- a/src/lib/command.ts +++ b/src/lib/command.ts @@ -36,6 +36,7 @@ import { buildCommand as stricliCommand, numberParser as stricliNumberParser, } from "@stricli/core"; +import { OutputError } from "./errors.js"; import { parseFieldsList } from "./formatters/json.js"; import { type CommandOutput, @@ -379,19 +380,46 @@ export function buildCommand< setArgsContext(args); } + // OutputError handler: render data through the output system, then + // exit with the error's code. Stricli overwrites process.exitCode = 0 + // after successful returns, so process.exit() is the only way to + // preserve a non-zero code. This lives in the framework — commands + // simply `throw new OutputError(data)`. + const handleOutputError = (err: unknown): never => { + if (err instanceof OutputError && outputConfig) { + // Only render if there's actual data to show + if (err.data !== null && err.data !== undefined) { + handleReturnValue( + this, + { data: err.data } as CommandOutput, + cleanFlags + ); + } + process.exit(err.exitCode); + } + throw err; + }; + // Call original and intercept data returns. // Commands with output config return { data, hint? }; // the wrapper renders automatically. Void returns are ignored. - const result = originalFunc.call( - this, - cleanFlags as FLAGS, - ...(args as unknown as ARGS) - ); + let result: ReturnType; + try { + result = originalFunc.call( + this, + cleanFlags as FLAGS, + ...(args as unknown as ARGS) + ); + } catch (err) { + handleOutputError(err); + } if (result instanceof Promise) { - return result.then((resolved) => { - handleReturnValue(this, resolved, cleanFlags); - }) as ReturnType; + return result + .then((resolved) => { + handleReturnValue(this, resolved, cleanFlags); + }) + .catch(handleOutputError) as ReturnType; } handleReturnValue(this, result, cleanFlags); diff --git a/src/lib/errors.ts b/src/lib/errors.ts index 811bc257..0edc59f1 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -130,6 +130,26 @@ export class ConfigError extends CliError { } } +/** + * Thrown when a command produces valid output but should exit non-zero. + * + * Unlike other errors, the output data is rendered to stdout (not stderr) + * through the normal output system — the `buildCommand` wrapper catches + * this before it reaches the global error handler. Think "HTTP 404 body": + * useful data, but the operation itself failed. + * + * @param data - The output data to render (same type as CommandOutput.data) + */ +export class OutputError extends CliError { + readonly data: unknown; + + constructor(data: unknown) { + super("", 1); + this.name = "OutputError"; + this.data = data; + } +} + const DEFAULT_CONTEXT_ALTERNATIVES = [ "Run from a directory with a Sentry-configured project", "Set SENTRY_ORG and SENTRY_PROJECT (or SENTRY_DSN) environment variables", diff --git a/src/lib/formatters/human.ts b/src/lib/formatters/human.ts index 4979c10b..910084c8 100644 --- a/src/lib/formatters/human.ts +++ b/src/lib/formatters/human.ts @@ -1631,6 +1631,8 @@ export type ProjectCreatedResult = { slugDiverged: boolean; /** The slug the user expected (derived from the project name) */ expectedSlug: string; + /** When true, nothing was actually created — output uses tentative wording */ + dryRun?: boolean; }; /** @@ -1644,13 +1646,19 @@ export type ProjectCreatedResult = { */ export function formatProjectCreated(result: ProjectCreatedResult): string { const lines: string[] = []; + const dry = result.dryRun === true; + const nameEsc = escapeMarkdownInline(result.project.name); + const orgEsc = escapeMarkdownInline(result.orgSlug); - lines.push( - `## Created project '${escapeMarkdownInline(result.project.name)}' in ${escapeMarkdownInline(result.orgSlug)}` - ); + // Heading + if (dry) { + lines.push(`## Dry run — project '${nameEsc}' in ${orgEsc}`); + } else { + lines.push(`## Created project '${nameEsc}' in ${orgEsc}`); + } lines.push(""); - // Slug divergence note + // Slug divergence note (never applies in dry-run — we can't predict server renames) if (result.slugDiverged) { lines.push( `> **Note:** Slug \`${result.project.slug}\` was assigned because \`${result.expectedSlug}\` is already taken.` @@ -1658,21 +1666,25 @@ export function formatProjectCreated(result: ProjectCreatedResult): string { lines.push(""); } - // Team source notes + // Team source notes — tentative wording in dry-run if (result.teamSource === "auto-created") { lines.push( - `> **Note:** Created team '${escapeMarkdownInline(result.teamSlug)}' (org had no teams).` + dry + ? `> **Note:** Would create team '${escapeMarkdownInline(result.teamSlug)}' (org has no teams).` + : `> **Note:** Created team '${escapeMarkdownInline(result.teamSlug)}' (org had no teams).` ); lines.push(""); } else if (result.teamSource === "auto-selected") { lines.push( - `> **Note:** Using team '${escapeMarkdownInline(result.teamSlug)}'. See all teams: \`sentry team list\`` + dry + ? `> **Note:** Would use team '${escapeMarkdownInline(result.teamSlug)}'. See all teams: \`sentry team list\`` + : `> **Note:** Using team '${escapeMarkdownInline(result.teamSlug)}'. See all teams: \`sentry team list\`` ); lines.push(""); } const kvRows: [string, string][] = [ - ["Project", escapeMarkdownInline(result.project.name)], + ["Project", nameEsc], ["Slug", safeCodeSpan(result.project.slug)], ["Org", safeCodeSpan(result.orgSlug)], ["Team", safeCodeSpan(result.teamSlug)], @@ -1681,13 +1693,19 @@ export function formatProjectCreated(result: ProjectCreatedResult): string { if (result.dsn) { kvRows.push(["DSN", safeCodeSpan(result.dsn)]); } - kvRows.push(["URL", result.url]); + if (result.url) { + kvRows.push(["URL", result.url]); + } lines.push(mdKvTable(kvRows)); - lines.push(""); - lines.push( - `*Tip: Use \`sentry project view ${result.orgSlug}/${result.project.slug}\` for details*` - ); + + // Tip footer — only when a real project exists to view + if (!dry) { + lines.push(""); + lines.push( + `*Tip: Use \`sentry project view ${result.orgSlug}/${result.project.slug}\` for details*` + ); + } return renderMarkdown(lines.join("\n")); } diff --git a/src/lib/logger.ts b/src/lib/logger.ts index c6b60bca..ec936d52 100644 --- a/src/lib/logger.ts +++ b/src/lib/logger.ts @@ -154,11 +154,11 @@ const scopedLoggers: ConsolaInstance[] = []; */ export const logger = createConsola({ level: DEFAULT_LOG_LEVEL, - // stderr is the correct stream for diagnostic/log output in CLIs — - // stdout is reserved for command output (data, JSON, tables). + // All diagnostic/log output goes to stderr — stdout is reserved for + // command output (data, JSON, tables). Both streams must be set because + // consola's BasicReporter (non-TTY) routes debug/info to stdout by default. + stdout: process.stderr, stderr: process.stderr, - // FancyReporter is included by default for TTY, BasicReporter for CI/non-TTY. - // Sentry reporter is added after Sentry.init() via attachSentryReporter(). }); /** diff --git a/src/lib/resolve-team.ts b/src/lib/resolve-team.ts index 52830623..ab4b4e2d 100644 --- a/src/lib/resolve-team.ts +++ b/src/lib/resolve-team.ts @@ -65,6 +65,12 @@ export type ResolveTeamOptions = { * If not provided and the org has zero teams, an error is thrown instead. */ autoCreateSlug?: string; + /** + * When true, skip the actual team creation API call and return what + * would be created. The returned ResolvedTeam has source "auto-created" + * with the autoCreateSlug value. + */ + dryRun?: boolean; }; /** Result of team resolution, including how the team was determined */ @@ -116,14 +122,7 @@ export async function resolveOrCreateTeam( // No teams — auto-create one if a slug was provided if (teams.length === 0) { - if (options.autoCreateSlug) { - return await autoCreateTeam(orgSlug, options.autoCreateSlug); - } - const teamsUrl = `${getSentryBaseUrl()}/settings/${orgSlug}/teams/`; - throw new ContextError("Team", `${options.usageHint} --team `, [ - `No teams found in org '${orgSlug}'`, - `Create a team at ${teamsUrl}`, - ]); + return resolveEmptyTeams(orgSlug, options); } // Single team — auto-select @@ -155,6 +154,27 @@ export async function resolveOrCreateTeam( ); } +/** + * Handle the case when an org has zero teams. + * Either auto-creates a team, returns a dry-run preview, or throws. + */ +function resolveEmptyTeams( + orgSlug: string, + options: ResolveTeamOptions +): Promise | ResolvedTeam { + if (!options.autoCreateSlug) { + const teamsUrl = `${getSentryBaseUrl()}/settings/${orgSlug}/teams/`; + throw new ContextError("Team", `${options.usageHint} --team `, [ + `No teams found in org '${orgSlug}'`, + `Create a team at ${teamsUrl}`, + ]); + } + if (options.dryRun) { + return { slug: options.autoCreateSlug, source: "auto-created" }; + } + return autoCreateTeam(orgSlug, options.autoCreateSlug); +} + /** * Auto-create a team in an org that has no teams. * Uses the provided slug as the team name. diff --git a/test/commands/api.property.test.ts b/test/commands/api.property.test.ts index d15f83fa..135ed63a 100644 --- a/test/commands/api.property.test.ts +++ b/test/commands/api.property.test.ts @@ -16,6 +16,7 @@ import { oneof, property, record, + string, stringMatching, tuple, uniqueArray, @@ -30,6 +31,8 @@ import { parseFieldValue, parseMethod, resolveBody, + resolveEffectiveHeaders, + resolveRequestUrl, setNestedValue, } from "../../src/commands/api.js"; import { ValidationError } from "../../src/lib/errors.js"; @@ -788,3 +791,130 @@ describe("property: resolveBody", () => { ); }); }); + +// Dry-run property tests + +/** Arbitrary for HTTP methods */ + +/** Arbitrary for clean API endpoint paths (no query string, for dry-run tests) */ +const dryRunEndpointArb = stringMatching( + /^[a-z][a-z0-9-]*\/[a-z0-9-]*\/$/ +).filter((s) => s.length > 3 && s.length < 80); + +/** Arbitrary for query param values */ +const paramValueArb = stringMatching(/^[a-zA-Z0-9_-]+$/).filter( + (s) => s.length > 0 && s.length < 40 +); + +/** Arbitrary for query param maps */ +const paramsArb = dictionary( + stringMatching(/^[a-zA-Z][a-zA-Z0-9_]*$/).filter( + (s) => s.length > 0 && s.length < 20 + ), + paramValueArb, + { minKeys: 0, maxKeys: 3 } +); + +describe("property: resolveRequestUrl", () => { + test("always contains /api/0/ prefix", () => { + fcAssert( + property(dryRunEndpointArb, (endpoint) => { + const url = resolveRequestUrl(endpoint); + expect(url).toContain("/api/0/"); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("always contains the endpoint path", () => { + fcAssert( + property(dryRunEndpointArb, (endpoint) => { + const url = resolveRequestUrl(endpoint); + const normalized = endpoint.startsWith("/") + ? endpoint.slice(1) + : endpoint; + expect(url).toContain(normalized); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("includes query string when params provided", () => { + fcAssert( + property(dryRunEndpointArb, paramsArb, (endpoint, params) => { + const url = resolveRequestUrl(endpoint, params); + if (Object.keys(params).length > 0) { + expect(url).toContain("?"); + for (const key of Object.keys(params)) { + expect(url).toContain(key); + } + } + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); +}); + +describe("property: resolveEffectiveHeaders", () => { + /** Arbitrary for custom header entries (non-content-type keys) */ + const headerKeyArb = stringMatching(/^X-[A-Za-z]{1,10}$/); + const headerValueArb = string({ minLength: 1, maxLength: 20 }); + const customHeadersArb = dictionary(headerKeyArb, headerValueArb, { + minKeys: 0, + maxKeys: 3, + }); + + test("preserves all custom headers", () => { + fcAssert( + property(customHeadersArb, (custom) => { + const result = resolveEffectiveHeaders(custom, undefined); + for (const [key, value] of Object.entries(custom)) { + expect(result[key]).toBe(value); + } + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("auto-adds Content-Type for object bodies without it", () => { + fcAssert( + property(customHeadersArb, jsonValue(), (custom, body) => { + // Ensure body is a non-null object (not string/number/boolean/null) + if (body === null || body === undefined || typeof body !== "object") { + return; + } + const result = resolveEffectiveHeaders(custom, body); + expect(result["Content-Type"]).toBe("application/json"); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("never adds Content-Type for string bodies", () => { + fcAssert( + property(customHeadersArb, string(), (custom, body) => { + const result = resolveEffectiveHeaders(custom, body); + // Should not have auto-added Content-Type (only custom headers) + if (!custom["Content-Type"]) { + expect(result["Content-Type"]).toBeUndefined(); + } + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); + + test("does not override explicit Content-Type", () => { + const contentTypeArb = constantFrom( + "text/plain", + "text/xml", + "application/x-www-form-urlencoded" + ); + fcAssert( + property(contentTypeArb, jsonValue(), (ct, body) => { + const result = resolveEffectiveHeaders({ "Content-Type": ct }, body); + expect(result["Content-Type"]).toBe(ct); + }), + { numRuns: DEFAULT_NUM_RUNS } + ); + }); +}); diff --git a/test/commands/api.test.ts b/test/commands/api.test.ts index c27a399c..455d3fba 100644 --- a/test/commands/api.test.ts +++ b/test/commands/api.test.ts @@ -16,7 +16,7 @@ import { buildRawQueryParams, dataToQueryParams, extractJsonBody, - handleResponse, + formatApiResponse, normalizeEndpoint, normalizeFields, parseDataBody, @@ -26,11 +26,9 @@ import { prepareRequestOptions, readStdin, resolveBody, + resolveEffectiveHeaders, + resolveRequestUrl, setNestedValue, - writeResponseBody, - writeResponseHeaders, - writeVerboseRequest, - writeVerboseResponse, } from "../../src/commands/api.js"; import { ValidationError } from "../../src/lib/errors.js"; import type { Writer } from "../../src/types/index.js"; @@ -877,163 +875,37 @@ describe("buildBodyFromFields", () => { }); }); -describe("writeResponseHeaders", () => { - test("writes status and headers", () => { - const writer = createMockWriter(); - const headers = new Headers({ - "Content-Type": "application/json", - "X-Custom": "value", - }); - - writeResponseHeaders(writer, 200, headers); - - expect(writer.output).toMatch(/^HTTP 200\n/); - expect(writer.output).toMatch(/content-type: application\/json/i); - expect(writer.output).toMatch(/x-custom: value/i); - expect(writer.output).toMatch(/\n$/); - }); - - test("handles different status codes", () => { - const writer = createMockWriter(); - const headers = new Headers(); - - writeResponseHeaders(writer, 404, headers); - - expect(writer.output).toMatch(/^HTTP 404\n/); - }); - - test("handles empty headers", () => { - const writer = createMockWriter(); - const headers = new Headers(); - - writeResponseHeaders(writer, 200, headers); - - expect(writer.output).toBe("HTTP 200\n\n"); - }); -}); - -describe("writeResponseBody", () => { - test("writes JSON object with formatting", () => { - const writer = createMockWriter(); - - writeResponseBody(writer, { key: "value", num: 42 }); - - expect(writer.output).toBe('{\n "key": "value",\n "num": 42\n}\n'); - }); - - test("writes JSON array with formatting", () => { - const writer = createMockWriter(); - - writeResponseBody(writer, [1, 2, 3]); - - expect(writer.output).toBe("[\n 1,\n 2,\n 3\n]\n"); - }); - - test("writes string directly", () => { - const writer = createMockWriter(); - - writeResponseBody(writer, "plain text response"); - - expect(writer.output).toBe("plain text response\n"); - }); - - test("writes number as string", () => { - const writer = createMockWriter(); - - writeResponseBody(writer, 42); - - expect(writer.output).toBe("42\n"); +describe("formatApiResponse", () => { + test("formats JSON object with pretty-printing", () => { + expect(formatApiResponse({ key: "value", num: 42 })).toBe( + '{\n "key": "value",\n "num": 42\n}' + ); }); - test("writes boolean as string", () => { - const writer = createMockWriter(); - - writeResponseBody(writer, true); - - expect(writer.output).toBe("true\n"); + test("formats JSON array with pretty-printing", () => { + expect(formatApiResponse([1, 2, 3])).toBe("[\n 1,\n 2,\n 3\n]"); }); - test("does not write null", () => { - const writer = createMockWriter(); - - writeResponseBody(writer, null); - - expect(writer.output).toBe(""); + test("formats string directly without JSON quoting", () => { + expect(formatApiResponse("plain text response")).toBe( + "plain text response" + ); }); - test("does not write undefined", () => { - const writer = createMockWriter(); - - writeResponseBody(writer, undefined); - - expect(writer.output).toBe(""); + test("formats number as string", () => { + expect(formatApiResponse(42)).toBe("42"); }); -}); - -describe("writeVerboseRequest", () => { - test("writes method and endpoint", () => { - const writer = createMockWriter(); - writeVerboseRequest(writer, "GET", "organizations/", undefined); - - expect(writer.output).toBe("> GET /api/0/organizations/\n>\n"); + test("formats boolean as string", () => { + expect(formatApiResponse(true)).toBe("true"); }); - test("writes headers when provided", () => { - const writer = createMockWriter(); - - writeVerboseRequest(writer, "POST", "issues/", { - "Content-Type": "application/json", - "X-Custom": "value", - }); - - expect(writer.output).toMatch(/^> POST \/api\/0\/issues\/\n/); - expect(writer.output).toMatch(/> Content-Type: application\/json\n/); - expect(writer.output).toMatch(/> X-Custom: value\n/); - expect(writer.output).toMatch(/>\n$/); + test("returns empty string for null", () => { + expect(formatApiResponse(null)).toBe(""); }); - test("handles empty headers object", () => { - const writer = createMockWriter(); - - writeVerboseRequest(writer, "DELETE", "issues/123/", {}); - - expect(writer.output).toBe("> DELETE /api/0/issues/123/\n>\n"); - }); -}); - -describe("writeVerboseResponse", () => { - test("writes status and headers with < prefix", () => { - const writer = createMockWriter(); - const headers = new Headers({ - "Content-Type": "application/json", - "X-Request-Id": "abc123", - }); - - writeVerboseResponse(writer, 200, headers); - - expect(writer.output).toMatch(/^< HTTP 200\n/); - expect(writer.output).toMatch(/< content-type: application\/json/i); - expect(writer.output).toMatch(/< x-request-id: abc123/i); - expect(writer.output).toMatch(/<\n$/); - }); - - test("handles error status codes", () => { - const writer = createMockWriter(); - const headers = new Headers(); - - writeVerboseResponse(writer, 500, headers); - - expect(writer.output).toMatch(/^< HTTP 500\n/); - }); - - test("handles empty headers", () => { - const writer = createMockWriter(); - const headers = new Headers(); - - writeVerboseResponse(writer, 204, headers); - - expect(writer.output).toBe("< HTTP 204\n<\n"); + test("returns empty string for undefined", () => { + expect(formatApiResponse(undefined)).toBe(""); }); }); @@ -1128,154 +1000,56 @@ describe("buildBodyFromInput", () => { }); }); -describe("handleResponse", () => { - // Mock process.exit for tests - const originalExit = process.exit; - - test("outputs body for successful response", () => { - const writer = createMockWriter(); - const response = { - status: 200, - headers: new Headers(), - body: { success: true }, - }; - - handleResponse(writer, response, { - silent: false, - verbose: false, - include: false, - }); - - expect(writer.output).toContain('"success": true'); +describe("resolveEffectiveHeaders", () => { + test("auto-adds Content-Type for object bodies", () => { + const headers = resolveEffectiveHeaders(undefined, { key: "value" }); + expect(headers["Content-Type"]).toBe("application/json"); }); - test("outputs headers with --include flag", () => { - const writer = createMockWriter(); - const response = { - status: 200, - headers: new Headers({ "Content-Type": "application/json" }), - body: { data: "test" }, - }; - - handleResponse(writer, response, { - silent: false, - verbose: false, - include: true, - }); - - expect(writer.output).toMatch(/^HTTP 200\n/); - expect(writer.output).toMatch(/content-type:/i); + test("does not add Content-Type for string bodies", () => { + const headers = resolveEffectiveHeaders(undefined, "raw-string"); + expect(headers["Content-Type"]).toBeUndefined(); }); - test("outputs verbose format with --verbose flag", () => { - const writer = createMockWriter(); - const response = { - status: 200, - headers: new Headers({ "Content-Type": "application/json" }), - body: { data: "test" }, - }; - - handleResponse(writer, response, { - silent: false, - verbose: true, - include: false, - }); - - expect(writer.output).toMatch(/^< HTTP 200\n/); - expect(writer.output).toMatch(/< content-type:/i); + test("does not override explicit Content-Type", () => { + const headers = resolveEffectiveHeaders( + { "Content-Type": "text/plain" }, + { key: "value" } + ); + expect(headers["Content-Type"]).toBe("text/plain"); }); - test("verbose takes precedence over include", () => { - const writer = createMockWriter(); - const response = { - status: 200, - headers: new Headers(), - body: "test", - }; - - handleResponse(writer, response, { - silent: false, - verbose: true, - include: true, - }); - - // Should use verbose format (< prefix), not include format - expect(writer.output).toMatch(/^< HTTP/); + test("case-insensitive Content-Type check", () => { + const headers = resolveEffectiveHeaders( + { "content-type": "text/xml" }, + { key: "value" } + ); + expect(headers["content-type"]).toBe("text/xml"); + expect(headers["Content-Type"]).toBeUndefined(); }); - test("silent mode produces no output for success", () => { - const writer = createMockWriter(); - const response = { - status: 200, - headers: new Headers(), - body: { data: "test" }, - }; - - handleResponse(writer, response, { - silent: true, - verbose: false, - include: false, - }); - - expect(writer.output).toBe(""); + test("preserves custom headers", () => { + const headers = resolveEffectiveHeaders( + { Authorization: "Bearer token", "X-Custom": "value" }, + undefined + ); + expect(headers.Authorization).toBe("Bearer token"); + expect(headers["X-Custom"]).toBe("value"); }); - test("silent mode with error calls process.exit(1)", () => { - const writer = createMockWriter(); - const response = { - status: 500, - headers: new Headers(), - body: { error: "Internal Server Error" }, - }; - - let exitCode: number | undefined; - process.exit = ((code?: number) => { - exitCode = code; - throw new Error("process.exit called"); - }) as typeof process.exit; - - try { - expect(() => - handleResponse(writer, response, { - silent: true, - verbose: false, - include: false, - }) - ).toThrow("process.exit called"); - expect(exitCode).toBe(1); - } finally { - process.exit = originalExit; - } + test("defaults to empty object when no headers provided", () => { + const headers = resolveEffectiveHeaders(undefined, undefined); + expect(headers).toEqual({}); }); - test("error response calls process.exit(1) after output", () => { - const writer = createMockWriter(); - const response = { - status: 404, - headers: new Headers(), - body: { detail: "Not found" }, - }; - - let exitCode: number | undefined; - process.exit = ((code?: number) => { - exitCode = code; - throw new Error("process.exit called"); - }) as typeof process.exit; + test("adds Content-Type for null body (matches rawApiRequest)", () => { + const headers = resolveEffectiveHeaders(undefined, null); + expect(headers["Content-Type"]).toBe("application/json"); + }); - try { - expect(() => - handleResponse(writer, response, { - silent: false, - verbose: false, - include: false, - }) - ).toThrow("process.exit called"); - expect(exitCode).toBe(1); - // Should have output the body before exiting - expect(writer.output).toContain("Not found"); - } finally { - process.exit = originalExit; - } + test("does not add Content-Type for undefined body", () => { + const headers = resolveEffectiveHeaders(undefined, undefined); + expect(headers["Content-Type"]).toBeUndefined(); }); }); @@ -1730,3 +1504,36 @@ describe("dataToQueryParams", () => { ).toThrow(ValidationError); }); }); + +// Dry-run tests + +describe("resolveRequestUrl", () => { + test("builds URL with base URL and endpoint", () => { + const url = resolveRequestUrl("organizations/"); + expect(url).toMatch(/\/api\/0\/organizations\/$/); + }); + + test("strips leading slash from endpoint", () => { + const url = resolveRequestUrl("/organizations/"); + expect(url).toMatch(/\/api\/0\/organizations\/$/); + }); + + test("appends query params", () => { + const url = resolveRequestUrl("issues/", { status: "unresolved" }); + expect(url).toContain("?status=unresolved"); + expect(url).toContain("/api/0/issues/"); + }); + + test("handles array params", () => { + const url = resolveRequestUrl("events/", { + field: ["title", "timestamp"], + }); + expect(url).toContain("field=title"); + expect(url).toContain("field=timestamp"); + }); + + test("omits query string when no params", () => { + const url = resolveRequestUrl("projects/"); + expect(url).not.toContain("?"); + }); +}); diff --git a/test/commands/project/create.test.ts b/test/commands/project/create.test.ts index fb4b27d7..2671e26f 100644 --- a/test/commands/project/create.test.ts +++ b/test/commands/project/create.test.ts @@ -626,4 +626,119 @@ describe("project create", () => { expect(err).toBeInstanceOf(CliError); expect(err.message).toContain("Invalid platform 'python-django-rest'"); }); + + // --dry-run tests + + test("dry-run shows what would be created without API call", async () => { + const { context, stdoutWrite } = createMockContext(); + const func = await createCommand.loader(); + await func.call( + context, + { json: false, "dry-run": true }, + "my-app", + "node" + ); + + // Should NOT call createProject + expect(createProjectSpy).not.toHaveBeenCalled(); + // Should NOT fetch DSN + expect(tryGetPrimaryDsnSpy).not.toHaveBeenCalled(); + + const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); + expect(output).toContain("Dry run"); + expect(output).toContain("acme-corp"); + expect(output).toContain("engineering"); + expect(output).toContain("my-app"); + expect(output).toContain("node"); + }); + + test("dry-run still validates platform", async () => { + const { context } = createMockContext(); + const func = await createCommand.loader(); + + const err = await func + .call( + context, + { json: false, "dry-run": true }, + "my-app", + "invalid-platform" + ) + .catch((e: Error) => e); + expect(err).toBeInstanceOf(CliError); + expect(err.message).toContain("Invalid platform"); + }); + + test("dry-run still resolves org", async () => { + const { context } = createMockContext(); + const func = await createCommand.loader(); + await func.call( + context, + { json: false, "dry-run": true }, + "my-org/my-app", + "python" + ); + + expect(resolveOrgSpy).toHaveBeenCalledWith({ + org: "my-org", + cwd: "/tmp", + }); + }); + + test("dry-run outputs JSON when --json is set", async () => { + const { context, stdoutWrite } = createMockContext(); + const func = await createCommand.loader(); + await func.call(context, { json: true, "dry-run": true }, "my-app", "node"); + + const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); + const parsed = JSON.parse(output); + // Same ProjectCreatedResult shape as normal path + expect(parsed.orgSlug).toBe("acme-corp"); + expect(parsed.teamSlug).toBe("engineering"); + expect(parsed.project.name).toBe("my-app"); + expect(parsed.project.slug).toBe("my-app"); + expect(parsed.project.platform).toBe("node"); + expect(parsed.dsn).toBeNull(); + expect(parsed.dryRun).toBe(true); + + // Should NOT call createProject + expect(createProjectSpy).not.toHaveBeenCalled(); + }); + + test("dry-run shows team source for auto-selected teams", async () => { + const { context, stdoutWrite } = createMockContext(); + const func = await createCommand.loader(); + await func.call( + context, + { json: false, "dry-run": true }, + "my-app", + "node" + ); + + const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); + // Single team = auto-selected → note about team usage + expect(output).toContain("Would use team"); + }); + + test("dry-run with no teams shows auto-created team without creating it", async () => { + listTeamsSpy.mockResolvedValue([]); + + const { context, stdoutWrite } = createMockContext(); + const func = await createCommand.loader(); + await func.call( + context, + { json: false, "dry-run": true }, + "my-app", + "node" + ); + + // Should NOT call createTeam + expect(createTeamSpy).not.toHaveBeenCalled(); + // Should NOT call createProject + expect(createProjectSpy).not.toHaveBeenCalled(); + + const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); + expect(output).toContain("Dry run"); + expect(output).toContain("my-app"); + expect(output).toContain("Would create team"); + }); }); diff --git a/test/e2e/api.test.ts b/test/e2e/api.test.ts index 9c30bae5..5cc43aff 100644 --- a/test/e2e/api.test.ts +++ b/test/e2e/api.test.ts @@ -66,21 +66,6 @@ describe("sentry api", () => { { timeout: 15_000 } ); - test( - "--include flag shows response headers", - async () => { - await ctx.setAuthToken(TEST_TOKEN); - - const result = await ctx.run(["api", "organizations/", "--include"]); - - expect(result.exitCode).toBe(0); - // Should include HTTP status and headers before JSON body - expect(result.stdout).toMatch(/^HTTP \d{3}/); - expect(result.stdout).toMatch(/content-type:/i); - }, - { timeout: 15_000 } - ); - test( "invalid endpoint returns non-zero exit code", async () => { @@ -179,19 +164,6 @@ describe("sentry api", () => { { timeout: 15_000 } ); - test( - "-i alias for --include works", - async () => { - await ctx.setAuthToken(TEST_TOKEN); - - const result = await ctx.run(["api", "organizations/", "-i"]); - - expect(result.exitCode).toBe(0); - expect(result.stdout).toMatch(/^HTTP \d{3}/); - }, - { timeout: 15_000 } - ); - test( "-H alias for --header works", async () => { @@ -225,12 +197,14 @@ describe("sentry api", () => { const result = await ctx.run(["api", "organizations/", "--verbose"]); expect(result.exitCode).toBe(0); - // Should show request line with > prefix - expect(result.stdout).toMatch(/^> GET \/api\/0\/organizations\//m); - // Should show response status with < prefix - expect(result.stdout).toMatch(/^< HTTP \d{3}/m); - // Should show response headers with < prefix - expect(result.stdout).toMatch(/^< content-type:/im); + // Verbose output goes to stderr via logger.debug() + // consola formats as: [debug] [api] > GET /api/0/organizations/ + expect(result.stderr).toMatch(/> GET \/api\/0\/organizations\//); + expect(result.stderr).toMatch(/< HTTP \d{3}/); + expect(result.stderr).toMatch(/< content-type:/i); + // stdout should still contain the response body + const data = JSON.parse(result.stdout); + expect(Array.isArray(data)).toBe(true); }, { timeout: 15_000 } ); diff --git a/test/lib/command.test.ts b/test/lib/command.test.ts index 80e0a151..df689020 100644 --- a/test/lib/command.test.ts +++ b/test/lib/command.test.ts @@ -24,6 +24,7 @@ import { numberParser, VERBOSE_FLAG, } from "../../src/lib/command.js"; +import { OutputError } from "../../src/lib/errors.js"; import { LOG_LEVEL_NAMES, logger, setLogLevel } from "../../src/lib/logger.js"; /** Minimal context for test commands */ @@ -42,10 +43,11 @@ type TestContext = CommandContext & { * Access collected output via `ctx.output`. */ function createTestContext() { - const collected: string[] = []; + const stdoutCollected: string[] = []; + const stderrCollected: string[] = []; const stdoutWriter = { write: (s: string) => { - collected.push(s); + stdoutCollected.push(s); return true; }, }; @@ -54,15 +56,17 @@ function createTestContext() { stdout: stdoutWriter, stderr: { write: (s: string) => { - collected.push(s); + stderrCollected.push(s); return true; }, }, }, /** stdout on context — used by buildCommand's return-based output handler */ stdout: stdoutWriter, - /** All collected output chunks */ - output: collected, + /** stdout output chunks only */ + output: stdoutCollected, + /** stderr output chunks only */ + errors: stderrCollected, }; } @@ -804,7 +808,7 @@ describe("buildCommand output: json", () => { expect(funcCalled).toBe(false); expect( - ctx.output.some((s) => s.includes("No flag registered for --json")) + ctx.errors.some((s) => s.includes("No flag registered for --json")) ).toBe(true); }); @@ -1236,4 +1240,104 @@ describe("buildCommand return-based output", () => { expect(jsonRaw).not.toContain("Detected from"); expect(JSON.parse(jsonRaw)).toEqual({ org: "sentry" }); }); + + test("OutputError renders data and exits with error code", async () => { + let exitCalledWith: number | undefined; + const originalExit = process.exit; + + const command = buildCommand< + { json: boolean; fields?: string[] }, + [], + TestContext + >({ + docs: { brief: "Test" }, + output: { + json: true, + human: (d: { error: string }) => `Error: ${d.error}`, + }, + parameters: {}, + async func(this: TestContext) { + throw new OutputError({ error: "not found" }); + }, + }); + + const routeMap = buildRouteMap({ + routes: { test: command }, + docs: { brief: "Test app" }, + }); + const app = buildApplication(routeMap, { name: "test" }); + const ctx = createTestContext(); + + // Mock process.exit — must throw to prevent fall-through since + // the real process.exit() is typed as `never` + class MockExit extends Error { + code: number; + constructor(code: number) { + super(`process.exit(${code})`); + this.code = code; + } + } + process.exit = ((code?: number) => { + exitCalledWith = code; + throw new MockExit(code ?? 0); + }) as typeof process.exit; + + try { + await run(app, ["test"], ctx as TestContext); + } finally { + process.exit = originalExit; + } + expect(exitCalledWith).toBe(1); + // Output was rendered BEFORE exit + expect(ctx.output.join("")).toContain("Error: not found"); + }); + + test("OutputError renders JSON in --json mode", async () => { + let exitCalledWith: number | undefined; + const originalExit = process.exit; + + const command = buildCommand< + { json: boolean; fields?: string[] }, + [], + TestContext + >({ + docs: { brief: "Test" }, + output: { + json: true, + human: (d: { error: string }) => `Error: ${d.error}`, + }, + parameters: {}, + async func(this: TestContext) { + throw new OutputError({ error: "not found" }); + }, + }); + + const routeMap = buildRouteMap({ + routes: { test: command }, + docs: { brief: "Test app" }, + }); + const app = buildApplication(routeMap, { name: "test" }); + const ctx = createTestContext(); + + class MockExit extends Error { + code: number; + constructor(code: number) { + super(`process.exit(${code})`); + this.code = code; + } + } + process.exit = ((code?: number) => { + exitCalledWith = code; + throw new MockExit(code ?? 0); + }) as typeof process.exit; + + try { + await run(app, ["test", "--json"], ctx as TestContext); + } finally { + process.exit = originalExit; + } + expect(exitCalledWith).toBe(1); + const jsonOutput = JSON.parse(ctx.output.join("")); + expect(jsonOutput).toEqual({ error: "not found" }); + }); });