Skip to content

Commit f3da8a3

Browse files
bdmorganAdib234
authored andcommitted
refactor: localize ACP error parsing logic to cli package (#18193)
1 parent fec5426 commit f3da8a3

5 files changed

Lines changed: 97 additions & 66 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* @license
3+
* Copyright 2025 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import { describe, it, expect } from 'vitest';
8+
import { getAcpErrorMessage } from './acpErrors.js';
9+
10+
describe('getAcpErrorMessage', () => {
11+
it('should return plain error message', () => {
12+
expect(getAcpErrorMessage(new Error('plain error'))).toBe('plain error');
13+
});
14+
15+
it('should parse simple JSON error response', () => {
16+
const json = JSON.stringify({ error: { message: 'json error' } });
17+
expect(getAcpErrorMessage(new Error(json))).toBe('json error');
18+
});
19+
20+
it('should parse double-encoded JSON error response', () => {
21+
const innerJson = JSON.stringify({ error: { message: 'nested error' } });
22+
const outerJson = JSON.stringify({ error: { message: innerJson } });
23+
expect(getAcpErrorMessage(new Error(outerJson))).toBe('nested error');
24+
});
25+
26+
it('should parse array-style JSON error response', () => {
27+
const json = JSON.stringify([{ error: { message: 'array error' } }]);
28+
expect(getAcpErrorMessage(new Error(json))).toBe('array error');
29+
});
30+
31+
it('should parse JSON with top-level message field', () => {
32+
const json = JSON.stringify({ message: 'top-level message' });
33+
expect(getAcpErrorMessage(new Error(json))).toBe('top-level message');
34+
});
35+
36+
it('should handle JSON with trailing newline', () => {
37+
const json = JSON.stringify({ error: { message: 'newline error' } }) + '\n';
38+
expect(getAcpErrorMessage(new Error(json))).toBe('newline error');
39+
});
40+
41+
it('should return original message if JSON parsing fails', () => {
42+
const invalidJson = '{ not-json }';
43+
expect(getAcpErrorMessage(new Error(invalidJson))).toBe(invalidJson);
44+
});
45+
});
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* @license
3+
* Copyright 2025 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import { getErrorMessage as getCoreErrorMessage } from '@google/gemini-cli-core';
8+
9+
/**
10+
* Extracts a human-readable error message specifically for ACP (IDE) clients.
11+
* This function recursively parses JSON error blobs that are common in
12+
* Google API responses but ugly to display in an IDE's UI.
13+
*/
14+
export function getAcpErrorMessage(error: unknown): string {
15+
const coreMessage = getCoreErrorMessage(error);
16+
return extractRecursiveMessage(coreMessage);
17+
}
18+
19+
function extractRecursiveMessage(input: string): string {
20+
const trimmed = input.trim();
21+
22+
// Attempt to parse JSON error responses (common in Google API errors)
23+
if (
24+
(trimmed.startsWith('{') && trimmed.endsWith('}')) ||
25+
(trimmed.startsWith('[') && trimmed.endsWith(']'))
26+
) {
27+
try {
28+
const parsed = JSON.parse(trimmed);
29+
const next =
30+
parsed?.error?.message ||
31+
parsed?.[0]?.error?.message ||
32+
parsed?.message;
33+
34+
if (next && typeof next === 'string' && next !== input) {
35+
return extractRecursiveMessage(next);
36+
}
37+
} catch {
38+
// Fall back to original string if parsing fails
39+
}
40+
}
41+
return input;
42+
}

packages/cli/src/zed-integration/zedIntegration.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
} from '@google/gemini-cli-core';
3838
import * as acp from '@agentclientprotocol/sdk';
3939
import { AcpFileSystemService } from './fileSystemService.js';
40+
import { getAcpErrorMessage } from './acpErrors.js';
4041
import { Readable, Writable } from 'node:stream';
4142
import type { Content, Part, FunctionCall } from '@google/genai';
4243
import type { LoadedSettings } from '../config/settings.js';
@@ -142,7 +143,10 @@ export class GeminiAgent {
142143
try {
143144
await this.config.refreshAuth(method);
144145
} catch (e) {
145-
throw new acp.RequestError(getErrorStatus(e) || 401, getErrorMessage(e));
146+
throw new acp.RequestError(
147+
getErrorStatus(e) || 401,
148+
getAcpErrorMessage(e),
149+
);
146150
}
147151
this.settings.setValue(
148152
SettingScope.User,
@@ -184,7 +188,7 @@ export class GeminiAgent {
184188
}
185189
} catch (e) {
186190
isAuthenticated = false;
187-
authErrorMessage = getErrorMessage(e);
191+
authErrorMessage = getAcpErrorMessage(e);
188192
debugLogger.error(
189193
`Authentication failed: ${e instanceof Error ? e.stack : e}`,
190194
);
@@ -546,7 +550,7 @@ export class Session {
546550

547551
throw new acp.RequestError(
548552
getErrorStatus(error) || 500,
549-
getErrorMessage(error),
553+
getAcpErrorMessage(error),
550554
);
551555
}
552556

packages/core/src/utils/errors.test.ts

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,37 +19,6 @@ describe('getErrorMessage', () => {
1919
expect(getErrorMessage(new Error('plain error'))).toBe('plain error');
2020
});
2121

22-
it('should parse simple JSON error response', () => {
23-
const json = JSON.stringify({ error: { message: 'json error' } });
24-
expect(getErrorMessage(new Error(json))).toBe('json error');
25-
});
26-
27-
it('should parse double-encoded JSON error response', () => {
28-
const innerJson = JSON.stringify({ error: { message: 'nested error' } });
29-
const outerJson = JSON.stringify({ error: { message: innerJson } });
30-
expect(getErrorMessage(new Error(outerJson))).toBe('nested error');
31-
});
32-
33-
it('should parse array-style JSON error response', () => {
34-
const json = JSON.stringify([{ error: { message: 'array error' } }]);
35-
expect(getErrorMessage(new Error(json))).toBe('array error');
36-
});
37-
38-
it('should parse JSON with top-level message field', () => {
39-
const json = JSON.stringify({ message: 'top-level message' });
40-
expect(getErrorMessage(new Error(json))).toBe('top-level message');
41-
});
42-
43-
it('should handle JSON with trailing newline', () => {
44-
const json = JSON.stringify({ error: { message: 'newline error' } }) + '\n';
45-
expect(getErrorMessage(new Error(json))).toBe('newline error');
46-
});
47-
48-
it('should return original message if JSON parsing fails', () => {
49-
const invalidJson = '{ not-json }';
50-
expect(getErrorMessage(new Error(invalidJson))).toBe(invalidJson);
51-
});
52-
5322
it('should handle non-Error inputs', () => {
5423
expect(getErrorMessage('string error')).toBe('string error');
5524
expect(getErrorMessage(123)).toBe('123');

packages/core/src/utils/errors.ts

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,40 +16,11 @@ export function isNodeError(error: unknown): error is NodeJS.ErrnoException {
1616

1717
export function getErrorMessage(error: unknown): string {
1818
const friendlyError = toFriendlyError(error);
19-
return extractMessage(friendlyError);
20-
}
21-
22-
function extractMessage(input: unknown): string {
23-
if (input instanceof Error) {
24-
return extractMessage(input.message);
25-
}
26-
27-
if (typeof input === 'string') {
28-
const trimmed = input.trim();
29-
// Attempt to parse JSON error responses (common in Google API errors)
30-
if (
31-
(trimmed.startsWith('{') && trimmed.endsWith('}')) ||
32-
(trimmed.startsWith('[') && trimmed.endsWith(']'))
33-
) {
34-
try {
35-
const parsed = JSON.parse(trimmed);
36-
const next =
37-
parsed?.error?.message ||
38-
parsed?.[0]?.error?.message ||
39-
parsed?.message;
40-
41-
if (next && next !== input) {
42-
return extractMessage(next);
43-
}
44-
} catch {
45-
// Fall back to original string if parsing fails
46-
}
47-
}
48-
return input;
19+
if (friendlyError instanceof Error) {
20+
return friendlyError.message;
4921
}
50-
5122
try {
52-
return String(input);
23+
return String(friendlyError);
5324
} catch {
5425
return 'Failed to get error details';
5526
}

0 commit comments

Comments
 (0)