From 7c4570339efc7fe52a558dc6305501776fb355ca Mon Sep 17 00:00:00 2001 From: Shyam Raghuwanshi Date: Wed, 11 Mar 2026 05:20:25 +0530 Subject: [PATCH] fix: robust UX for remote agent errors (#20307) Co-authored-by: Adam Weidman --- .../src/agents/a2a-client-manager.test.ts | 6 +- .../core/src/agents/a2a-client-manager.ts | 39 +-- packages/core/src/agents/a2a-errors.test.ts | 298 ++++++++++++++++++ packages/core/src/agents/a2a-errors.ts | 206 ++++++++++++ packages/core/src/agents/registry.test.ts | 109 ++++++- packages/core/src/agents/registry.ts | 49 ++- .../core/src/agents/remote-invocation.test.ts | 71 +++++ packages/core/src/agents/remote-invocation.ts | 22 +- 8 files changed, 768 insertions(+), 32 deletions(-) create mode 100644 packages/core/src/agents/a2a-errors.test.ts create mode 100644 packages/core/src/agents/a2a-errors.ts diff --git a/packages/core/src/agents/a2a-client-manager.test.ts b/packages/core/src/agents/a2a-client-manager.test.ts index afa66d0e5f..8cd3cc0830 100644 --- a/packages/core/src/agents/a2a-client-manager.test.ts +++ b/packages/core/src/agents/a2a-client-manager.test.ts @@ -302,7 +302,7 @@ describe('A2AClientManager', () => { expect(call.message.taskId).toBe(expectedTaskId); }); - it('should throw prefixed error on failure', async () => { + it('should propagate the original error on failure', async () => { sendMessageStreamMock.mockImplementationOnce(() => { throw new Error('Network error'); }); @@ -312,9 +312,7 @@ describe('A2AClientManager', () => { for await (const _ of stream) { // consume } - }).rejects.toThrow( - '[A2AClientManager] sendMessageStream Error [TestAgent]: Network error', - ); + }).rejects.toThrow('Network error'); }); it('should throw an error if the agent is not found', async () => { diff --git a/packages/core/src/agents/a2a-client-manager.ts b/packages/core/src/agents/a2a-client-manager.ts index 7d8f27f02b..1597502c80 100644 --- a/packages/core/src/agents/a2a-client-manager.ts +++ b/packages/core/src/agents/a2a-client-manager.ts @@ -26,6 +26,7 @@ import { v4 as uuidv4 } from 'uuid'; import { Agent as UndiciAgent } from 'undici'; import { debugLogger } from '../utils/debugLogger.js'; import { safeLookup } from '../utils/fetch.js'; +import { classifyAgentError } from './a2a-errors.js'; // Remote agents can take 10+ minutes (e.g. Deep Research). // Use a dedicated dispatcher so the global 5-min timeout isn't affected. @@ -131,18 +132,22 @@ export class A2AClientManager { }, ); - const factory = new ClientFactory(options); - const client = await factory.createFromUrl(agentCardUrl, ''); - const agentCard = await client.getAgentCard(); + try { + const factory = new ClientFactory(options); + const client = await factory.createFromUrl(agentCardUrl, ''); + const agentCard = await client.getAgentCard(); - this.clients.set(name, client); - this.agentCards.set(name, agentCard); + this.clients.set(name, client); + this.agentCards.set(name, agentCard); - debugLogger.debug( - `[A2AClientManager] Loaded agent '${name}' from ${agentCardUrl}`, - ); + debugLogger.debug( + `[A2AClientManager] Loaded agent '${name}' from ${agentCardUrl}`, + ); - return agentCard; + return agentCard; + } catch (error: unknown) { + throw classifyAgentError(name, agentCardUrl, error); + } } /** @@ -183,19 +188,9 @@ export class A2AClientManager { }, }; - try { - yield* client.sendMessageStream(messageParams, { - signal: options?.signal, - }); - } catch (error: unknown) { - const prefix = `[A2AClientManager] sendMessageStream Error [${agentName}]`; - if (error instanceof Error) { - throw new Error(`${prefix}: ${error.message}`, { cause: error }); - } - throw new Error( - `${prefix}: Unexpected error during sendMessageStream: ${String(error)}`, - ); - } + yield* client.sendMessageStream(messageParams, { + signal: options?.signal, + }); } /** diff --git a/packages/core/src/agents/a2a-errors.test.ts b/packages/core/src/agents/a2a-errors.test.ts new file mode 100644 index 0000000000..e5039c5727 --- /dev/null +++ b/packages/core/src/agents/a2a-errors.test.ts @@ -0,0 +1,298 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { + A2AAgentError, + AgentCardNotFoundError, + AgentCardAuthError, + AgentAuthConfigMissingError, + AgentConnectionError, + classifyAgentError, +} from './a2a-errors.js'; + +describe('A2A Error Types', () => { + describe('A2AAgentError', () => { + it('should set name, agentName, and userMessage', () => { + const error = new A2AAgentError('my-agent', 'internal msg', 'user msg'); + expect(error.name).toBe('A2AAgentError'); + expect(error.agentName).toBe('my-agent'); + expect(error.message).toBe('internal msg'); + expect(error.userMessage).toBe('user msg'); + }); + }); + + describe('AgentCardNotFoundError', () => { + it('should produce a user-friendly 404 message', () => { + const error = new AgentCardNotFoundError( + 'my-agent', + 'https://example.com/card', + ); + expect(error.name).toBe('AgentCardNotFoundError'); + expect(error.agentName).toBe('my-agent'); + expect(error.userMessage).toContain('404'); + expect(error.userMessage).toContain('https://example.com/card'); + expect(error.userMessage).toContain('agent_card_url'); + }); + }); + + describe('AgentCardAuthError', () => { + it('should produce a user-friendly 401 message', () => { + const error = new AgentCardAuthError( + 'secure-agent', + 'https://example.com/card', + 401, + ); + expect(error.name).toBe('AgentCardAuthError'); + expect(error.statusCode).toBe(401); + expect(error.userMessage).toContain('401'); + expect(error.userMessage).toContain('Unauthorized'); + expect(error.userMessage).toContain('"auth" configuration'); + }); + + it('should produce a user-friendly 403 message', () => { + const error = new AgentCardAuthError( + 'secure-agent', + 'https://example.com/card', + 403, + ); + expect(error.statusCode).toBe(403); + expect(error.userMessage).toContain('403'); + expect(error.userMessage).toContain('Forbidden'); + }); + }); + + describe('AgentAuthConfigMissingError', () => { + it('should list missing config fields', () => { + const error = new AgentAuthConfigMissingError( + 'api-agent', + 'API Key (x-api-key): Send x-api-key in header', + [ + 'Authentication is required but not configured', + "Scheme 'api_key' requires apiKey authentication", + ], + ); + expect(error.name).toBe('AgentAuthConfigMissingError'); + expect(error.requiredAuth).toContain('API Key'); + expect(error.missingFields).toHaveLength(2); + expect(error.userMessage).toContain('API Key'); + expect(error.userMessage).toContain('no auth is configured'); + expect(error.userMessage).toContain('Missing:'); + }); + }); + + describe('AgentConnectionError', () => { + it('should wrap the original error cause', () => { + const cause = new Error('ECONNREFUSED'); + const error = new AgentConnectionError( + 'my-agent', + 'https://example.com/card', + cause, + ); + expect(error.name).toBe('AgentConnectionError'); + expect(error.userMessage).toContain('ECONNREFUSED'); + expect(error.userMessage).toContain('https://example.com/card'); + }); + + it('should handle non-Error causes', () => { + const error = new AgentConnectionError( + 'my-agent', + 'https://example.com/card', + 'raw string error', + ); + expect(error.userMessage).toContain('raw string error'); + }); + }); + + describe('classifyAgentError', () => { + it('should classify a 404 error message', () => { + const raw = new Error('HTTP 404: Not Found'); + const result = classifyAgentError( + 'agent-a', + 'https://example.com/card', + raw, + ); + expect(result).toBeInstanceOf(AgentCardNotFoundError); + expect(result.agentName).toBe('agent-a'); + }); + + it('should classify a "not found" error message (case-insensitive)', () => { + const raw = new Error('Agent card not found at the given URL'); + const result = classifyAgentError( + 'agent-a', + 'https://example.com/card', + raw, + ); + expect(result).toBeInstanceOf(AgentCardNotFoundError); + }); + + it('should classify a 401 error message', () => { + const raw = new Error('Request failed with status 401'); + const result = classifyAgentError( + 'agent-b', + 'https://example.com/card', + raw, + ); + expect(result).toBeInstanceOf(AgentCardAuthError); + expect((result as AgentCardAuthError).statusCode).toBe(401); + }); + + it('should classify an "unauthorized" error message', () => { + const raw = new Error('Unauthorized access to agent card'); + const result = classifyAgentError( + 'agent-b', + 'https://example.com/card', + raw, + ); + expect(result).toBeInstanceOf(AgentCardAuthError); + }); + + it('should classify a 403 error message', () => { + const raw = new Error('HTTP 403 Forbidden'); + const result = classifyAgentError( + 'agent-c', + 'https://example.com/card', + raw, + ); + expect(result).toBeInstanceOf(AgentCardAuthError); + expect((result as AgentCardAuthError).statusCode).toBe(403); + }); + + it('should fall back to AgentConnectionError for unknown errors', () => { + const raw = new Error('Something completely unexpected'); + const result = classifyAgentError( + 'agent-d', + 'https://example.com/card', + raw, + ); + expect(result).toBeInstanceOf(AgentConnectionError); + }); + + it('should classify ECONNREFUSED as AgentConnectionError', () => { + const raw = new Error('ECONNREFUSED 127.0.0.1:8080'); + const result = classifyAgentError( + 'agent-d', + 'https://example.com/card', + raw, + ); + expect(result).toBeInstanceOf(AgentConnectionError); + }); + + it('should handle non-Error values', () => { + const result = classifyAgentError( + 'agent-e', + 'https://example.com/card', + 'some string error', + ); + expect(result).toBeInstanceOf(AgentConnectionError); + }); + + describe('cause chain inspection', () => { + it('should detect 404 in a nested cause', () => { + const inner = new Error('HTTP 404 Not Found'); + const outer = new Error('fetch failed', { cause: inner }); + const result = classifyAgentError( + 'agent-nested', + 'https://example.com/card', + outer, + ); + expect(result).toBeInstanceOf(AgentCardNotFoundError); + }); + + it('should detect 401 in a deeply nested cause', () => { + const innermost = new Error('Server returned 401'); + const middle = new Error('Request error', { cause: innermost }); + const outer = new Error('fetch failed', { cause: middle }); + const result = classifyAgentError( + 'agent-deep', + 'https://example.com/card', + outer, + ); + expect(result).toBeInstanceOf(AgentCardAuthError); + expect((result as AgentCardAuthError).statusCode).toBe(401); + }); + + it('should detect ECONNREFUSED error code in cause chain', () => { + const inner = Object.assign(new Error('connect failed'), { + code: 'ECONNREFUSED', + }); + const outer = new Error('fetch failed', { cause: inner }); + const result = classifyAgentError( + 'agent-conn', + 'https://example.com/card', + outer, + ); + expect(result).toBeInstanceOf(AgentConnectionError); + }); + + it('should detect status property on error objects in cause chain', () => { + const inner = Object.assign(new Error('Bad response'), { + status: 403, + }); + const outer = new Error('agent card resolution failed', { + cause: inner, + }); + const result = classifyAgentError( + 'agent-status', + 'https://example.com/card', + outer, + ); + expect(result).toBeInstanceOf(AgentCardAuthError); + expect((result as AgentCardAuthError).statusCode).toBe(403); + }); + + it('should detect status on a plain-object cause (non-Error)', () => { + const outer = new Error('fetch failed'); + // Some HTTP libs set cause to a plain object, not an Error instance + (outer as unknown as { cause: unknown }).cause = { + message: 'Unauthorized', + status: 401, + }; + const result = classifyAgentError( + 'agent-plain-cause', + 'https://example.com/card', + outer, + ); + expect(result).toBeInstanceOf(AgentCardAuthError); + expect((result as AgentCardAuthError).statusCode).toBe(401); + }); + + it('should detect statusCode on a plain-object cause (non-Error)', () => { + const outer = new Error('fetch failed'); + (outer as unknown as { cause: unknown }).cause = { + message: 'Forbidden', + statusCode: 403, + }; + const result = classifyAgentError( + 'agent-plain-cause-403', + 'https://example.com/card', + outer, + ); + expect(result).toBeInstanceOf(AgentCardAuthError); + expect((result as AgentCardAuthError).statusCode).toBe(403); + }); + + it('should classify ENOTFOUND as AgentConnectionError, not 404', () => { + // ENOTFOUND (DNS resolution failure) should NOT be misclassified + // as a 404 despite containing "NOTFOUND" in the error code. + const inner = Object.assign( + new Error('getaddrinfo ENOTFOUND example.invalid'), + { + code: 'ENOTFOUND', + }, + ); + const outer = new Error('fetch failed', { cause: inner }); + const result = classifyAgentError( + 'agent-dns', + 'https://example.invalid/card', + outer, + ); + expect(result).toBeInstanceOf(AgentConnectionError); + expect(result).not.toBeInstanceOf(AgentCardNotFoundError); + }); + }); + }); +}); diff --git a/packages/core/src/agents/a2a-errors.ts b/packages/core/src/agents/a2a-errors.ts new file mode 100644 index 0000000000..4023d5fdbd --- /dev/null +++ b/packages/core/src/agents/a2a-errors.ts @@ -0,0 +1,206 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @fileoverview Custom error types for A2A remote agent operations. + * Provides structured, user-friendly error messages for common failure modes + * during agent card fetching, authentication, and communication. + */ + +/** + * Base class for all A2A agent errors. + * Provides a `userMessage` field with a human-readable description. + */ +export class A2AAgentError extends Error { + /** A user-friendly message suitable for display in the CLI. */ + readonly userMessage: string; + /** The agent name associated with this error. */ + readonly agentName: string; + + constructor( + agentName: string, + message: string, + userMessage: string, + options?: ErrorOptions, + ) { + super(message, options); + this.name = 'A2AAgentError'; + this.agentName = agentName; + this.userMessage = userMessage; + } +} + +/** + * Thrown when the agent card URL returns a 404 Not Found response. + */ +export class AgentCardNotFoundError extends A2AAgentError { + constructor(agentName: string, agentCardUrl: string) { + const message = `Agent card not found at ${agentCardUrl} (HTTP 404)`; + const userMessage = `Agent card not found (404) at ${agentCardUrl}. Verify the agent_card_url in your agent definition.`; + super(agentName, message, userMessage); + this.name = 'AgentCardNotFoundError'; + } +} + +/** + * Thrown when the agent card URL returns a 401/403 response, + * indicating an authentication or authorization failure. + */ +export class AgentCardAuthError extends A2AAgentError { + readonly statusCode: number; + + constructor(agentName: string, agentCardUrl: string, statusCode: 401 | 403) { + const statusText = statusCode === 401 ? 'Unauthorized' : 'Forbidden'; + const message = `Agent card request returned ${statusCode} ${statusText} for ${agentCardUrl}`; + const userMessage = `Authentication failed (${statusCode} ${statusText}) at ${agentCardUrl}. Check the "auth" configuration in your agent definition.`; + super(agentName, message, userMessage); + this.name = 'AgentCardAuthError'; + this.statusCode = statusCode; + } +} + +/** + * Thrown when the agent card's security schemes require authentication + * but the agent definition does not include the necessary auth configuration. + */ +export class AgentAuthConfigMissingError extends A2AAgentError { + /** Human-readable description of required authentication schemes. */ + readonly requiredAuth: string; + /** Specific fields or config entries that are missing. */ + readonly missingFields: string[]; + + constructor( + agentName: string, + requiredAuth: string, + missingFields: string[], + ) { + const message = `Agent "${agentName}" requires authentication but none is configured`; + const userMessage = `Agent requires ${requiredAuth} but no auth is configured. Missing: ${missingFields.join(', ')}`; + super(agentName, message, userMessage); + this.name = 'AgentAuthConfigMissingError'; + this.requiredAuth = requiredAuth; + this.missingFields = missingFields; + } +} + +/** + * Thrown when a generic/unexpected network or server error occurs + * while fetching the agent card or communicating with the remote agent. + */ +export class AgentConnectionError extends A2AAgentError { + constructor(agentName: string, agentCardUrl: string, cause: unknown) { + const causeMessage = cause instanceof Error ? cause.message : String(cause); + const message = `Failed to connect to agent "${agentName}" at ${agentCardUrl}: ${causeMessage}`; + const userMessage = `Connection failed for ${agentCardUrl}: ${causeMessage}`; + super(agentName, message, userMessage, { cause }); + this.name = 'AgentConnectionError'; + } +} + +/** Shape of an error-like object in a cause chain (Error, HTTP response, or plain object). */ +interface ErrorLikeObject { + message?: string; + code?: string; + status?: number; + statusCode?: number; + cause?: unknown; +} + +/** Type guard for objects that may carry error metadata (message, code, status, cause). */ +function isErrorLikeObject(val: unknown): val is ErrorLikeObject { + return typeof val === 'object' && val !== null; +} + +/** + * Collects all error messages from an error's cause chain into a single string + * for pattern matching. This is necessary because the A2A SDK and Node's fetch + * often wrap the real error (e.g. HTTP status) deep inside nested causes. + */ +function collectErrorMessages(error: unknown): string { + const parts: string[] = []; + let current: unknown = error; + let depth = 0; + const maxDepth = 10; + + while (current && depth < maxDepth) { + if (isErrorLikeObject(current)) { + // Save reference before instanceof narrows the type from ErrorLikeObject to Error. + const obj = current; + + if (current instanceof Error) { + parts.push(current.message); + } else if (typeof obj.message === 'string') { + parts.push(obj.message); + } + + if (typeof obj.code === 'string') { + parts.push(obj.code); + } + + if (typeof obj.status === 'number') { + parts.push(String(obj.status)); + } else if (typeof obj.statusCode === 'number') { + parts.push(String(obj.statusCode)); + } + + current = obj.cause; + } else if (typeof current === 'string') { + parts.push(current); + break; + } else { + parts.push(String(current)); + break; + } + depth++; + } + + return parts.join(' '); +} + +/** + * Attempts to classify a raw error from the A2A SDK into a typed A2AAgentError. + * + * Inspects the error message and full cause chain for HTTP status codes and + * well-known patterns to produce a structured, user-friendly error. + * + * @param agentName The name of the agent being loaded. + * @param agentCardUrl The URL of the agent card. + * @param error The raw error caught during agent loading. + * @returns A classified A2AAgentError subclass. + */ +export function classifyAgentError( + agentName: string, + agentCardUrl: string, + error: unknown, +): A2AAgentError { + // Collect messages from the entire cause chain for thorough matching. + const fullErrorText = collectErrorMessages(error); + + // Check for well-known connection error codes in the cause chain. + // NOTE: This is checked before the 404 pattern as a defensive measure + // to prevent DNS errors (ENOTFOUND) from being misclassified as 404s. + if ( + /\b(ECONNREFUSED|ENOTFOUND|EHOSTUNREACH|ETIMEDOUT)\b/i.test(fullErrorText) + ) { + return new AgentConnectionError(agentName, agentCardUrl, error); + } + + // Check for HTTP status code patterns across the full cause chain. + if (/\b404\b|\bnot[\s_-]?found\b/i.test(fullErrorText)) { + return new AgentCardNotFoundError(agentName, agentCardUrl); + } + + if (/\b401\b|unauthorized/i.test(fullErrorText)) { + return new AgentCardAuthError(agentName, agentCardUrl, 401); + } + + if (/\b403\b|forbidden/i.test(fullErrorText)) { + return new AgentCardAuthError(agentName, agentCardUrl, 403); + } + + // Fallback to a generic connection error. + return new AgentConnectionError(agentName, agentCardUrl, error); +} diff --git a/packages/core/src/agents/registry.test.ts b/packages/core/src/agents/registry.test.ts index 8dde75cf7f..9ac2ec0cf9 100644 --- a/packages/core/src/agents/registry.test.ts +++ b/packages/core/src/agents/registry.test.ts @@ -48,6 +48,8 @@ vi.mock('./a2a-client-manager.js', () => ({ vi.mock('./auth-provider/factory.js', () => ({ A2AAuthProviderFactory: { create: vi.fn(), + validateAuthConfig: vi.fn().mockReturnValue({ valid: true }), + describeRequiredAuth: vi.fn().mockReturnValue('API key required'), }, })); @@ -665,6 +667,111 @@ describe('AgentRegistry', () => { ); }); + it('should emit error feedback with userMessage when A2AAgentError is thrown', async () => { + const { AgentConnectionError } = await import('./a2a-errors.js'); + const feedbackSpy = vi + .spyOn(coreEvents, 'emitFeedback') + .mockImplementation(() => {}); + + const remoteAgent: AgentDefinition = { + kind: 'remote', + name: 'FailAgent', + description: 'An agent that fails to load', + agentCardUrl: 'https://unreachable.example.com/card', + inputConfig: { inputSchema: { type: 'object' } }, + }; + + const a2aError = new AgentConnectionError( + 'FailAgent', + 'https://unreachable.example.com/card', + new Error('ECONNREFUSED'), + ); + + vi.mocked(A2AClientManager.getInstance).mockReturnValue({ + loadAgent: vi.fn().mockRejectedValue(a2aError), + } as unknown as A2AClientManager); + + await registry.testRegisterAgent(remoteAgent); + + expect(feedbackSpy).toHaveBeenCalledWith( + 'error', + `[FailAgent] ${a2aError.userMessage}`, + ); + expect(registry.getDefinition('FailAgent')).toBeUndefined(); + }); + + it('should emit generic error feedback for non-A2AAgentError failures', async () => { + const feedbackSpy = vi + .spyOn(coreEvents, 'emitFeedback') + .mockImplementation(() => {}); + + const remoteAgent: AgentDefinition = { + kind: 'remote', + name: 'FailAgent', + description: 'An agent that fails', + agentCardUrl: 'https://example.com/card', + inputConfig: { inputSchema: { type: 'object' } }, + }; + + vi.mocked(A2AClientManager.getInstance).mockReturnValue({ + loadAgent: vi.fn().mockRejectedValue(new Error('unexpected crash')), + } as unknown as A2AClientManager); + + await registry.testRegisterAgent(remoteAgent); + + expect(feedbackSpy).toHaveBeenCalledWith( + 'error', + '[FailAgent] Failed to load remote agent: unexpected crash', + ); + expect(registry.getDefinition('FailAgent')).toBeUndefined(); + }); + + it('should emit warning feedback when auth config is missing for secured agent', async () => { + const feedbackSpy = vi + .spyOn(coreEvents, 'emitFeedback') + .mockImplementation(() => {}); + + vi.mocked(A2AAuthProviderFactory.validateAuthConfig).mockReturnValue({ + valid: false, + diff: { requiredSchemes: ['api_key'], missingConfig: ['api_key'] }, + }); + vi.mocked(A2AAuthProviderFactory.describeRequiredAuth).mockReturnValue( + 'apiKey (header: x-api-key)', + ); + + const remoteAgent: AgentDefinition = { + kind: 'remote', + name: 'SecuredAgent', + description: 'A secured remote agent', + agentCardUrl: 'https://example.com/card', + inputConfig: { inputSchema: { type: 'object' } }, + // No auth configured + }; + + vi.mocked(A2AClientManager.getInstance).mockReturnValue({ + loadAgent: vi.fn().mockResolvedValue({ + name: 'SecuredAgent', + securitySchemes: { + api_key: { + type: 'apiKey', + in: 'header', + name: 'x-api-key', + }, + }, + }), + } as unknown as A2AClientManager); + + await registry.testRegisterAgent(remoteAgent); + + // Agent should still be registered (ADC fallback) + expect(registry.getDefinition('SecuredAgent')).toBeDefined(); + // But a warning should have been emitted + expect(feedbackSpy).toHaveBeenCalledWith( + 'warning', + expect.stringContaining('SecuredAgent'), + ); + }); + it('should surface an error if remote agent registration fails', async () => { const remoteAgent: AgentDefinition = { kind: 'remote', @@ -685,7 +792,7 @@ describe('AgentRegistry', () => { expect(feedbackSpy).toHaveBeenCalledWith( 'error', - `Error loading A2A agent "FailingRemoteAgent": 401 Unauthorized`, + `[FailingRemoteAgent] Failed to load remote agent: 401 Unauthorized`, ); }); diff --git a/packages/core/src/agents/registry.ts b/packages/core/src/agents/registry.ts index f9a078c1b7..c4b08eba22 100644 --- a/packages/core/src/agents/registry.ts +++ b/packages/core/src/agents/registry.ts @@ -24,6 +24,7 @@ import { ModelConfigService, } from '../services/modelConfigService.js'; import { PolicyDecision, PRIORITY_SUBAGENT_TOOL } from '../policy/types.js'; +import { A2AAgentError, AgentAuthConfigMissingError } from './a2a-errors.js'; /** * Returns the model config alias for a given agent definition. @@ -366,6 +367,9 @@ export class AgentRegistry { /** * Registers a remote agent definition asynchronously. + * Provides robust error handling with user-friendly messages for: + * - Agent card fetch failures (404, 401/403, network errors) + * - Missing authentication configuration */ protected async registerRemoteAgent( definition: AgentDefinition, @@ -408,7 +412,7 @@ export class AgentRegistry { remoteDef.originalDescription = remoteDef.description; } - // Log remote A2A agent registration for visibility. + // Load the remote A2A agent card and register. try { const clientManager = A2AClientManager.getInstance(); let authHandler: AuthenticationHandler | undefined; @@ -432,6 +436,30 @@ export class AgentRegistry { authHandler, ); + // Validate auth configuration against the agent card's security schemes. + if (agentCard.securitySchemes) { + const validation = A2AAuthProviderFactory.validateAuthConfig( + definition.auth, + agentCard.securitySchemes, + ); + if (!validation.valid && validation.diff) { + const requiredAuth = A2AAuthProviderFactory.describeRequiredAuth( + agentCard.securitySchemes, + ); + const authError = new AgentAuthConfigMissingError( + definition.name, + requiredAuth, + validation.diff.missingConfig, + ); + coreEvents.emitFeedback( + 'warning', + `[${definition.name}] Agent requires authentication: ${requiredAuth}`, + ); + debugLogger.warn(`[AgentRegistry] ${authError.message}`); + // Still register the agent — the user can fix config and retry. + } + } + const userDescription = remoteDef.originalDescription; const agentDescription = agentCard.description; const descriptions: string[] = []; @@ -464,9 +492,22 @@ export class AgentRegistry { this.agents.set(definition.name, definition); this.addAgentPolicy(definition); } catch (e) { - const errorMessage = `Error loading A2A agent "${definition.name}": ${e instanceof Error ? e.message : String(e)}`; - debugLogger.warn(`[AgentRegistry] ${errorMessage}`, e); - coreEvents.emitFeedback('error', errorMessage); + // Surface structured, user-friendly error messages for known failure modes. + if (e instanceof A2AAgentError) { + coreEvents.emitFeedback( + 'error', + `[${definition.name}] ${e.userMessage}`, + ); + } else { + coreEvents.emitFeedback( + 'error', + `[${definition.name}] Failed to load remote agent: ${e instanceof Error ? e.message : String(e)}`, + ); + } + debugLogger.warn( + `[AgentRegistry] Error loading A2A agent "${definition.name}":`, + e, + ); } } diff --git a/packages/core/src/agents/remote-invocation.test.ts b/packages/core/src/agents/remote-invocation.test.ts index d295373fb0..e870090a31 100644 --- a/packages/core/src/agents/remote-invocation.test.ts +++ b/packages/core/src/agents/remote-invocation.test.ts @@ -613,4 +613,75 @@ describe('RemoteAgentInvocation', () => { } }); }); + + describe('Error Handling', () => { + it('should use A2AAgentError.userMessage for structured errors', async () => { + const { AgentConnectionError } = await import('./a2a-errors.js'); + const a2aError = new AgentConnectionError( + 'test-agent', + 'http://test-agent/card', + new Error('ECONNREFUSED'), + ); + + mockClientManager.getClient.mockReturnValue(undefined); + mockClientManager.loadAgent.mockRejectedValue(a2aError); + + const invocation = new RemoteAgentInvocation( + mockDefinition, + { query: 'hi' }, + mockMessageBus, + ); + const result = await invocation.execute(new AbortController().signal); + + expect(result.error).toBeDefined(); + expect(result.returnDisplay).toContain(a2aError.userMessage); + }); + + it('should use generic message for non-A2AAgentError errors', async () => { + mockClientManager.getClient.mockReturnValue(undefined); + mockClientManager.loadAgent.mockRejectedValue( + new Error('something unexpected'), + ); + + const invocation = new RemoteAgentInvocation( + mockDefinition, + { query: 'hi' }, + mockMessageBus, + ); + const result = await invocation.execute(new AbortController().signal); + + expect(result.error).toBeDefined(); + expect(result.returnDisplay).toContain( + 'Error calling remote agent: something unexpected', + ); + }); + + it('should include partial output when error occurs mid-stream', async () => { + mockClientManager.getClient.mockReturnValue({}); + mockClientManager.sendMessageStream.mockImplementation( + async function* () { + yield { + kind: 'message', + messageId: 'msg-1', + role: 'agent', + parts: [{ kind: 'text', text: 'Partial response' }], + }; + // Raw errors propagate from the A2A SDK — no wrapping or classification. + throw new Error('connection reset'); + }, + ); + + const invocation = new RemoteAgentInvocation( + mockDefinition, + { query: 'hi' }, + mockMessageBus, + ); + const result = await invocation.execute(new AbortController().signal); + + expect(result.error).toBeDefined(); + // Should contain both the partial output and the error message + expect(result.returnDisplay).toContain('Partial response'); + expect(result.returnDisplay).toContain('connection reset'); + }); + }); }); diff --git a/packages/core/src/agents/remote-invocation.ts b/packages/core/src/agents/remote-invocation.ts index 4deb14d081..fe1e3cd077 100644 --- a/packages/core/src/agents/remote-invocation.ts +++ b/packages/core/src/agents/remote-invocation.ts @@ -28,6 +28,7 @@ import { debugLogger } from '../utils/debugLogger.js'; import { safeJsonToMarkdown } from '../utils/markdownUtils.js'; import type { AnsiOutput } from '../utils/terminalSerializer.js'; import { A2AAuthProviderFactory } from './auth-provider/factory.js'; +import { A2AAgentError } from './a2a-errors.js'; /** * Authentication handler implementation using Google Application Default Credentials (ADC). @@ -228,7 +229,8 @@ export class RemoteAgentInvocation extends BaseToolInvocation< }; } catch (error: unknown) { const partialOutput = reassembler.toString(); - const errorMessage = `Error calling remote agent: ${error instanceof Error ? error.message : String(error)}`; + // Surface structured, user-friendly error messages. + const errorMessage = this.formatExecutionError(error); const fullDisplay = partialOutput ? `${partialOutput}\n\n${errorMessage}` : errorMessage; @@ -245,4 +247,22 @@ export class RemoteAgentInvocation extends BaseToolInvocation< }); } } + + /** + * Formats an execution error into a user-friendly message. + * Recognizes typed A2AAgentError subclasses and falls back to + * a generic message for unknown errors. + */ + private formatExecutionError(error: unknown): string { + // All A2A-specific errors include a human-friendly `userMessage` on the + // A2AAgentError base class. Rely on that to avoid duplicating messages + // for specific subclasses, which improves maintainability. + if (error instanceof A2AAgentError) { + return error.userMessage; + } + + return `Error calling remote agent: ${ + error instanceof Error ? error.message : String(error) + }`; + } }