Skip to content

Commit 3905aee

Browse files
committed
refactor: use typed errors in dev command helpers + emit browser telemetry eagerly
Error classification: - ConnectionError for connection-refused (new class in lib/errors/types.ts) - ValidationError for invalid user input (missing --tool, bad JSON, unknown command) - ResourceNotFoundError for missing container runtime Browser mode telemetry: - Emit telemetry eagerly via TelemetryClientAccessor before the blocking runBrowserMode call (which never returns). Do not copy this pattern — prefer withCommandRunTelemetry for commands that return.
1 parent 53c5d65 commit 3905aee

2 files changed

Lines changed: 52 additions & 28 deletions

File tree

src/cli/commands/dev/command.tsx

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1-
import { type Result, findConfigRoot, getWorkingDirectory } from '../../../lib';
1+
import {
2+
type Result,
3+
ConnectionError,
4+
ResourceNotFoundError,
5+
ValidationError,
6+
findConfigRoot,
7+
getWorkingDirectory,
8+
} from '../../../lib';
29
import { getErrorMessage } from '../../errors';
310
import { detectContainerRuntime } from '../../external-requirements';
411
import { ExecLogger } from '../../logging';
@@ -19,6 +26,7 @@ import {
1926
} from '../../operations/dev';
2027
import { OtelCollector, startOtelCollector } from '../../operations/dev/otel';
2128
import { withCommandRunTelemetry } from '../../telemetry/cli-command-run.js';
29+
import { TelemetryClientAccessor } from '../../telemetry/client-accessor.js';
2230
import { Protocol, standardize } from '../../telemetry/schemas/common-shapes.js';
2331
import { FatalError } from '../../tui/components';
2432
import { LayoutProvider } from '../../tui/context';
@@ -55,7 +63,9 @@ async function invokeDevServer(
5563
}
5664
} catch (err) {
5765
throw isConnectionRefused(err)
58-
? new Error(`Dev server not running on port ${port}. Start it with: agentcore dev --logs`, { cause: err })
66+
? new ConnectionError(`Dev server not running on port ${port}. Start it with: agentcore dev --logs`, {
67+
cause: err,
68+
})
5969
: err;
6070
}
6171
}
@@ -68,7 +78,9 @@ async function invokeA2ADevServer(port: number, prompt: string, headers?: Record
6878
process.stdout.write('\n');
6979
} catch (err) {
7080
throw isConnectionRefused(err)
71-
? new Error(`Dev server not running on port ${port}. Start it with: agentcore dev --logs`, { cause: err })
81+
? new ConnectionError(`Dev server not running on port ${port}. Start it with: agentcore dev --logs`, {
82+
cause: err,
83+
})
7284
: err;
7385
}
7486
}
@@ -102,7 +114,7 @@ async function handleMcpInvoke(
102114
}
103115
} else if (invokeValue === 'call-tool') {
104116
if (!toolName) {
105-
throw new Error(
117+
throw new ValidationError(
106118
'--tool is required with call-tool. Usage: agentcore dev call-tool --tool <name> --input \'{"arg": "value"}\''
107119
);
108120
}
@@ -112,27 +124,29 @@ async function handleMcpInvoke(
112124
try {
113125
args = JSON.parse(input) as Record<string, unknown>;
114126
} catch {
115-
throw new Error(`Invalid JSON for --input: ${input}. Expected format: --input '{"key": "value"}'`);
127+
throw new ValidationError(`Invalid JSON for --input: ${input}. Expected format: --input '{"key": "value"}'`);
116128
}
117129
}
118130
const result = await callMcpTool(port, toolName, args, sessionId, undefined, headers);
119131
console.log(result);
120132
} else {
121-
throw new Error(
133+
throw new ValidationError(
122134
`Unknown MCP invoke command "${invokeValue}". Usage: agentcore dev list-tools | agentcore dev call-tool --tool <name>`
123135
);
124136
}
125137
} catch (err) {
126138
throw isConnectionRefused(err)
127-
? new Error(`Dev server not running on port ${port}. Start it with: agentcore dev --logs`, { cause: err })
139+
? new ConnectionError(`Dev server not running on port ${port}. Start it with: agentcore dev --logs`, {
140+
cause: err,
141+
})
128142
: err;
129143
}
130144
}
131145

132146
async function execInContainer(command: string, containerName: string): Promise<void> {
133147
const detection = await detectContainerRuntime();
134148
if (!detection.runtime) {
135-
throw new Error('No container runtime found (docker, podman, or finch required)');
149+
throw new ResourceNotFoundError('No container runtime found (docker, podman, or finch required)');
136150
}
137151
return new Promise((resolve, reject) => {
138152
const child = spawn(detection.runtime!.binary, ['exec', containerName, 'bash', '-c', command], {
@@ -465,32 +479,32 @@ export const registerDev = (program: Command) => {
465479
}
466480

467481
// Default: launch web UI in browser
468-
// runBrowserMode blocks forever (internal await new Promise(() => {})).
469-
// On SIGINT, its internal handler stops the server and the process exits.
470-
// Telemetry only emits here on startup failure; normal shutdown telemetry
471-
// requires refactoring runWebUI to return a shutdown promise (follow-up).
472-
const browserResult = await withCommandRunTelemetry(
473-
'dev',
474-
{
482+
// NOTE: Do not copy this pattern. runBrowserMode blocks forever (internal
483+
// await new Promise(() => {})) so we cannot use withCommandRunTelemetry here.
484+
// We emit telemetry eagerly before the blocking call. If startup fails, the
485+
// error propagates to the outer catch. Prefer withCommandRunTelemetry for
486+
// commands that return.
487+
{
488+
const client = await TelemetryClientAccessor.get().catch(() => undefined);
489+
const devAttrs = {
475490
action: 'server' as const,
476491
ui_mode: 'browser' as const,
477492
has_stream: false,
478493
protocol: standardize(Protocol, (targetDevAgent?.protocol ?? 'http').toLowerCase()),
479494
invoke_count: 0,
480-
},
481-
async (): Promise<Result> => {
482-
await runBrowserMode({
483-
workingDir,
484-
project,
485-
port,
486-
agentName: opts.runtime,
487-
otelEnvVars,
488-
collector,
489-
});
490-
return { success: true };
495+
};
496+
if (client) {
497+
await client.withCommandRun('dev', () => devAttrs);
491498
}
492-
);
493-
if (!browserResult.success) throw browserResult.error;
499+
await runBrowserMode({
500+
workingDir,
501+
project,
502+
port,
503+
agentName: opts.runtime,
504+
otelEnvVars,
505+
collector,
506+
});
507+
}
494508
} catch (error) {
495509
console.error(`Error: ${getErrorMessage(error)}`);
496510
process.exit(1);

src/lib/errors/types.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,16 @@ export class ValidationError extends Error {
6565
}
6666
}
6767

68+
/**
69+
* Error indicating a network connection failure (e.g., server not reachable).
70+
*/
71+
export class ConnectionError extends Error {
72+
constructor(message: string, options?: ErrorOptions) {
73+
super(message, options);
74+
this.name = 'ConnectionError';
75+
}
76+
}
77+
6878
/**
6979
* Converts an unknown thrown value to an Error instance.
7080
* Use in catch blocks to ensure the error field is always an Error object.

0 commit comments

Comments
 (0)