From 3e8dd253aeef1fe07b3f3d3d67aed2585d1cf300 Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Mon, 11 May 2026 21:42:13 +0000 Subject: [PATCH 1/2] feat: record command attrs on telemetry failure via fallbackAttrs Add optional fallbackAttrs parameter to client.withCommandRun so command-specific attributes are recorded even when the callback throws. - client.ts: accept fallbackAttrs, use on failure instead of {} - client.ts: run resilientParse on all non-empty attrs (not just success) - cli-command-run.ts: withCommandRunTelemetry passes attrs as fallbackAttrs - cli-command-run.ts: runCliCommand accepts optional knownAttrs param - command.tsx: extract knownAttrs upfront, pass to runCliCommand - client.test.ts: add unit tests for fallbackAttrs behavior - create-edge-cases.test.ts: assert attrs present on failure entry --- integ-tests/create-edge-cases.test.ts | 2 + src/cli/commands/create/command.tsx | 165 +++++++++++---------- src/cli/telemetry/README.md | 23 ++- src/cli/telemetry/__tests__/client.test.ts | 49 ++++++ src/cli/telemetry/cli-command-run.ts | 20 ++- src/cli/telemetry/client.ts | 8 +- 6 files changed, 174 insertions(+), 93 deletions(-) diff --git a/integ-tests/create-edge-cases.test.ts b/integ-tests/create-edge-cases.test.ts index 6313d7aff..b2f07640c 100644 --- a/integ-tests/create-edge-cases.test.ts +++ b/integ-tests/create-edge-cases.test.ts @@ -38,6 +38,8 @@ describe.skipIf(!prereqs.npm || !prereqs.git)('integration: create edge cases', telemetry.assertMetricEmitted({ command: 'create', exit_reason: 'failure', + language: 'python', + has_agent: 'true', }); }); diff --git a/src/cli/commands/create/command.tsx b/src/cli/commands/create/command.tsx index d4a9944d4..097ca646c 100644 --- a/src/cli/commands/create/command.tsx +++ b/src/cli/commands/create/command.tsx @@ -114,92 +114,99 @@ async function handleCreateCLI(options: CreateOptions): Promise { process.exit(0); } - await runCliCommand('create', !!options.json, async () => { - const validation = validateCreateOptions(options, cwd); - if (!validation.valid) { - throw new Error(validation.error); - } - const green = '\x1b[32m'; - const reset = '\x1b[0m'; + const knownAttrs = { + language: standardize(Language, options.language), + framework: standardize(Framework, options.framework), + model_provider: standardize(ModelProviderEnum, options.modelProvider), + memory: standardize(Memory, options.memory ?? 'none'), + protocol: standardize(Protocol, options.protocol ?? 'http'), + build: standardize(Build, options.build ?? 'codezip'), + agent_type: standardize(AgentType, options.type ?? 'create'), + network_mode: standardize(NetworkModeEnum, options.networkMode ?? 'public'), + has_agent: options.agent !== false, + }; - // Progress callback for real-time output - const onProgress: ProgressCallback | undefined = options.json - ? undefined - : (step, status) => { - if (status === 'done') { - console.log(`${green}[done]${reset} ${step}`); - } else if (status === 'error') { - console.log(`\x1b[31m[error]${reset} ${step}`); - } - // 'start' is silent - we only show when done - }; + await runCliCommand( + 'create', + !!options.json, + async () => { + const validation = validateCreateOptions(options, cwd); + if (!validation.valid) { + throw new Error(validation.error); + } + const green = '\x1b[32m'; + const reset = '\x1b[0m'; - // Commander.js --no-agent sets agent=false, not noAgent=true - const skipAgent = options.agent === false; + // Progress callback for real-time output + const onProgress: ProgressCallback | undefined = options.json + ? undefined + : (step, status) => { + if (status === 'done') { + console.log(`${green}[done]${reset} ${step}`); + } else if (status === 'error') { + console.log(`\x1b[31m[error]${reset} ${step}`); + } + // 'start' is silent - we only show when done + }; - const result = skipAgent - ? await createProject({ - name: projectName!, - cwd, - skipGit: options.skipGit, - skipInstall: options.skipInstall, - onProgress, - }) - : await createProjectWithAgent({ - name: name!, - projectName, - cwd, - type: options.type as 'create' | 'import' | undefined, - buildType: (options.build as BuildType) ?? 'CodeZip', - language: (options.language as TargetLanguage) ?? (options.type === 'import' ? 'Python' : undefined), - framework: options.framework as SDKFramework | undefined, - modelProvider: options.modelProvider as ModelProvider | undefined, - apiKey: options.apiKey, - memory: (options.memory as 'none' | 'shortTerm' | 'longAndShortTerm') ?? 'none', - protocol: options.protocol as ProtocolMode | undefined, - agentId: options.agentId, - agentAliasId: options.agentAliasId, - region: options.region, - networkMode: options.networkMode as NetworkMode | undefined, - subnets: parseCommaSeparatedList(options.subnets), - securityGroups: parseCommaSeparatedList(options.securityGroups), - idleTimeout: options.idleTimeout ? Number(options.idleTimeout) : undefined, - maxLifetime: options.maxLifetime ? Number(options.maxLifetime) : undefined, - sessionStorageMountPath: options.sessionStorageMountPath, - withConfigBundle: options.withConfigBundle, - skipGit: options.skipGit, - skipInstall: options.skipInstall, - skipPythonSetup: options.skipPythonSetup, - onProgress, - }); + // Commander.js --no-agent sets agent=false, not noAgent=true + const skipAgent = options.agent === false; - if (!result.success) { - throw result.error; - } + const result = skipAgent + ? await createProject({ + name: projectName!, + cwd, + skipGit: options.skipGit, + skipInstall: options.skipInstall, + onProgress, + }) + : await createProjectWithAgent({ + name: name!, + projectName, + cwd, + type: options.type as 'create' | 'import' | undefined, + buildType: (options.build as BuildType) ?? 'CodeZip', + language: (options.language as TargetLanguage) ?? (options.type === 'import' ? 'Python' : undefined), + framework: options.framework as SDKFramework | undefined, + modelProvider: options.modelProvider as ModelProvider | undefined, + apiKey: options.apiKey, + memory: (options.memory as 'none' | 'shortTerm' | 'longAndShortTerm') ?? 'none', + protocol: options.protocol as ProtocolMode | undefined, + agentId: options.agentId, + agentAliasId: options.agentAliasId, + region: options.region, + networkMode: options.networkMode as NetworkMode | undefined, + subnets: parseCommaSeparatedList(options.subnets), + securityGroups: parseCommaSeparatedList(options.securityGroups), + idleTimeout: options.idleTimeout ? Number(options.idleTimeout) : undefined, + maxLifetime: options.maxLifetime ? Number(options.maxLifetime) : undefined, + sessionStorageMountPath: options.sessionStorageMountPath, + withConfigBundle: options.withConfigBundle, + skipGit: options.skipGit, + skipInstall: options.skipInstall, + skipPythonSetup: options.skipPythonSetup, + onProgress, + }); - if (options.json) { - console.log(JSON.stringify(serializeResult(result))); - } else { - printCreateSummary(projectName!, result.agentName, options.language, options.framework); - if (options.skipInstall) { - console.log( - "\nDependency installation was skipped. Run 'npm install' in agentcore/cdk/ and 'uv sync' in your agent directory manually." - ); + if (!result.success) { + throw new Error(result.error); } - } - return { - language: standardize(Language, options.language), - framework: standardize(Framework, options.framework), - model_provider: standardize(ModelProviderEnum, options.modelProvider), - memory: standardize(Memory, options.memory ?? 'none'), - protocol: standardize(Protocol, options.protocol ?? 'http'), - build: standardize(Build, options.build ?? 'codezip'), - agent_type: standardize(AgentType, options.type ?? 'create'), - network_mode: standardize(NetworkModeEnum, options.networkMode ?? 'public'), - has_agent: options.agent !== false, - }; - }); + if (options.json) { + console.log(JSON.stringify(result)); + } else { + printCreateSummary(projectName!, result.agentName, options.language, options.framework); + if (options.skipInstall) { + console.log( + "\nDependency installation was skipped. Run 'npm install' in agentcore/cdk/ and 'uv sync' in your agent directory manually." + ); + } + } + + return knownAttrs; + }, + knownAttrs + ); } export const registerCreate = (program: Command) => { diff --git a/src/cli/telemetry/README.md b/src/cli/telemetry/README.md index 5d24a6a9d..49f98bbcb 100644 --- a/src/cli/telemetry/README.md +++ b/src/cli/telemetry/README.md @@ -62,11 +62,12 @@ async function withCommandRunTelemetry { }); ``` +To record attrs on failure, pass `knownAttrs` as the fourth argument: + +```ts +const knownAttrs = { widget_type: standardize(WidgetType, options.type), count: options.items.length }; +await runCliCommand( + 'add.widget', + !!options.json, + async () => { + const result = await widgetPrimitive.add(options); + if (!result.success) throw new Error(result.error); + return knownAttrs; + }, + knownAttrs +); +``` + ## Key Points - Telemetry never crashes the CLI — `standardize()` falls back gracefully, `resilientParse` defaults invalid fields to diff --git a/src/cli/telemetry/__tests__/client.test.ts b/src/cli/telemetry/__tests__/client.test.ts index 96adebafc..cfba2a0c2 100644 --- a/src/cli/telemetry/__tests__/client.test.ts +++ b/src/cli/telemetry/__tests__/client.test.ts @@ -173,5 +173,54 @@ describe('TelemetryClient', () => { exit_reason: 'cancel', }); }); + + it('records fallbackAttrs on failure when provided', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); + + await expect( + client.withCommandRun( + 'create', + async () => { + throw new Error('validation failed'); + }, + { + language: 'python', + framework: 'strands', + model_provider: 'bedrock', + memory: 'none', + protocol: 'http', + build: 'codezip', + agent_type: 'create', + network_mode: 'public', + has_agent: true, + } + ) + ).rejects.toThrow('validation failed'); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs).toMatchObject({ + exit_reason: 'failure', + error_name: 'UnknownError', + language: 'python', + framework: 'strands', + model_provider: 'bedrock', + has_agent: 'true', + }); + }); + + it('records empty attrs on failure when fallbackAttrs not provided', async () => { + const sink = new InMemorySink(); + const client = new TelemetryClient(sink); + + await expect( + client.withCommandRun('deploy', async () => { + throw new Error('boom'); + }) + ).rejects.toThrow('boom'); + + expect(sink.metrics).toHaveLength(1); + expect(sink.metrics[0]!.attrs.language).toBeUndefined(); + }); }); }); diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index d6294ac0e..6ad56117b 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -31,11 +31,15 @@ export async function withCommandRunTelemetry { - result = await fn(); - if (!result.success) throw result.error; - return attrs; - }); + await client.withCommandRun( + command, + async () => { + result = await fn(); + if (!result.success) throw new Error(result.error); + return attrs; + }, + attrs + ); } catch (e) { // withCommandRun re-throws after recording failure telemetry. // If result was set, fn() returned a failure result — return it directly. @@ -52,11 +56,13 @@ export async function withCommandRunTelemetry( command: C, json: boolean, - fn: () => Promise> + fn: () => Promise>, + knownAttrs?: Partial> ): Promise { try { const client = await getTelemetryClient(); @@ -64,7 +70,7 @@ export async function runCliCommand( await fn(); process.exit(0); } - await client.withCommandRun(command, fn); + await client.withCommandRun(command, fn, knownAttrs); process.exit(0); } catch (error) { if (json) { diff --git a/src/cli/telemetry/client.ts b/src/cli/telemetry/client.ts index 91dffd94f..2341231ea 100644 --- a/src/cli/telemetry/client.ts +++ b/src/cli/telemetry/client.ts @@ -26,7 +26,8 @@ export class TelemetryClient { */ async withCommandRun( command: C, - fn: () => CommandAttrs | typeof CANCELLED | Promise | typeof CANCELLED> + fn: () => CommandAttrs | typeof CANCELLED | Promise | typeof CANCELLED>, + fallbackAttrs?: Partial> ): Promise { const start = performance.now(); try { @@ -43,7 +44,7 @@ export class TelemetryClient { error_name: classifyError(err), is_user_error: isUserError(err), }; - this.recordCommandRun(command, failureResult, {}, Math.round(performance.now() - start)); + this.recordCommandRun(command, failureResult, fallbackAttrs ?? {}, Math.round(performance.now() - start)); throw err; } finally { try { @@ -75,9 +76,8 @@ export class TelemetryClient { // Validate command attrs resiliently: invalid fields default to 'unknown' // instead of dropping the entire metric. - // On failure/cancel the callback attrs are empty so validation is skipped. const validatedAttrs = - result.exit_reason !== 'failure' && result.exit_reason !== 'cancel' + Object.keys(attrs as Record).length > 0 ? resilientParse(COMMAND_SCHEMAS[command], attrs as Record) : attrs; From a725d92bcc44694cbd4fb31cbc3014ea51a4d4aa Mon Sep 17 00:00:00 2001 From: Harrison Weinstock Date: Tue, 12 May 2026 21:25:30 +0000 Subject: [PATCH 2/2] chore: rebase onto mainline --- src/cli/commands/create/command.tsx | 2 +- src/cli/telemetry/cli-command-run.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli/commands/create/command.tsx b/src/cli/commands/create/command.tsx index 097ca646c..a21ece491 100644 --- a/src/cli/commands/create/command.tsx +++ b/src/cli/commands/create/command.tsx @@ -189,7 +189,7 @@ async function handleCreateCLI(options: CreateOptions): Promise { }); if (!result.success) { - throw new Error(result.error); + throw result.error; } if (options.json) { diff --git a/src/cli/telemetry/cli-command-run.ts b/src/cli/telemetry/cli-command-run.ts index 6ad56117b..2fabd39fe 100644 --- a/src/cli/telemetry/cli-command-run.ts +++ b/src/cli/telemetry/cli-command-run.ts @@ -35,7 +35,7 @@ export async function withCommandRunTelemetry { result = await fn(); - if (!result.success) throw new Error(result.error); + if (!result.success) throw result.error; return attrs; }, attrs