diff --git a/src/__tests__/e2e/cli-error-exit.test.ts b/src/__tests__/e2e/cli-error-exit.test.ts new file mode 100644 index 000000000..b5c13cefc --- /dev/null +++ b/src/__tests__/e2e/cli-error-exit.test.ts @@ -0,0 +1,76 @@ +import { describe, it, expect } from 'vitest'; +import { spawn } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..', '..'); +const CLI = path.join(ROOT, 'dist', 'index.js'); + +function runCLI( + args: string[], + home: string, + cwd: string, +): Promise<{ code: number | null; stdout: string; stderr: string }> { + return new Promise((resolve) => { + const child = spawn('node', [CLI, ...args], { + env: { ...process.env, FORCE_COLOR: '0', HOME: home, USERPROFILE: home }, + stdio: ['pipe', 'pipe', 'pipe'], + cwd, + }); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (d: Buffer) => { + stdout += d.toString(); + }); + child.stderr.on('data', (d: Buffer) => { + stderr += d.toString(); + }); + child.stdin.end(); + child.on('close', (code) => resolve({ code, stdout, stderr })); + }); +} + +describe('an async command action that throws', () => { + // #1270: a rejection reached the user as an unhandled one, so Node printed + // the error under a `cli.js:NNNN` header with its stack. `session list` + // throws a plain Error rather than a NotInitializedError, so the entry + // boundary has to cover it, not only the commands that wrap themselves. + function fixture(): { root: string; home: string; work: string } { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'teamai-cmd-error-')); + const home = path.join(root, 'home'); + const work = path.join(root, 'work'); + fs.mkdirSync(home); + fs.mkdirSync(work); + return { root, home, work }; + } + + it('prints one line on stderr and exits 1, with no stack', async () => { + const { root, home, work } = fixture(); + try { + const { code, stdout, stderr } = await runCLI(['session', 'list'], home, work); + expect(code).toBe(1); + expect(stderr).toContain('No teamai project config found'); + // The `cli.js:NNNN` header and the `at ...` frames of an unhandled rejection. + expect(stderr).not.toContain('cli.js:'); + expect(stderr).not.toMatch(/^\s+at /m); + expect(stdout).toBe(''); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }, 60_000); + + it('keeps the stack reachable under --verbose', async () => { + const { root, home, work } = fixture(); + try { + const { code, stdout, stderr } = await runCLI(['-v', 'session', 'list'], home, work); + expect(code).toBe(1); + expect(stderr).toContain('No teamai project config found'); + expect(stdout).toMatch(/^\s+at /m); + expect(stdout).toContain('resolveRepoRoot'); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }, 60_000); +}); diff --git a/src/__tests__/index-codebase-help.test.ts b/src/__tests__/index-codebase-help.test.ts index 02e1a96f9..1a43c67b1 100644 --- a/src/__tests__/index-codebase-help.test.ts +++ b/src/__tests__/index-codebase-help.test.ts @@ -4,6 +4,7 @@ const originalArgv = process.argv; afterEach(() => { process.argv = originalArgv; + process.exitCode = undefined; vi.restoreAllMocks(); vi.resetModules(); }); @@ -19,7 +20,9 @@ async function readCodebaseHelp(): Promise { throw new Error(`process.exit(${code})`); }) as never); - await expect(import('../index.js')).rejects.toThrow('process.exit(0)'); + // The help text is on stdout by now. The entry reports the throw from the + // mocked exit as an error and resolves instead of rejecting (#1270). + await import('../index.js'); return output; } diff --git a/src/__tests__/index-import-help.test.ts b/src/__tests__/index-import-help.test.ts index 0cc8936e7..b2d877db3 100644 --- a/src/__tests__/index-import-help.test.ts +++ b/src/__tests__/index-import-help.test.ts @@ -4,6 +4,7 @@ const originalArgv = process.argv; afterEach(() => { process.argv = originalArgv; + process.exitCode = undefined; vi.restoreAllMocks(); vi.resetModules(); }); @@ -19,7 +20,9 @@ async function readImportHelp(): Promise { throw new Error(`process.exit(${code})`); }) as never); - await expect(import('../index.js')).rejects.toThrow('process.exit(0)'); + // The help text is on stdout by now. The entry reports the throw from the + // mocked exit as an error and resolves instead of rejecting (#1270). + await import('../index.js'); return output; } diff --git a/src/__tests__/index-not-initialized.test.ts b/src/__tests__/index-not-initialized.test.ts index 108f5774a..e9c3ff78a 100644 --- a/src/__tests__/index-not-initialized.test.ts +++ b/src/__tests__/index-not-initialized.test.ts @@ -1,18 +1,19 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { loadCommandTable } from './helpers/command-table.js'; -const { status, list, push, logError } = vi.hoisted(() => ({ +const { status, list, push, logError, logDebug } = vi.hoisted(() => ({ status: vi.fn(), list: vi.fn(), push: vi.fn(), logError: vi.fn(), + logDebug: vi.fn(), })); vi.mock('../status.js', () => ({ status, list })); vi.mock('../push.js', () => ({ push })); vi.mock('../migrate.js', () => ({ maybeMigrate: vi.fn(async () => undefined), queueKeptInCheckout: vi.fn() })); vi.mock('../utils/logger.js', async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, log: { ...actual.log, error: logError } }; + return { ...actual, log: { ...actual.log, error: logError, debug: logDebug } }; }); import { NotInitializedError } from '../config.js'; @@ -46,7 +47,7 @@ describe('status, list and push before init', () => { expect(process.exitCode).toBe(1); }); - it('still lets other errors escape', async () => { + it('the per-command wrapper rethrows other errors, for the entry boundary to report', async () => { status.mockRejectedValue(new Error('disk full')); const program = await loadCommandTable(); @@ -54,3 +55,63 @@ describe('status, list and push before init', () => { expect(logError).not.toHaveBeenCalled(); }); }); + +/** + * The entry boundary (#1270): whatever a command action throws becomes one line + * on stderr and exit 1, with the stack left to `--verbose` and debug.log. + */ +describe('reportCommandError', () => { + /** `loadCommandTable` stubs TEAMAI_COMMAND_TABLE_ONLY first, so this import does not run the CLI. */ + async function load(): Promise<(e: unknown) => Promise> { + await loadCommandTable(); + const { reportCommandError } = await import('../index.js'); + return reportCommandError; + } + + beforeEach(() => { + logError.mockReset(); + logDebug.mockReset(); + }); + + afterEach(() => { + process.exitCode = undefined; + vi.unstubAllEnvs(); + }); + + it('prints the message and exits 1 instead of rejecting', async () => { + const reportCommandError = await load(); + + await expect(reportCommandError(new Error('disk full'))).resolves.toBeUndefined(); + + expect(logError).toHaveBeenCalledTimes(1); + expect(logError).toHaveBeenCalledWith('disk full'); + expect(process.exitCode).toBe(1); + }); + + it('keeps the stack for the debug log and --verbose', async () => { + const reportCommandError = await load(); + const error = new Error('disk full'); + + await reportCommandError(error); + + expect(logDebug).toHaveBeenCalledWith(error.stack); + }); + + it('falls back to the error name when the message is empty', async () => { + const reportCommandError = await load(); + + await reportCommandError(new Error('')); + + expect(logError).toHaveBeenCalledWith('Error'); + }); + + it('reports a thrown value that is not an Error, without inventing a stack', async () => { + const reportCommandError = await load(); + + await reportCommandError('boom'); + + expect(logError).toHaveBeenCalledWith('boom'); + expect(logDebug).not.toHaveBeenCalled(); + expect(process.exitCode).toBe(1); + }); +}); diff --git a/src/index.ts b/src/index.ts index 3e0831ee1..dc614de0c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -64,6 +64,26 @@ async function reportNotInitialized(run: () => Promise): Promise { } } +/** + * Report an error a command action threw as one line on stderr, and make the + * process exit 1 (#1270). Without this, a rejection from an async action + * reaches the user as an unhandled one: Node prints the error with its stack + * and a `cli.js:NNNN` header, which says nothing a person can act on. + * + * A command that wraps itself in `reportNotInitialized` is unchanged; this is + * the boundary that also covers the commands that never did, such as + * `session list`. + * + * The trace is not lost: `log.debug` always appends it to ~/.teamai/debug.log, + * and prints it on the console only under `--verbose`. + */ +export async function reportCommandError(e: unknown): Promise { + const error = e instanceof Error ? e : undefined; + log.error(error?.message || error?.name || String(e)); + if (error?.stack) log.debug(error.stack); + process.exitCode = 1; +} + /** * Fire a command-lifecycle webhook (`push` / `pull`) best-effort. These are * default subscription events with no other production call site into @@ -1648,13 +1668,7 @@ if (!process.env.TEAMAI_COMMAND_TABLE_ONLY) { // (see bundled-runtime.ts) and pull shells out to git. ensureBundledRuntimeOnPath(); - // Commands that need an initialized teamai reject with NotInitializedError. - // Report it as a plain error, not an unhandled rejection with a stack trace. - await program.parseAsync().catch(async (e: unknown) => { - const { NotInitializedError } = await import('./config.js'); - if (!(e instanceof NotInitializedError)) throw e; - const { log } = await import('./utils/logger.js'); - log.error(e.message); - process.exitCode = 1; - }); + // Every error a command action throws leaves as one line on stderr with exit + // 1, never as an unhandled rejection with a Node stack trace (#1270). + await program.parseAsync().catch(reportCommandError); }