From 0e7944df4f3afbd3ea6b0b9c853931c4bde6475a Mon Sep 17 00:00:00 2001 From: Bryan Morgan Date: Tue, 3 Feb 2026 11:32:20 -0500 Subject: [PATCH] refactor: localize ACP error parsing logic to cli package (#18193) --- .../cli/src/zed-integration/acpErrors.test.ts | 45 +++++++++++++++++++ packages/cli/src/zed-integration/acpErrors.ts | 42 +++++++++++++++++ .../cli/src/zed-integration/zedIntegration.ts | 10 +++-- packages/core/src/utils/errors.test.ts | 31 ------------- packages/core/src/utils/errors.ts | 35 ++------------- 5 files changed, 97 insertions(+), 66 deletions(-) create mode 100644 packages/cli/src/zed-integration/acpErrors.test.ts create mode 100644 packages/cli/src/zed-integration/acpErrors.ts diff --git a/packages/cli/src/zed-integration/acpErrors.test.ts b/packages/cli/src/zed-integration/acpErrors.test.ts new file mode 100644 index 0000000000..2ea4d528d0 --- /dev/null +++ b/packages/cli/src/zed-integration/acpErrors.test.ts @@ -0,0 +1,45 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { getAcpErrorMessage } from './acpErrors.js'; + +describe('getAcpErrorMessage', () => { + it('should return plain error message', () => { + expect(getAcpErrorMessage(new Error('plain error'))).toBe('plain error'); + }); + + it('should parse simple JSON error response', () => { + const json = JSON.stringify({ error: { message: 'json error' } }); + expect(getAcpErrorMessage(new Error(json))).toBe('json error'); + }); + + it('should parse double-encoded JSON error response', () => { + const innerJson = JSON.stringify({ error: { message: 'nested error' } }); + const outerJson = JSON.stringify({ error: { message: innerJson } }); + expect(getAcpErrorMessage(new Error(outerJson))).toBe('nested error'); + }); + + it('should parse array-style JSON error response', () => { + const json = JSON.stringify([{ error: { message: 'array error' } }]); + expect(getAcpErrorMessage(new Error(json))).toBe('array error'); + }); + + it('should parse JSON with top-level message field', () => { + const json = JSON.stringify({ message: 'top-level message' }); + expect(getAcpErrorMessage(new Error(json))).toBe('top-level message'); + }); + + it('should handle JSON with trailing newline', () => { + const json = JSON.stringify({ error: { message: 'newline error' } }) + '\n'; + expect(getAcpErrorMessage(new Error(json))).toBe('newline error'); + }); + + it('should return original message if JSON parsing fails', () => { + const invalidJson = '{ not-json }'; + expect(getAcpErrorMessage(new Error(invalidJson))).toBe(invalidJson); + }); +}); diff --git a/packages/cli/src/zed-integration/acpErrors.ts b/packages/cli/src/zed-integration/acpErrors.ts new file mode 100644 index 0000000000..2e111b2876 --- /dev/null +++ b/packages/cli/src/zed-integration/acpErrors.ts @@ -0,0 +1,42 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { getErrorMessage as getCoreErrorMessage } from '@google/gemini-cli-core'; + +/** + * Extracts a human-readable error message specifically for ACP (IDE) clients. + * This function recursively parses JSON error blobs that are common in + * Google API responses but ugly to display in an IDE's UI. + */ +export function getAcpErrorMessage(error: unknown): string { + const coreMessage = getCoreErrorMessage(error); + return extractRecursiveMessage(coreMessage); +} + +function extractRecursiveMessage(input: string): string { + const trimmed = input.trim(); + + // Attempt to parse JSON error responses (common in Google API errors) + if ( + (trimmed.startsWith('{') && trimmed.endsWith('}')) || + (trimmed.startsWith('[') && trimmed.endsWith(']')) + ) { + try { + const parsed = JSON.parse(trimmed); + const next = + parsed?.error?.message || + parsed?.[0]?.error?.message || + parsed?.message; + + if (next && typeof next === 'string' && next !== input) { + return extractRecursiveMessage(next); + } + } catch { + // Fall back to original string if parsing fails + } + } + return input; +} diff --git a/packages/cli/src/zed-integration/zedIntegration.ts b/packages/cli/src/zed-integration/zedIntegration.ts index e48e3dbf01..634c20a1a0 100644 --- a/packages/cli/src/zed-integration/zedIntegration.ts +++ b/packages/cli/src/zed-integration/zedIntegration.ts @@ -37,6 +37,7 @@ import { } from '@google/gemini-cli-core'; import * as acp from '@agentclientprotocol/sdk'; import { AcpFileSystemService } from './fileSystemService.js'; +import { getAcpErrorMessage } from './acpErrors.js'; import { Readable, Writable } from 'node:stream'; import type { Content, Part, FunctionCall } from '@google/genai'; import type { LoadedSettings } from '../config/settings.js'; @@ -142,7 +143,10 @@ export class GeminiAgent { try { await this.config.refreshAuth(method); } catch (e) { - throw new acp.RequestError(getErrorStatus(e) || 401, getErrorMessage(e)); + throw new acp.RequestError( + getErrorStatus(e) || 401, + getAcpErrorMessage(e), + ); } this.settings.setValue( SettingScope.User, @@ -184,7 +188,7 @@ export class GeminiAgent { } } catch (e) { isAuthenticated = false; - authErrorMessage = getErrorMessage(e); + authErrorMessage = getAcpErrorMessage(e); debugLogger.error( `Authentication failed: ${e instanceof Error ? e.stack : e}`, ); @@ -546,7 +550,7 @@ export class Session { throw new acp.RequestError( getErrorStatus(error) || 500, - getErrorMessage(error), + getAcpErrorMessage(error), ); } diff --git a/packages/core/src/utils/errors.test.ts b/packages/core/src/utils/errors.test.ts index e94805ea27..58c7004190 100644 --- a/packages/core/src/utils/errors.test.ts +++ b/packages/core/src/utils/errors.test.ts @@ -19,37 +19,6 @@ describe('getErrorMessage', () => { expect(getErrorMessage(new Error('plain error'))).toBe('plain error'); }); - it('should parse simple JSON error response', () => { - const json = JSON.stringify({ error: { message: 'json error' } }); - expect(getErrorMessage(new Error(json))).toBe('json error'); - }); - - it('should parse double-encoded JSON error response', () => { - const innerJson = JSON.stringify({ error: { message: 'nested error' } }); - const outerJson = JSON.stringify({ error: { message: innerJson } }); - expect(getErrorMessage(new Error(outerJson))).toBe('nested error'); - }); - - it('should parse array-style JSON error response', () => { - const json = JSON.stringify([{ error: { message: 'array error' } }]); - expect(getErrorMessage(new Error(json))).toBe('array error'); - }); - - it('should parse JSON with top-level message field', () => { - const json = JSON.stringify({ message: 'top-level message' }); - expect(getErrorMessage(new Error(json))).toBe('top-level message'); - }); - - it('should handle JSON with trailing newline', () => { - const json = JSON.stringify({ error: { message: 'newline error' } }) + '\n'; - expect(getErrorMessage(new Error(json))).toBe('newline error'); - }); - - it('should return original message if JSON parsing fails', () => { - const invalidJson = '{ not-json }'; - expect(getErrorMessage(new Error(invalidJson))).toBe(invalidJson); - }); - it('should handle non-Error inputs', () => { expect(getErrorMessage('string error')).toBe('string error'); expect(getErrorMessage(123)).toBe('123'); diff --git a/packages/core/src/utils/errors.ts b/packages/core/src/utils/errors.ts index f833d9aad6..bd6512e04b 100644 --- a/packages/core/src/utils/errors.ts +++ b/packages/core/src/utils/errors.ts @@ -16,40 +16,11 @@ export function isNodeError(error: unknown): error is NodeJS.ErrnoException { export function getErrorMessage(error: unknown): string { const friendlyError = toFriendlyError(error); - return extractMessage(friendlyError); -} - -function extractMessage(input: unknown): string { - if (input instanceof Error) { - return extractMessage(input.message); + if (friendlyError instanceof Error) { + return friendlyError.message; } - - if (typeof input === 'string') { - const trimmed = input.trim(); - // Attempt to parse JSON error responses (common in Google API errors) - if ( - (trimmed.startsWith('{') && trimmed.endsWith('}')) || - (trimmed.startsWith('[') && trimmed.endsWith(']')) - ) { - try { - const parsed = JSON.parse(trimmed); - const next = - parsed?.error?.message || - parsed?.[0]?.error?.message || - parsed?.message; - - if (next && next !== input) { - return extractMessage(next); - } - } catch { - // Fall back to original string if parsing fails - } - } - return input; - } - try { - return String(input); + return String(friendlyError); } catch { return 'Failed to get error details'; }