Skip to content

Commit ef149b1

Browse files
ankitranjan7claude
andauthored
fix(cli): report built-in command errors instead of crashing (#363)
* fix(cli): report built-in command errors instead of crashing `runCli` was `createProgram(...).parse()` with no error handling, so anything a built-in command threw escaped: sync throws printed a raw Node stack trace, and async ones surfaced as unhandled rejections. Either way the `exitCode` the error carried was discarded. Five commands hit this in practice: $ webcmd adapter path hackernews/top file:///…/dist/src/cli.js:1635 throw new ArgumentError(`Adapter source is unavailable …`); ^ ArgumentError: Adapter source is unavailable for hackernews/top. at localAdapterPath (file:///…/dist/src/cli.js:1635:19) … Adapter commands never had this problem — execution.ts wraps them and renders the shared error envelope. Use the same envelope here, so built-ins and adapters report failures identically: $ webcmd adapter path hackernews/top ok: false error: code: ARGUMENT message: Adapter source is unavailable for hackernews/top. exitCode: 2 Exit codes now come from the error rather than being lost, which also settles the taxonomy in errors.ts for these paths — `site fixture get` on a missing fixture exits 66 (EMPTY_RESULT) instead of 1, and `session close <bad-id>` exits 2 (USAGE_ERROR). Stacks stay off unless WEBCMD_DEBUG is set. `parse()` -> `parseAsync()` is what lets async rejections reach the handler, and main.ts now awaits runCli. That also keeps the daemon-run signal cancellation installed for the real duration of a run: `parse()` returned as soon as it kicked off an async action, so main.ts's `finally` uninstalled the SIGINT handler while the run was still in flight, and Ctrl-C never cancelled it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: isolate doctor profiles and unflake plugin-pr-scope Doctor rendering was reading ~/.webcmd aliases. Windows git fixtures were hitting the 5s default timeout. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 3919f6f commit ef149b1

5 files changed

Lines changed: 82 additions & 6 deletions

File tree

src/cli-error-report.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { afterEach, describe, expect, it, vi } from 'vitest';
2+
import yaml from 'js-yaml';
3+
import { reportCliError } from './cli.js';
4+
import { CliError, EXIT_CODES } from './errors.js';
5+
6+
describe('reportCliError', () => {
7+
const previousExitCode = process.exitCode;
8+
afterEach(() => { process.exitCode = previousExitCode; });
9+
10+
function capture(err: unknown): string {
11+
let text = '';
12+
reportCliError(err, { write: (chunk: string) => { text += chunk; return true; } } as unknown as NodeJS.WritableStream);
13+
return text;
14+
}
15+
16+
it('renders a CliError as the shared envelope and keeps its exit code', () => {
17+
const text = capture(new CliError('SITE_MEMORY_NOT_FOUND', 'Verify fixture a/b was not found.', undefined, EXIT_CODES.EMPTY_RESULT));
18+
19+
expect(yaml.load(text)).toEqual({
20+
ok: false,
21+
error: { code: 'SITE_MEMORY_NOT_FOUND', message: 'Verify fixture a/b was not found.', exitCode: EXIT_CODES.EMPTY_RESULT },
22+
});
23+
expect(process.exitCode).toBe(EXIT_CODES.EMPTY_RESULT);
24+
});
25+
26+
it('carries the hint through as help', () => {
27+
const text = capture(new CliError('X', 'broke', 'try this', EXIT_CODES.USAGE_ERROR));
28+
29+
expect(yaml.load(text)).toMatchObject({ error: { help: 'try this', exitCode: EXIT_CODES.USAGE_ERROR } });
30+
expect(process.exitCode).toBe(EXIT_CODES.USAGE_ERROR);
31+
});
32+
33+
it('falls back to UNKNOWN and a generic exit code for a plain error', () => {
34+
const text = capture(new Error('boom'));
35+
36+
expect(yaml.load(text)).toMatchObject({ error: { code: 'UNKNOWN', message: 'boom', exitCode: EXIT_CODES.GENERIC_ERROR } });
37+
expect(process.exitCode).toBe(EXIT_CODES.GENERIC_ERROR);
38+
});
39+
40+
it('omits the stack unless WEBCMD_DEBUG is set', () => {
41+
expect(yaml.load(capture(new Error('boom'))) as any).not.toHaveProperty('error.stack');
42+
vi.stubEnv('WEBCMD_DEBUG', '1');
43+
try {
44+
expect(String((yaml.load(capture(new Error('boom'))) as any).error.stack)).toContain('Error: boom');
45+
} finally {
46+
vi.unstubAllEnvs();
47+
}
48+
});
49+
});

src/cli.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@ import './fetch/command.js';
1919
import { commandListPresentation, filterCommandsByTag, toPresentableCommand } from './command-presentation.js';
2020
import { configureCompletionCommandSurface, configureListCommandSurface, configurePluginInstallSurface, configurePluginListSurface, configurePluginSearchSurface } from './builtin-command-surface.js';
2121
import { OUTPUT_FORMAT_HELP, resolveOutputFormat } from './command-surface.js';
22-
import { render as renderOutput } from './output.js';
22+
import { render as renderOutput, formatErrorEnvelope } from './output.js';
2323
import { PKG_VERSION } from './version.js';
2424
import { printCompletionScript } from './completion.js';
2525
import { loadExternalClis, executeExternalCli, installExternalCli, registerExternalCli, isBinaryInstalled, formatExternalCliLabel } from './external.js';
2626
import { addWebcmdSkills, listWebcmdSkills, removeWebcmdSkills, updateWebcmdSkill, type WebcmdSkillAddResult } from './skills.js';
2727
import { registerAllCommands } from './commanderAdapter.js';
2828
import { buildRootHelpPresentation, classifyAdapter, installCommanderNamespaceStructuredHelp, installRootPresentationHelp, leadingPositionalFromUsage, rootHelpData, type RootAdapterGroups } from './help.js';
29-
import { EXIT_CODES, getErrorMessage, BrowserConnectError, CliError, ArgumentError } from './errors.js';
29+
import { EXIT_CODES, getErrorMessage, toEnvelope, BrowserConnectError, CliError, ArgumentError } from './errors.js';
3030
import { TargetError, type TargetErrorCode } from './browser/target-errors.js';
3131
import { resolveTargetJs, getTextResolvedJs, getValueResolvedJs, getAttributesResolvedJs, selectResolvedJs, isAutocompleteResolvedJs, type ResolveOptions, type TargetMatchLevel } from './browser/target-resolver.js';
3232
import { buildFindJs, buildSemanticFindJs, isFindError, type FindResult, type FindError, type SemanticFindOptions } from './browser/find.js';
@@ -2074,8 +2074,31 @@ export async function loadAntigravityServe(pluginsDir: string = PLUGINS_DIR): Pr
20742074
return import(pathToFileURL(path.join(pluginsDir, 'antigravity', 'serve.js')).href);
20752075
}
20762076

