diff --git a/packages/cli/README.md b/packages/cli/README.md index 07df7f7..feb416b 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -57,11 +57,11 @@ List commands return 20 results by default. Use `--limit` to return from 1 to 50 ## Scripts and agents -Add `--json` to any command for machine-readable output on stdout. Messages and progress go to stderr, so `telemetry traces --json > traces.json` writes only data. List commands print the MCP tool's result as returned, with the fields that the [MCP server documentation](https://telemetry.dev/docs/platform/mcp-server) describes. +Add `--json` to any command except `login` and `logout` for machine-readable output on stdout. Messages and progress go to stderr, so `telemetry traces --json > traces.json` writes only data. List commands print the MCP tool's result as returned, with the fields that the [MCP server documentation](https://telemetry.dev/docs/platform/mcp-server) describes. `telemetry trace` and `telemetry payload` read every page of a trace or a captured text, up to a limit: 2,000 spans for a trace, and 1,000,000 characters for each payload field. Their JSON output has `complete: false` when the limit cut it short, and human output says so on stderr. -Captured text comes from your users, models, and tools. Human-readable output, including `payload --field`, removes terminal escape sequences and control characters from that text. JSON output escapes them, so use `--json` when you need the exact text. +Captured text comes from your users, models, and tools. telemetry.dev removes ANSI color and cursor sequences, bidirectional overrides, and Unicode tag characters from results before they reach the CLI; it does not clean the text of error messages. Human-readable output, including `payload --field`, removes the remaining escape sequences and control characters. JSON output escapes them instead of removing them, so use `--json` when you need everything the server sent. | Exit code | Meaning | | --------- | --------------------------------------------------------------- | @@ -71,6 +71,33 @@ Captured text comes from your users, models, and tools. Human-readable output, i | 4 | You are not signed in, the sign-in expired, or access is denied | | 130 | Interrupted with Ctrl-C | +With `--json`, an error is one line of JSON on stderr, and stdout stays empty. Ctrl-C exits with 130 and no JSON, and `status --json` reports a missing sign-in as data on stdout with exit code 4. + +```text +{"error":{"code":"not_signed_in","message":"You're not signed in. Run `telemetry login` first.","hint":"telemetry login","retryable":false,"exit_code":4,"cli_version":"0.1.0"}} +``` + +Branch on `code`, not on `message`, whose wording can change. `hint` is a command to run next, when there is one; `telemetry login` needs a person at a browser, so an agent should ask for it rather than run it. `retryable` is true when running the same command again may succeed. New codes may be added, and existing codes keep their meaning. + +| Code | Exit | Meaning | +| ---------------------- | ---- | -------------------------------------------------------------------------------- | +| `invalid_arguments` | 2 | An option or argument is not valid | +| `unknown_command` | 2 | There is no command with that name | +| `project_not_found` | 2 | No project in the workspace has that name or id | +| `project_ambiguous` | 2 | Several projects have that name; pass its id | +| `project_required` | 2 | The workspace has several projects and none was chosen | +| `no_projects` | 1 | The workspace has no projects yet | +| `not_signed_in` | 4 | No sign-in is saved for this deployment | +| `sign_in_expired` | 4 | The sign-in expired or was disconnected | +| `payloads_not_allowed` | 4 | The sign-in doesn't include reading captured prompts and outputs | +| `unreachable` | 1 | telemetry.dev could not be reached | +| `timeout` | 1 | telemetry.dev took too long to answer | +| `tool_error` | 1 | telemetry.dev returned an error; `message` says what it was | +| `result_too_large` | 1 | The result was too large to read; narrow it with filters or `--limit` | +| `request_failed` | 1 | The request failed for another reason | +| `failed` | 1 | The command failed for another reason | +| `internal` | 1 | An unexpected error, such as a configuration file the CLI couldn't read or write | + ## Configuration | Variable | Default | Purpose | diff --git a/packages/cli/src/api.ts b/packages/cli/src/api.ts index b9ee36d..890ff8d 100644 --- a/packages/cli/src/api.ts +++ b/packages/cli/src/api.ts @@ -75,7 +75,9 @@ export class Api { const text = result.content.find((block) => block.type === "text")?.text ?? ""; if (result.isError) - throw new CliError(text || "telemetry.dev could not complete this request."); + throw new CliError(text || "telemetry.dev could not complete this request.", { + code: "tool_error", + }); try { return JSON.parse(text) as T; @@ -83,6 +85,7 @@ export class Api { // The server cuts results over its size cap, which leaves the JSON unterminated. throw new CliError( "The result was too large to read. Narrow it with filters or a smaller --limit.", + { code: "result_too_large" }, ); } } @@ -96,35 +99,71 @@ export class Api { // Each is a refresh or authorization the server refused for good. type AuthErrors = readonly (abstract new (...args: never[]) => Error)[]; +const NETWORK_CODES = new Set([ + "ECONNREFUSED", + "ECONNRESET", + "EAI_AGAIN", + "EHOSTUNREACH", + "ENETUNREACH", + "ENOTFOUND", + "ETIMEDOUT", + "UND_ERR_SOCKET", +]); + +// undici's own timeouts, raised by fetch before any of the CLI's timers fire. +const TIMEOUT_CODES = new Set([ + "UND_ERR_CONNECT_TIMEOUT", + "UND_ERR_HEADERS_TIMEOUT", + "UND_ERR_BODY_TIMEOUT", +]); + const translateError = (error: unknown, authErrors: AuthErrors): never => { if (error instanceof CliError) throw error; if (authErrors.some((type) => error instanceof type)) { throw new CliError( "Your sign-in has expired or was disconnected. Run `telemetry login` again.", - EXIT.auth, + { exitCode: EXIT.auth, code: "sign_in_expired", hint: "telemetry login" }, ); } - const cause = error instanceof Error && error.cause instanceof Error ? error.cause : error; - const message = cause instanceof Error ? cause.message : String(cause); - const code = (cause as { code?: unknown } | null)?.code; + // The network error sits at the bottom of a chain: the SDK wraps fetch's + // "fetch failed", which wraps the socket error that has the code. + const chain: unknown[] = [error]; - if ( - typeof code === "string" && - ["ECONNREFUSED", "ENOTFOUND", "EAI_AGAIN", "ECONNRESET", "ETIMEDOUT"].includes(code) - ) { + for (let link = error; link instanceof Error && link.cause !== undefined && chain.length < 8; ) { + link = link.cause; + chain.push(link); + } + + const codes = chain + .map((link) => (link as { code?: unknown } | null)?.code) + .filter((code): code is string => typeof code === "string"); + const network = codes.find((code) => NETWORK_CODES.has(code)); + // Node's AggregateError for a failed connection has an empty message. + const message = + chain.findLast((link): link is Error => link instanceof Error && link.message !== "") + ?.message ?? String(error); + + if (network) { throw new CliError( - `Couldn't reach telemetry.dev (${code}). Check your connection and try again.`, + `Couldn't reach telemetry.dev (${network}). Check your connection and try again.`, + { code: "unreachable", retryable: true }, ); } if ( - (error as { code?: unknown } | null)?.code === "REQUEST_TIMEOUT" || - (cause instanceof Error && (cause.name === "TimeoutError" || cause.name === "AbortError")) + codes.some((code) => code === "REQUEST_TIMEOUT" || TIMEOUT_CODES.has(code)) || + chain.some( + (link) => + link instanceof Error && (link.name === "TimeoutError" || link.name === "AbortError"), + ) ) { - throw new CliError("telemetry.dev took too long to answer. Try again shortly."); + throw new CliError("telemetry.dev took too long to answer. Try again shortly.", { + code: "timeout", + retryable: true, + }); } - throw new CliError(`telemetry.dev request failed: ${message}`); + throw new CliError(`telemetry.dev request failed: ${message}`, { code: "request_failed" }); }; diff --git a/packages/cli/src/auth.ts b/packages/cli/src/auth.ts index 1e8a14c..7fcdde0 100644 --- a/packages/cli/src/auth.ts +++ b/packages/cli/src/auth.ts @@ -264,10 +264,11 @@ export class SessionAuthProvider implements OAuthClientProvider { } const sessionExpired = () => - new CliError( - "Your sign-in has expired or was disconnected. Run `telemetry login` again.", - EXIT.auth, - ); + new CliError("Your sign-in has expired or was disconnected. Run `telemetry login` again.", { + exitCode: EXIT.auth, + code: "sign_in_expired", + hint: "telemetry login", + }); export const grantedScopes = (session: Session) => new Set((session.tokens?.scope ?? "").split(" ").filter(Boolean)); @@ -441,7 +442,7 @@ const authorizationParams = (address: string, state: string) => { if (params?.get("state") !== state) { throw new CliError( "That isn't the address from this sign-in. Copy the browser's whole address bar after you select Allow.", - EXIT.auth, + { exitCode: EXIT.auth, code: "sign_in_failed" }, ); } @@ -475,14 +476,14 @@ const authorize = async ( error.code === "invalid_grant" ? "That sign-in link was already used or has expired. Run `telemetry login` again." : `Sign-in failed: ${oneLine(error.message).slice(0, 300)}`, - EXIT.auth, + { exitCode: EXIT.auth, code: "sign_in_failed", hint: "telemetry login" }, ); } if (error instanceof IssuerMismatchError || error instanceof AuthorizationServerMismatchError) { throw new CliError( "The sign-in answer came from a different authorization server than this sign-in started with. Run `telemetry login` again.", - EXIT.auth, + { exitCode: EXIT.auth, code: "sign_in_failed", hint: "telemetry login" }, ); } @@ -499,7 +500,7 @@ const redeem = async (provider: SessionAuthProvider, params: URLSearchParams) => error === "access_denied" ? "Sign-in was cancelled in the browser." : `Sign-in failed: ${oneLine(params.get("error_description") ?? error)}.`, - EXIT.auth, + { exitCode: EXIT.auth, code: "sign_in_failed", hint: "telemetry login" }, ); } @@ -508,7 +509,7 @@ const redeem = async (provider: SessionAuthProvider, params: URLSearchParams) => if (!code) { throw new CliError( "The browser returned without an authorization code. Run `telemetry login` again.", - EXIT.auth, + { exitCode: EXIT.auth, code: "sign_in_failed", hint: "telemetry login" }, ); } @@ -587,7 +588,7 @@ export const login = async ({ if (!pending) { throw new CliError( "No sign-in is waiting to finish. Run `telemetry login --no-browser` to start one.", - EXIT.auth, + { exitCode: EXIT.auth, code: "no_pending_sign_in", hint: "telemetry login --no-browser" }, ); } @@ -661,7 +662,7 @@ export const login = async ({ reject( new CliError( "Sign-in timed out after 10 minutes. Run `telemetry login` to try again.", - EXIT.auth, + { exitCode: EXIT.auth, code: "sign_in_failed", hint: "telemetry login" }, ), ), LOGIN_TIMEOUT_MS, diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 19c6769..0369660 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -5,6 +5,7 @@ import { VERSION } from "./api.ts"; import { type Context, home, + jsonText, loginCommand, logoutCommand, logsCommand, @@ -18,7 +19,7 @@ import { usageCommand, useCommand, } from "./commands.ts"; -import { CliError, EXIT, usageError } from "./errors.ts"; +import { CliError, type ErrorCode, EXIT, usageError } from "./errors.ts"; import { clean, disableColor, SHOW_CURSOR, style } from "./terminal.ts"; type Option = { @@ -309,8 +310,18 @@ const closest = (input: string, names: readonly string[]) => { const commandNamed = (name: string | undefined) => name !== undefined && Object.hasOwn(COMMANDS, name) ? COMMANDS[name] : undefined; +// Arguments after `--` are values, never options. +const optionArgs = (argv: readonly string[]) => { + const end = argv.indexOf("--"); + + return end === -1 ? argv : argv.slice(0, end); +}; + +const jsonOutput = optionArgs(process.argv.slice(2)).includes("--json"); + const main = async (argv: readonly string[]) => { - if (argv.includes("--no-color")) disableColor(); + // With --json, stderr carries no spinner or color, so an error line is plain JSON. + if (argv.includes("--no-color") || jsonOutput) disableColor(); // The command comes first; only --no-color may precede it. const first = argv.findIndex((arg) => arg !== "--no-color"); @@ -338,7 +349,12 @@ const main = async (argv: readonly string[]) => { const command = commandNamed(topic); - if (!command) throw usageError(`Unknown command "${topic}". Run telemetry --help.`); + if (!command) { + throw usageError(`Unknown command "${topic}". Run telemetry --help.`, { + code: "unknown_command", + hint: "telemetry --help", + }); + } return void process.stdout.write(`${commandHelp(topic, command)}\n`); } @@ -350,6 +366,7 @@ const main = async (argv: readonly string[]) => { throw usageError( `Unknown command "${name}".${suggestion ? ` Did you mean \`telemetry ${suggestion}\`?` : ""} Run telemetry --help.`, + { code: "unknown_command", hint: "telemetry --help" }, ); } @@ -373,13 +390,17 @@ const main = async (argv: readonly string[]) => { } catch (error) { const message = error instanceof Error ? error.message.split("\n")[0] : String(error); - throw usageError(`${message} Run \`telemetry ${name} --help\`.`); + throw usageError(`${message} Run \`telemetry ${name} --help\`.`, { + hint: `telemetry ${name} --help`, + }); } const extra = parsed.positionals[command.arguments ?? 0]; if (extra !== undefined) { - throw usageError(`Unexpected argument "${extra}". Run \`telemetry ${name} --help\`.`); + throw usageError(`Unexpected argument "${extra}". Run \`telemetry ${name} --help\`.`, { + hint: `telemetry ${name} --help`, + }); } await command.run({ @@ -390,21 +411,40 @@ const main = async (argv: readonly string[]) => { }; const fail = (error: unknown) => { - process.stderr.write(SHOW_CURSOR.repeat(process.stderr.isTTY ? 1 : 0)); - - // Tool errors are written by the server and can quote what the request named. - if (error instanceof CliError) { - process.stderr.write(`${style.red("error", process.stderr)} ${clean(error.message)}\n`); - process.exitCode = error.exitCode; + if (!jsonOutput) process.stderr.write(SHOW_CURSOR.repeat(process.stderr.isTTY ? 1 : 0)); + + const known = error instanceof CliError ? error : undefined; + const exitCode = known?.exitCode ?? EXIT.failure; + process.exitCode = exitCode; + + // With --json, a script or agent reads the error as one JSON line on stderr; stdout + // stays empty, so it never mistakes an error for data. + if (jsonOutput) { + process.stderr.write( + `${jsonText({ + error: { + code: known?.code ?? ("internal" satisfies ErrorCode), + message: known ? known.message : error instanceof Error ? error.message : String(error), + ...(known?.hint ? { hint: known.hint } : {}), + retryable: known?.retryable ?? false, + exit_code: exitCode, + cli_version: VERSION, + }, + })}\n`, + ); return; } - // Unexpected, so the stack is kept for a bug report; it can still quote a server. - process.stderr.write( - `${style.red("error", process.stderr)} ${clean(error instanceof Error ? (error.stack ?? error.message) : String(error))}\n`, - ); - process.exitCode = EXIT.failure; + // Tool errors are written by the server and can quote what the request named. An + // unexpected error keeps its stack for a bug report; it can still quote a server. + const message = known + ? known.message + : error instanceof Error + ? (error.stack ?? error.message) + : String(error); + + process.stderr.write(`${style.red("error", process.stderr)} ${clean(message)}\n`); }; // The MCP SDK warns app developers on the console, for instance when it retries a refresh. diff --git a/packages/cli/src/commands.ts b/packages/cli/src/commands.ts index 5755555..06f401f 100644 --- a/packages/cli/src/commands.ts +++ b/packages/cli/src/commands.ts @@ -58,16 +58,17 @@ const note = (text = "") => process.stderr.write(`${text}\n`); /** * JSON for scripts and agents. JSON.stringify escapes C0 controls but not C1 controls - * or bidirectional overrides, which some terminals act on, so those are escaped too. + * or bidirectional overrides, which some terminals act on, or the line and paragraph + * separators some line readers split on, so those are escaped too. */ -const printJson = (value: unknown) => - out( - JSON.stringify(value, null, 2).replace( - /[\u0080-\u009f\u061c\u200e\u200f\u202a-\u202e\u2066-\u2069]/g, - (char) => `\\u${char.charCodeAt(0).toString(16).padStart(4, "0")}`, - ), +export const jsonText = (value: unknown, indent?: number) => + JSON.stringify(value, null, indent).replace( + /[\u0080-\u009f\u061c\u200e\u200f\u2028\u2029\u202a-\u202e\u2066-\u2069]/g, + (char) => `\\u${char.charCodeAt(0).toString(16).padStart(4, "0")}`, ); +const printJson = (value: unknown) => out(jsonText(value, 2)); + const json = (context: Context) => context.values.json === true; const string = (context: Context, name: string) => { @@ -123,10 +124,14 @@ const matchProject = (projects: ProjectList["projects"], wanted: string): Projec if (byName.length > 1) { throw usageError( `Several projects are named "${wanted}". Pass its id instead: telemetry projects`, + { code: "project_ambiguous", hint: "telemetry projects" }, ); } - throw usageError(`No project "${wanted}" in this workspace. Projects: ${names || "none"}.`); + throw usageError(`No project "${wanted}" in this workspace. Projects: ${names || "none"}.`, { + code: "project_not_found", + hint: "telemetry projects", + }); }; /** @@ -148,10 +153,13 @@ const resolveProject = async (api: Api, context: Context): Promise => { if (projects.length === 1) return toProject(projects[0]!); if (projects.length === 0) - throw new CliError("This workspace has no projects yet. Create one at telemetry.dev."); + throw new CliError("This workspace has no projects yet. Create one at telemetry.dev.", { + code: "no_projects", + }); throw usageError( `Choose a project with --project , or set a default with \`telemetry use \`. Projects: ${projectNames(projects)}.`, + { code: "project_required", hint: "telemetry projects" }, ); }; @@ -1085,7 +1093,7 @@ export async function payloadCommand(context: Context) { if (!grantedScopes(session).has(PAYLOADS_SCOPE)) { throw new CliError( 'This sign-in can\'t read captured prompts and outputs. Run `telemetry login`, check "Also read captured prompts, outputs, and tool arguments", and select Allow.', - EXIT.auth, + { exitCode: EXIT.auth, code: "payloads_not_allowed", hint: "telemetry login" }, ); } diff --git a/packages/cli/src/errors.ts b/packages/cli/src/errors.ts index 2973b3e..1cb0b3c 100644 --- a/packages/cli/src/errors.ts +++ b/packages/cli/src/errors.ts @@ -7,17 +7,64 @@ export const EXIT = { interrupted: 130, } as const; +/** What scripts and agents branch on. Listed in the README; only ever added to. */ +export type ErrorCode = + | "failed" + | "internal" + | "invalid_arguments" + | "unknown_command" + | "project_not_found" + | "project_ambiguous" + | "project_required" + | "no_projects" + | "not_signed_in" + | "sign_in_expired" + | "sign_in_failed" + | "no_pending_sign_in" + | "payloads_not_allowed" + | "unreachable" + | "timeout" + | "tool_error" + | "result_too_large" + | "request_failed"; + +type ErrorDetails = { + readonly exitCode?: number; + readonly code?: ErrorCode; + /** A command to run next. `telemetry login` needs a person at a browser. */ + readonly hint?: string; + /** Whether running the same command again may succeed. */ + readonly retryable?: boolean; +}; + /** A failure with a message written for the person running the command. */ export class CliError extends Error { readonly exitCode: number; + readonly code: ErrorCode; + readonly hint: string | undefined; + readonly retryable: boolean; - constructor(message: string, exitCode: number = EXIT.failure) { + constructor(message: string, details: number | ErrorDetails = {}) { super(message); + const { + exitCode = EXIT.failure, + code, + hint, + retryable = false, + } = typeof details === "number" ? { exitCode: details } : details; this.exitCode = exitCode; + this.code = code ?? (exitCode === EXIT.usage ? "invalid_arguments" : "failed"); + this.hint = hint; + this.retryable = retryable; } } -export const usageError = (message: string) => new CliError(message, EXIT.usage); +export const usageError = (message: string, details: Omit = {}) => + new CliError(message, { ...details, exitCode: EXIT.usage }); export const notSignedIn = () => - new CliError("You're not signed in. Run `telemetry login` first.", EXIT.auth); + new CliError("You're not signed in. Run `telemetry login` first.", { + exitCode: EXIT.auth, + code: "not_signed_in", + hint: "telemetry login", + }); diff --git a/packages/cli/tests/cli.test.ts b/packages/cli/tests/cli.test.ts index 71a80e6..f2c310c 100644 --- a/packages/cli/tests/cli.test.ts +++ b/packages/cli/tests/cli.test.ts @@ -1,10 +1,11 @@ import { execFile } from "node:child_process"; import { mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises"; +import { type AddressInfo, createServer } from "node:net"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; -import { afterAll, beforeAll, beforeEach, expect, test } from "vitest"; +import { afterAll, beforeAll, beforeEach, expect, test, vi } from "vitest"; import { sessionFile } from "../src/session.ts"; import { @@ -192,6 +193,106 @@ test("input that isn't valid exits 2 before calling telemetry.dev, even from a s expect(calls).toEqual([]); }); +test("with --json, an error is one JSON line on stderr with a code to branch on, and stdout stays empty", async () => { + const errorOf = async (result: { code: number; stdout: string; stderr: string }) => { + expect(result.stdout).toBe(""); + expect(result.stderr.trim().split("\n")).toHaveLength(1); + + return { exit: result.code, ...JSON.parse(result.stderr).error }; + }; + + expect(await errorOf(await run("traces", "--json"))).toMatchObject({ + exit: 4, + code: "not_signed_in", + hint: "telemetry login", + retryable: false, + exit_code: 4, + }); + + await signIn(PROJECTS[0]); + + expect(await errorOf(await run("traces", "--limit", "99", "--json"))).toMatchObject({ + exit: 2, + code: "invalid_arguments", + exit_code: 2, + }); + expect(await errorOf(await run("traces", "--project", "nope", "--json"))).toMatchObject({ + exit: 2, + code: "project_not_found", + hint: "telemetry projects", + }); + expect(await errorOf(await run("trace", "0000", "--json"))).toMatchObject({ + exit: 1, + code: "tool_error", + message: "Trace not found.", + }); + + // A port that was just closed refuses the connection. + const closed = await new Promise((resolve) => { + const server = createServer().listen(0, "127.0.0.1", () => { + const { port } = server.address() as AddressInfo; + server.close(() => resolve(`http://127.0.0.1:${port}`)); + }); + }); + await writeFile( + join(home, sessionFile(closed)), + JSON.stringify({ server: closed, tokens: { access_token: TOKEN, token_type: "Bearer" } }), + ); + expect( + await errorOf( + await runWith( + { TELEMETRY_DEV_URL: closed }, + "traces", + "--project", + PROJECTS[0]!.id, + "--json", + ), + ), + ).toMatchObject({ exit: 1, code: "unreachable", retryable: true }); + + // After `--`, --json is a value, so the error stays the line a person reads. + const value = await run("use", "--", "--json"); + expect(value.code).toBe(2); + expect(value.stderr).toMatch(/^error No project "--json"/); + + // The same failures without --json keep the line a person reads. + const human = await run("trace", "0000"); + expect(human.code).toBe(1); + expect(human.stderr).toBe("error Trace not found.\n"); +}); + +test("a request that times out inside fetch is a retryable timeout, not a failed request", async () => { + const { Api } = await import("../src/api.ts"); + + for (const code of [ + "UND_ERR_CONNECT_TIMEOUT", + "UND_ERR_HEADERS_TIMEOUT", + "UND_ERR_BODY_TIMEOUT", + ]) { + vi.stubGlobal("fetch", () => + Promise.reject( + new TypeError("fetch failed", { cause: Object.assign(new Error(""), { code }) }), + ), + ); + + const api = new Api({ + server: fixture.url, + tokens: { access_token: TOKEN, token_type: "Bearer" }, + }); + + try { + await expect(api.call("list_projects")).rejects.toMatchObject({ + code: "timeout", + retryable: true, + exitCode: 1, + }); + } finally { + vi.unstubAllGlobals(); + await api.close(); + } + } +}); + const savedSession = async () => JSON.parse(await readFile(sessionPath(), "utf8")); test("a shell without a terminal signs in with --no-browser, then --finish, keeping the old sign-in until then and revoking it after", async () => {