Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,38 @@ See `docs/TESTING.md` for details.

New features must include telemetry instrumentation. See `src/cli/telemetry/README.md` for how to add metrics.

## Error Types

Custom error types are encouraged — each distinct error class appears as its own category in telemetry, giving more
granular failure data. All errors live in `src/lib/errors/types.ts`.

To add a new error:

1. Define a class in `src/lib/errors/types.ts` extending `BaseError` with a default `errorSource`:
- `'user'` — user-fixable (bad input, missing credentials, invalid config)
- `'client'` — our bug or unexpected local failure
- `'service'` — remote service issue (timeout, connection failure, server error)
- `'unknown'` — source cannot be determined

```ts
export class MyNewError extends BaseError {
constructor(message: string, options?: BaseErrorOptions) {
super(message, { defaultSource: 'user', ...options });
}
}
Comment thread
Hweinstock marked this conversation as resolved.
```

Callers can override the source per throw site:

```ts
throw new MyNewError('not found', { errorSource: 'service', cause: originalErr });
```

2. Add `'MyNewError'` to the `ErrorName` enum in `src/cli/telemetry/schemas/common-shapes.ts`.

That's it. The telemetry client reads `errorSource` and `name` directly from `BaseError` instances — no classification
logic to update.

## Multi-Partition Support (GovCloud, China)

The CLI supports multiple AWS partitions (commercial, GovCloud, China) through a central utility at
Expand Down
3 changes: 2 additions & 1 deletion src/cli/aws/__tests__/account-extended.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { AwsCredentialsError, detectAccount, getCredentialProvider, validateAwsCredentials } from '../account.js';
import { AwsCredentialsError } from '../../../lib/errors/types.js';
import { detectAccount, getCredentialProvider, validateAwsCredentials } from '../account.js';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