2077-
export function runCli(BUILTIN_CLIS: string, USER_CLIS: string): void {
2078-
createProgram(BUILTIN_CLIS, USER_CLIS).parse();
2077+
/**
2078+
* Run the local CLI, reporting anything a built-in command throws as the same
2079+
* error envelope adapter commands already emit.
2080+
*
2081+
* Built-in actions used to have no handler at all: `parse()` does not await
2082+
* async actions, so a throw either crashed with a raw Node stack trace or
2083+
* surfaced as an unhandled rejection, and the `exitCode` the error carried was
2084+
* lost. `parseAsync` lets the rejection reach this catch.
2085+
*/
2086+
export async function runCli(BUILTIN_CLIS: string, USER_CLIS: string): Promise<void> {
2087+
try {
2088+
await createProgram(BUILTIN_CLIS, USER_CLIS).parseAsync();
2089+
} catch (err) {
2090+
reportCliError(err);
2091+
}
2092+
}
2093+
2094+
/** Render a thrown error as the shared envelope and set the exit code it carries. */
2095+
export function reportCliError(err: unknown, stderr: NodeJS.WritableStream = process.stderr): void {
2096+
const envelope = toEnvelope(err);
2097+
if (process.env.WEBCMD_DEBUG && err instanceof Error && err.stack) {
2098+
envelope.error.stack = err.stack;
2099+
}
2100+
stderr.write(formatErrorEnvelope(envelope));
2101+
process.exitCode = envelope.error.exitCode;
20792102
}
20802103

20812104
// ── Helpers ─────────────────────────────────────────────────────────────────

src/doctor.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,13 @@ afterAll(() => fs.rmSync(managedBinaryDir, { recursive: true, force: true }));
6565

6666
describe('doctor report rendering', () => {
6767
const strip = (s: string) => s.replace(/\x1b\[[0-9;]*m/g, '');
68+
const isolatedConfigDir = fs.mkdtempSync(path.join(os.tmpdir(), 'webcmd-doctor-render-'));
69+
afterAll(() => fs.rmSync(isolatedConfigDir, { recursive: true, force: true }));
6870

6971
beforeEach(() => {
7072
vi.clearAllMocks();
7173
vi.unstubAllEnvs();
74+
vi.stubEnv('WEBCMD_CONFIG_DIR', isolatedConfigDir);
7275
mockFindShadowedUserAdapters.mockReturnValue([]);
7376
mockSetDaemonCommandTimeoutSeconds.mockClear();
7477
mockEnsureBinary.mockResolvedValue(managedBinaryPath);

src/main.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,5 +211,5 @@ try {
211211
}
212212

213213
await emitHook('onStartup', { command: '__startup__', args: {} });
214-
runCli(BUILTIN_CLIS, USER_CLIS);
214+
await runCli(BUILTIN_CLIS, USER_CLIS);
215215
}

src/plugin-pr-scope.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ function fixture(change: (root: string) => void) {
3535
git(root, 'init', '--quiet');
3636
git(root, 'config', 'user.email', 'test@example.com');
3737
git(root, 'config', 'user.name', 'Test');
38+
git(root, 'config', 'commit.gpgsign', 'false');
3839
write(root, 'README.md', 'before\n');
3940
write(root, 'webcmd-plugin.json', '{}\n');
4041
write(root, 'package.json', '{}\n');
@@ -56,7 +57,7 @@ function runGuard(root: string, ...args: string[]) {
5657
return spawnSync(process.execPath, [checkerPath, ...args], { cwd: root, encoding: 'utf8' });
5758
}
5859

59-
describe('plugin-only PR scope guard', () => {
60+
describe('plugin-only PR scope guard', { timeout: 20_000 }, () => {
6061
it('accepts changes contained in one newly added plugin', () => {
6162
const { root, base, head } = fixture((repo) => addPlugin(repo, 'foo'));
6263

0 commit comments

Comments
 (0)