Skip to content

Commit 5e1a24f

Browse files
authored
refactor: centralize error types for consistent definition shapes (#1238)
* refactor: centralize errors with BaseError + errorSource, remove error-classification - Add BaseError abstract class with errorSource: 'user' | 'client' | 'service' - All error classes extend BaseError with their source baked in - Move error classes from 5 scattered files into src/lib/errors/types.ts - Delete src/lib/errors/config.ts, src/lib/packaging/errors.ts - Extract Zod formatting helpers into src/lib/errors/zod.ts - Delete src/cli/telemetry/error-classification.ts entirely - Extract classifyError into src/cli/telemetry/error.ts - Classification reads errorSource directly from BaseError instances - SDK name-based fallback map for AWS exceptions - Replace is_user_error boolean with error_source enum in telemetry schema - Preserve PackagingError hierarchy (subclasses extend PackagingError) * docs: fix BaseError constructor example in AGENTS.md * fix: align ConflictException SDK mapping to 'user' source ConflictException/ResourceAlreadyExistsException are user-fixable conditions (duplicate names, unresolved references), matching the ConflictError class default.
1 parent a2588b7 commit 5e1a24f

40 files changed

Lines changed: 412 additions & 409 deletions

AGENTS.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,38 @@ See `docs/TESTING.md` for details.
148148

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

151+
## Error Types
152+
153+
Custom error types are encouraged — each distinct error class appears as its own category in telemetry, giving more
154+
granular failure data. All errors live in `src/lib/errors/types.ts`.
155+
156+
To add a new error:
157+
158+
1. Define a class in `src/lib/errors/types.ts` extending `BaseError` with a default `errorSource`:
159+
- `'user'` — user-fixable (bad input, missing credentials, invalid config)
160+
- `'client'` — our bug or unexpected local failure
161+
- `'service'` — remote service issue (timeout, connection failure, server error)
162+
- `'unknown'` — source cannot be determined
163+
164+
```ts
165+
export class MyNewError extends BaseError {
166+
constructor(message: string, options?: BaseErrorOptions) {
167+
super(message, { defaultSource: 'user', ...options });
168+
}
169+
}
170+
```
171+
172+
Callers can override the source per throw site:
173+
174+
```ts
175+
throw new MyNewError('not found', { errorSource: 'service', cause: originalErr });
176+
```
177+
178+
2. Add `'MyNewError'` to the `ErrorName` enum in `src/cli/telemetry/schemas/common-shapes.ts`.
179+
180+
That's it. The telemetry client reads `errorSource` and `name` directly from `BaseError` instances — no classification
181+
logic to update.
182+
151183
## Multi-Partition Support (GovCloud, China)
152184

153185
The CLI supports multiple AWS partitions (commercial, GovCloud, China) through a central utility at

src/cli/aws/__tests__/account-extended.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { AwsCredentialsError, detectAccount, getCredentialProvider, validateAwsCredentials } from '../account.js';
1+
import { AwsCredentialsError } from '../../../lib/errors/types.js';
2+
import { detectAccount, getCredentialProvider, validateAwsCredentials } from '../account.js';
23
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
34

45
const { mockSend } = vi.hoisted(() => ({

src/cli/aws/__tests__/account.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { AwsCredentialsError } from '../account.js';
1+
import { AwsCredentialsError } from '../../../lib/errors/types.js';
22
import { describe, expect, it } from 'vitest';
33

44
describe('AwsCredentialsError', () => {

src/cli/aws/account.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { AwsCredentialsError } from '../../lib/errors/types.js';
12
import { getAwsLoginGuidance } from '../external-requirements/checks';
23
import { GetCallerIdentityCommand, STSClient } from '@aws-sdk/client-sts';
34
import { fromEnv, fromNodeProviderChain } from '@aws-sdk/credential-providers';
@@ -13,21 +14,6 @@ export function getCredentialProvider(): AwsCredentialIdentityProvider {
1314
return hasEnvCreds ? fromEnv() : fromNodeProviderChain();
1415
}
1516

16-
/**
17-
* Error thrown when AWS credentials are not configured or invalid.
18-
* Supports both a short message (for interactive mode) and detailed message (for CLI mode).
19-
*/
20-
export class AwsCredentialsError extends Error {
21-
/** Short message suitable for interactive mode where UI handles recovery */
22-
readonly shortMessage: string;
23-
24-
constructor(shortMessage: string, detailedMessage?: string) {
25-
super(detailedMessage ?? shortMessage);
26-
this.name = 'AwsCredentialsError';
27-
this.shortMessage = shortMessage;
28-
}
29-
}
30-
3117
/**
3218
* Get AWS account ID using STS GetCallerIdentity with detailed error handling.
3319
* Throws AwsCredentialsError with helpful messages for common credential issues.

src/cli/commands/import/phase2-import.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { PollExhaustedError, PollTimeoutError, isThrottlingError, poll } from '../../../lib/utils/polling';
1+
import { PollExhaustedError, PollTimeoutError } from '../../../lib/errors/types';
2+
import { isThrottlingError, poll } from '../../../lib/utils/polling';
23
import { getCredentialProvider } from '../../aws/account';
34
import type { CfnTemplate } from './template-utils';
45
import { buildImportTemplate } from './template-utils';

src/cli/operations/dev/__tests__/invoke-a2a.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ServerError } from '../invoke';
1+
import { ServerError } from '../../../../lib/errors/types';
22
import { invokeA2AStreaming } from '../invoke-a2a';
33
import { beforeEach, describe, expect, it, vi } from 'vitest';
44

src/cli/operations/dev/__tests__/invoke-mcp.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ServerError } from '../invoke';
1+
import { ServerError } from '../../../../lib/errors/types';
22
import { callMcpTool, listMcpTools } from '../invoke-mcp';
33
import { beforeEach, describe, expect, it, vi } from 'vitest';
44

src/cli/operations/dev/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export {
1111

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

14-
export { ConnectionError, ServerError, invokeAgent, invokeAgentStreaming, invokeForProtocol } from './invoke';
14+
export { invokeAgent, invokeAgentStreaming, invokeForProtocol } from './invoke';
1515

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

src/cli/operations/dev/invoke-a2a.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { ConnectionError, type InvokeStreamingOptions, type SSELogger, ServerError } from './invoke-types';
1+
import { ConnectionError, ServerError } from '../../../lib/errors/types';
2+
import { type InvokeStreamingOptions, type SSELogger } from './invoke-types';
23
import { isConnectionError, sleep } from './utils';
34
import { randomUUID } from 'crypto';
45

@@ -153,7 +154,9 @@ export async function* invokeA2AStreaming(options: InvokeStreamingOptions): Asyn
153154
}
154155
}
155156

156-
const finalError = new ConnectionError(lastError ?? new Error('Failed to connect to A2A server after retries'));
157+
const finalError = new ConnectionError(lastError?.message ?? 'Failed to connect to A2A server after retries', {
158+
cause: lastError,
159+
});
157160
logger?.log?.('error', `Failed to connect after ${maxRetries} attempts: ${finalError.message}`);
158161
throw finalError;
159162
}

src/cli/operations/dev/invoke-agui.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1+
import { ConnectionError, ServerError } from '../../../lib/errors/types';
12
import { parseAguiSSEStream } from '../../aws/agui-parser';
23
import { AguiEventType } from '../../aws/agui-types';
3-
import { ConnectionError, type InvokeStreamingOptions, ServerError } from './invoke-types';
4+
import { type InvokeStreamingOptions } from './invoke-types';
45
import { isConnectionError, sleep } from './utils';
56
import { randomUUID } from 'crypto';
67

@@ -147,7 +148,9 @@ export async function* invokeAguiStreaming(options: InvokeStreamingOptions): Asy
147148
}
148149
}
149150

150-
const finalError = new ConnectionError(lastError ?? new Error('Failed to connect to AGUI server after retries'));
151+
const finalError = new ConnectionError(lastError?.message ?? 'Failed to connect to AGUI server after retries', {
152+
cause: lastError,
153+
});
151154
logger?.log?.('error', `Failed to connect after ${maxRetries} attempts: ${finalError.message}`);
152155
throw finalError;
153156
}

0 commit comments

Comments
 (0)