const { mockSend } = vi.hoisted(() => ({
Expand Down
2 changes: 1 addition & 1 deletion src/cli/aws/__tests__/account.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AwsCredentialsError } from '../account.js';
import { AwsCredentialsError } from '../../../lib/errors/types.js';
import { describe, expect, it } from 'vitest';

describe('AwsCredentialsError', () => {
Expand Down
16 changes: 1 addition & 15 deletions src/cli/aws/account.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { AwsCredentialsError } from '../../lib/errors/types.js';
import { getAwsLoginGuidance } from '../external-requirements/checks';
import { GetCallerIdentityCommand, STSClient } from '@aws-sdk/client-sts';
import { fromEnv, fromNodeProviderChain } from '@aws-sdk/credential-providers';
Expand All @@ -13,21 +14,6 @@ export function getCredentialProvider(): AwsCredentialIdentityProvider {
return hasEnvCreds ? fromEnv() : fromNodeProviderChain();
}

/**
* Error thrown when AWS credentials are not configured or invalid.
* Supports both a short message (for interactive mode) and detailed message (for CLI mode).
*/
export class AwsCredentialsError extends Error {
/** Short message suitable for interactive mode where UI handles recovery */
readonly shortMessage: string;

constructor(shortMessage: string, detailedMessage?: string) {
super(detailedMessage ?? shortMessage);
this.name = 'AwsCredentialsError';
this.shortMessage = shortMessage;
}
}

/**
* Get AWS account ID using STS GetCallerIdentity with detailed error handling.
* Throws AwsCredentialsError with helpful messages for common credential issues.
Expand Down
3 changes: 2 additions & 1 deletion src/cli/commands/import/phase2-import.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { PollExhaustedError, PollTimeoutError, isThrottlingError, poll } from '../../../lib/utils/polling';
import { PollExhaustedError, PollTimeoutError } from '../../../lib/errors/types';
import { isThrottlingError, poll } from '../../../lib/utils/polling';
import { getCredentialProvider } from '../../aws/account';
import type { CfnTemplate } from './template-utils';
import { buildImportTemplate } from './template-utils';
Expand Down
2 changes: 1 addition & 1 deletion src/cli/operations/dev/__tests__/invoke-a2a.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ServerError } from '../invoke';
import { ServerError } from '../../../../lib/errors/types';
import { invokeA2AStreaming } from '../invoke-a2a';
import { beforeEach, describe, expect, it, vi } from 'vitest';

Expand Down
2 changes: 1 addition & 1 deletion src/cli/operations/dev/__tests__/invoke-mcp.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ServerError } from '../invoke';
import { ServerError } from '../../../../lib/errors/types';
import { callMcpTool, listMcpTools } from '../invoke-mcp';
import { beforeEach, describe, expect, it, vi } from 'vitest';

Expand Down
2 changes: 1 addition & 1 deletion src/cli/operations/dev/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export {

export { getDevConfig, getDevSupportedAgents, getAgentPort, loadProjectConfig, type DevConfig } from './config';

export { ConnectionError, ServerError, invokeAgent, invokeAgentStreaming, invokeForProtocol } from './invoke';
export { invokeAgent, invokeAgentStreaming, invokeForProtocol } from './invoke';

export { invokeA2AStreaming, fetchA2AAgentCard, type A2AAgentCard } from './invoke-a2a';

Expand Down
7 changes: 5 additions & 2 deletions src/cli/operations/dev/invoke-a2a.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ConnectionError, type InvokeStreamingOptions, type SSELogger, ServerError } from './invoke-types';
import { ConnectionError, ServerError } from '../../../lib/errors/types';
import { type InvokeStreamingOptions, type SSELogger } from './invoke-types';
import { isConnectionError, sleep } from './utils';
import { randomUUID } from 'crypto';

Expand Down Expand Up @@ -153,7 +154,9 @@ export async function* invokeA2AStreaming(options: InvokeStreamingOptions): Asyn
}
}

const finalError = new ConnectionError(lastError ?? new Error('Failed to connect to A2A server after retries'));
const finalError = new ConnectionError(lastError?.message ?? 'Failed to connect to A2A server after retries', {
cause: lastError,
});
logger?.log?.('error', `Failed to connect after ${maxRetries} attempts: ${finalError.message}`);
throw finalError;
}
Expand Down
7 changes: 5 additions & 2 deletions src/cli/operations/dev/invoke-agui.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ConnectionError, ServerError } from '../../../lib/errors/types';
import { parseAguiSSEStream } from '../../aws/agui-parser';
import { AguiEventType } from '../../aws/agui-types';
import { ConnectionError, type InvokeStreamingOptions, ServerError } from './invoke-types';
import { type InvokeStreamingOptions } from './invoke-types';
import { isConnectionError, sleep } from './utils';
import { randomUUID } from 'crypto';

Expand Down Expand Up @@ -147,7 +148,9 @@ export async function* invokeAguiStreaming(options: InvokeStreamingOptions): Asy
}
}

const finalError = new ConnectionError(lastError ?? new Error('Failed to connect to AGUI server after retries'));
const finalError = new ConnectionError(lastError?.message ?? 'Failed to connect to AGUI server after retries', {
cause: lastError,
});
logger?.log?.('error', `Failed to connect after ${maxRetries} attempts: ${finalError.message}`);
throw finalError;
}
7 changes: 5 additions & 2 deletions src/cli/operations/dev/invoke-mcp.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { ConnectionError, ServerError } from '../../../lib/errors/types';
import { parseJsonRpcResponse } from '../../../lib/utils/json-rpc';
import { ConnectionError, type SSELogger, ServerError } from './invoke-types';
import { type SSELogger } from './invoke-types';
import { isConnectionError, sleep } from './utils';

let requestId = 1;
Expand Down Expand Up @@ -143,7 +144,9 @@ export async function listMcpTools(
}
}

const finalError = new ConnectionError(lastError ?? new Error('Failed to connect to MCP server after retries'));
const finalError = new ConnectionError(lastError?.message ?? 'Failed to connect to MCP server after retries', {
cause: lastError,
});
logger?.log?.('error', `Failed to connect after ${maxRetries} attempts: ${finalError.message}`);
throw finalError;
}
Expand Down
19 changes: 0 additions & 19 deletions src/cli/operations/dev/invoke-types.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,3 @@
/** Error thrown when the dev server returns a non-OK HTTP response. */
export class ServerError extends Error {
constructor(
public readonly statusCode: number,
body: string
) {
super(body || `Server returned ${statusCode}`);
this.name = 'ServerError';
}
}

/** Error thrown when the connection to the dev server fails. */
export class ConnectionError extends Error {
constructor(cause: Error) {
super(cause.message);
this.name = 'ConnectionError';
}
}

/** Logger interface for SSE events and error logging */
export interface SSELogger {
logSSEEvent(rawLine: string): void;
Expand Down
14 changes: 9 additions & 5 deletions src/cli/operations/dev/invoke.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { ConnectionError, ServerError } from '../../../lib/errors/types';
import { invokeA2AStreaming } from './invoke-a2a';
import { invokeAguiStreaming } from './invoke-agui';
import { ConnectionError, type InvokeStreamingOptions, type SSELogger, ServerError } from './invoke-types';
import { type InvokeStreamingOptions, type SSELogger } from './invoke-types';
import { isConnectionError, sleep } from './utils';

// Re-export shared types so existing consumers don't break
export { ConnectionError, ServerError, type InvokeStreamingOptions, type SSELogger } from './invoke-types';
export { type InvokeStreamingOptions, type SSELogger } from './invoke-types';

/**
* Parse a single SSE data line and extract the content.
Expand Down Expand Up @@ -196,7 +196,9 @@ export async function* invokeAgentStreaming(
}

// Log final failure after all retries exhausted with full details
const finalError = new ConnectionError(lastError ?? new Error('Failed to connect to dev server after retries'));
const finalError = new ConnectionError(lastError?.message ?? 'Failed to connect to dev server after retries', {
cause: lastError,
});
logger?.log?.('error', `Failed to connect after ${maxRetries} attempts: ${finalError.message}`);
throw finalError;
}
Expand Down Expand Up @@ -304,7 +306,9 @@ export async function invokeAgent(portOrOptions: number | InvokeOptions, message
}

// Log final failure after all retries exhausted with full details
const finalError = new ConnectionError(lastError ?? new Error('Failed to connect to dev server after retries'));
const finalError = new ConnectionError(lastError?.message ?? 'Failed to connect to dev server after retries', {
cause: lastError,
});
logger?.log?.('error', `Failed to connect after ${maxRetries} attempts: ${finalError.message}`);
throw finalError;
}
29 changes: 8 additions & 21 deletions src/cli/telemetry/__tests__/client.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable @typescript-eslint/require-await */
import { AccessDeniedError, DependencyCheckError } from '../../../lib/errors/types';
import { CANCELLED, TelemetryClient } from '../client';
import { InMemorySink } from '../sinks/in-memory-sink';
import { describe, expect, it } from 'vitest';
Expand Down Expand Up @@ -48,49 +49,35 @@ describe('TelemetryClient', () => {
});
});

it('classifies PackagingError subclasses', async () => {
it('classifies DependencyCheckError correctly', async () => {
const sink = new InMemorySink();
const client = new TelemetryClient(sink);

class MissingDependencyError extends Error {
constructor() {
super('missing dep');
this.name = 'MissingDependencyError';
}
}

await expect(
client.withCommandRun('deploy', async () => {
throw new MissingDependencyError();
throw new DependencyCheckError(['missing docker']);
})
).rejects.toThrow();

expect(sink.metrics[0]!.attrs).toMatchObject({
error_name: 'PackagingError',
is_user_error: 'false',
error_name: 'DependencyCheckError',
error_source: 'user',
});
});

it('marks credential errors as user errors', async () => {
const sink = new InMemorySink();
const client = new TelemetryClient(sink);

class AwsCredentialsError extends Error {
constructor() {
super('creds expired');
this.name = 'AwsCredentialsError';
}
}

await expect(
client.withCommandRun('invoke', async () => {
throw new AwsCredentialsError();
throw new AccessDeniedError('creds expired');
})
).rejects.toThrow();

expect(sink.metrics[0]!.attrs).toMatchObject({
error_name: 'CredentialsError',
is_user_error: 'true',
error_name: 'AccessDeniedError',
error_source: 'user',
Comment thread
Hweinstock marked this conversation as resolved.
});
});

Expand Down
63 changes: 0 additions & 63 deletions src/cli/telemetry/__tests__/error-classification.test.ts

This file was deleted.

7 changes: 4 additions & 3 deletions src/cli/telemetry/client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { classifyError, isUserError } from './error-classification.js';
import { classifyError } from './error.js';
import { COMMAND_SCHEMAS, type Command, type CommandAttrs, deriveCommandGroup } from './schemas/command-run.js';
import { type CommandResult, CommandResultSchema, resilientParse } from './schemas/common-shapes.js';
import type { MetricSink } from './sinks/metric-sink.js';
Expand Down Expand Up @@ -39,10 +39,11 @@ export class TelemetryClient {
this.recordCommandRun(command, { exit_reason: 'success' }, result, durationMs);
}
} catch (err) {
const { category, source } = classifyError(err);
const failureResult: CommandResult & { exit_reason: 'failure' } = {
exit_reason: 'failure',
error_name: classifyError(err),
is_user_error: isUserError(err),
error_name: category,
error_source: source,
};
this.recordCommandRun(command, failureResult, fallbackAttrs ?? {}, Math.round(performance.now() - start));
throw err;
Expand Down
Loading
Loading