MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-03-10 22:21:22 -07:00
Code Issues Packages Projects Releases Wiki Activity

Add clarity to error messages (#14879)

Browse Source
This commit is contained in:
Gaurav Sehgal
2025-12-13 06:31:12 +05:30
committed by GitHub
parent ad60cbfc2c
commit d9f94103cd
5 changed files with 88 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
packages/cli/src/nonInteractiveCli.test.ts
View File

@@ -198,7 +198,6 @@ describe('runNonInteractive', () => {
);
vi.mocked(handleAtCommand).mockImplementation(async ({ query }) => ({
processedQuery: [{ text: query }],
shouldProceed: true,
}));
});
@@ -573,7 +572,6 @@ describe('runNonInteractive', () => {
// 3. Setup the mock to return the processed parts
mockHandleAtCommand.mockResolvedValue({
processedQuery: processedParts,
shouldProceed: true,
});
// Mock a simple stream response from the Gemini client

6
packages/cli/src/nonInteractiveCli.ts
View File

@@ -230,7 +230,7 @@ export async function runNonInteractive({
}
if (!query) {
const { processedQuery, shouldProceed } = await handleAtCommand({
const { processedQuery, error } = await handleAtCommand({
query: input,
config,
addItem: (_item, _timestamp) => 0,
@@ -239,11 +239,11 @@ export async function runNonInteractive({
signal: abortController.signal,
});
if (!shouldProceed || !processedQuery) {
if (error || !processedQuery) {
// An error occurred during @include processing (e.g., file not found).
// The error message is already logged by handleAtCommand.
throw new FatalInputError(
'Exiting due to an error processing the @ command.',
error || 'Exiting due to an error processing the @ command.',
);
}
query = processedQuery as Part[];

88
packages/cli/src/ui/hooks/atCommandProcessor.test.ts
View File

@@ -17,6 +17,7 @@ import {
COMMON_IGNORE_PATTERNS,
// DEFAULT_FILE_EXCLUDES,
} from '@google/gemini-cli-core';
import * as core from '@google/gemini-cli-core';
import * as os from 'node:os';
import { ToolCallStatus } from '../types.js';
import type { UseHistoryManagerReturn } from './useHistoryManager.js';
@@ -120,7 +121,6 @@ describe('handleAtCommand', () => {
expect(result).toEqual({
processedQuery: [{ text: query }],
shouldProceed: true,
});
});
@@ -138,7 +138,6 @@ describe('handleAtCommand', () => {
expect(result).toEqual({
processedQuery: [{ text: queryWithSpaces }],
shouldProceed: true,
});
expect(mockOnDebugMessage).toHaveBeenCalledWith(
'Lone @ detected, will be treated as text in the modified query.',
@@ -171,7 +170,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
expect(mockAddItem).toHaveBeenCalledWith(
expect.objectContaining({
@@ -211,7 +209,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
expect(mockOnDebugMessage).toHaveBeenCalledWith(
`Path ${dirPath} resolved to directory, using glob: ${resolvedGlob}`,
@@ -246,7 +243,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -276,7 +272,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
expect(mockAddItem).toHaveBeenCalledWith(
expect.objectContaining({
@@ -321,7 +316,6 @@ describe('handleAtCommand', () => {
{ text: content2 },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -362,7 +356,6 @@ describe('handleAtCommand', () => {
{ text: content2 },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -401,7 +394,6 @@ describe('handleAtCommand', () => {
{ text: content1 },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
expect(mockOnDebugMessage).toHaveBeenCalledWith(
`Path ${invalidFile} not found directly, attempting glob search.`,
@@ -428,7 +420,6 @@ describe('handleAtCommand', () => {
expect(result).toEqual({
processedQuery: [{ text: 'Check @nonexistent.txt and @ also' }],
shouldProceed: true,
});
});
@@ -462,7 +453,6 @@ describe('handleAtCommand', () => {
expect(result).toEqual({
processedQuery: [{ text: query }],
shouldProceed: true,
});
expect(mockOnDebugMessage).toHaveBeenCalledWith(
`Path ${gitIgnoredFile} is git-ignored and will be skipped.`,
@@ -501,7 +491,6 @@ describe('handleAtCommand', () => {
{ text: 'console.log("Hello world");' },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -534,7 +523,6 @@ describe('handleAtCommand', () => {
{ text: '# Project README' },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
expect(mockOnDebugMessage).toHaveBeenCalledWith(
`Path ${gitIgnoredFile} is git-ignored and will be skipped.`,
@@ -562,7 +550,6 @@ describe('handleAtCommand', () => {
expect(result).toEqual({
processedQuery: [{ text: query }],
shouldProceed: true,
});
expect(mockOnDebugMessage).toHaveBeenCalledWith(
`Path ${gitFile} is git-ignored and will be skipped.`,
@@ -595,7 +582,8 @@ describe('handleAtCommand', () => {
`Glob tool not found. Path ${invalidFile} will be skipped.`,
);
expect(result.processedQuery).toEqual([{ text: query }]);
expect(result.shouldProceed).toBe(true);
expect(result.processedQuery).not.toBeNull();
expect(result.error).toBeUndefined();
});
});
@@ -622,7 +610,6 @@ describe('handleAtCommand', () => {
expect(result).toEqual({
processedQuery: [{ text: query }],
shouldProceed: true,
});
expect(mockOnDebugMessage).toHaveBeenCalledWith(
`Path ${geminiIgnoredFile} is gemini-ignored and will be skipped.`,
@@ -660,7 +647,6 @@ describe('handleAtCommand', () => {
{ text: 'console.log("Hello world");' },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -696,7 +682,6 @@ describe('handleAtCommand', () => {
{ text: '// Main application entry' },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
expect(mockOnDebugMessage).toHaveBeenCalledWith(
`Path ${geminiIgnoredFile} is gemini-ignored and will be skipped.`,
@@ -824,7 +809,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
},
);
@@ -863,7 +847,6 @@ describe('handleAtCommand', () => {
{ text: content2 },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -893,7 +876,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -924,7 +906,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -955,7 +936,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -986,11 +966,10 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
it('should not terminate at period within file name', async () => {
it('should correctly handle file paths with multiple periods', async () => {
const fileContent = 'Version info';
const filePath = await createTestFile(
path.join(testRootDir, 'version.1.2.3.txt'),
@@ -1017,7 +996,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -1046,7 +1024,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -1075,7 +1052,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
@@ -1104,7 +1080,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
});
@@ -1136,7 +1111,6 @@ describe('handleAtCommand', () => {
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
expect(mockOnDebugMessage).toHaveBeenCalledWith(
@@ -1165,7 +1139,8 @@ describe('handleAtCommand', () => {
signal: abortController.signal,
});
expect(result.shouldProceed).toBe(true);
expect(result.processedQuery).not.toBeNull();
expect(result.error).toBeUndefined();
expect(result.processedQuery).toEqual(
expect.arrayContaining([
{ text: `Check @${path.join(subDirPath, '**')} please.` },
@@ -1207,7 +1182,6 @@ describe('handleAtCommand', () => {
expect(result).toEqual({
processedQuery: [{ text: `Check @${outsidePath} please.` }],
shouldProceed: true,
});
expect(mockOnDebugMessage).toHaveBeenCalledWith(
@@ -1326,7 +1300,8 @@ describe('handleAtCommand', () => {
signal: abortController.signal,
});
expect(result.shouldProceed).toBe(false);
expect(result.processedQuery).toBeNull();
expect(result.error).toBeDefined();
expect(mockAddItem).toHaveBeenCalledWith(
expect.objectContaining({
type: 'tool_group',
@@ -1342,4 +1317,51 @@ describe('handleAtCommand', () => {
);
});
});
it('should return error if the read_many_files tool is cancelled by user', async () => {
const fileContent = 'Some content';
const filePath = await createTestFile(
path.join(testRootDir, 'file.txt'),
fileContent,
);
const query = `@${filePath}`;
// Simulate user cancellation
const mockToolInstance = {
buildAndExecute: vi
.fn()
.mockRejectedValue(new Error('User cancelled operation')),
displayName: 'Read Many Files',
build: vi.fn(() => ({
execute: mockToolInstance.buildAndExecute,
getDescription: vi.fn(() => 'Mocked tool description'),
})),
};
const viSpy = vi.spyOn(core, 'ReadManyFilesTool');
viSpy.mockImplementation(
() => mockToolInstance as unknown as core.ReadManyFilesTool,
);
const result = await handleAtCommand({
query,
config: mockConfig,
addItem: mockAddItem,
onDebugMessage: mockOnDebugMessage,
messageId: 134,
signal: abortController.signal,
});
expect(result).toEqual({
processedQuery: null,
error: `Exiting due to an error processing the @ command: Error reading files (file.txt): User cancelled operation`,
});
expect(mockAddItem).toHaveBeenCalledWith(
expect.objectContaining({
type: 'tool_group',
tools: [expect.objectContaining({ status: ToolCallStatus.Error })],
}),
134,
);
});
});

43
packages/cli/src/ui/hooks/atCommandProcessor.ts
View File

@@ -35,7 +35,7 @@ interface HandleAtCommandParams {
interface HandleAtCommandResult {
processedQuery: PartListUnion | null;
shouldProceed: boolean;
error?: string;
}
interface AtCommandPart {
@@ -144,7 +144,7 @@ export async function handleAtCommand({
);
if (atPathCommandParts.length === 0) {
return { processedQuery: [{ text: query }], shouldProceed: true };
return { processedQuery: [{ text: query }] };
}
// Get centralized file discovery service
@@ -172,7 +172,10 @@ export async function handleAtCommand({
{ type: 'error', text: 'Error: read_many_files tool not found.' },
userMessageTimestamp,
);
return { processedQuery: null, shouldProceed: false };
return {
processedQuery: null,
error: 'Error: read_many_files tool not found.',
};
}
for (const atPathPart of atPathCommandParts) {
@@ -189,16 +192,17 @@ export async function handleAtCommand({
if (!pathName) {
// This case should ideally not be hit if parseAllAtCommands ensures content after @
// but as a safeguard:
const errMsg = `Error: Invalid @ command '${originalAtPath}'. No path specified.`;
addItem(
{
type: 'error',
text: `Error: Invalid @ command '${originalAtPath}'. No path specified.`,
text: errMsg,
},
userMessageTimestamp,
);
// Decide if this is a fatal error for the whole command or just skip this @ part
// For now, let's be strict and fail the command if one @path is malformed.
return { processedQuery: null, shouldProceed: false };
return { processedQuery: null, error: errMsg };
}
// Check if this is an MCP resource reference (serverName:uri format)
@@ -417,16 +421,13 @@ export async function handleAtCommand({
onDebugMessage('No valid file paths found in @ commands to read.');
if (initialQueryText === '@' && query.trim() === '@') {
// If the only thing was a lone @, pass original query (which might have spaces)
return { processedQuery: [{ text: query }], shouldProceed: true };
return { processedQuery: [{ text: query }] };
} else if (!initialQueryText && query) {
// If all @-commands were invalid and no surrounding text, pass original query
return { processedQuery: [{ text: query }], shouldProceed: true };
return { processedQuery: [{ text: query }] };
}
// Otherwise, proceed with the (potentially modified) query text that doesn't involve file reading
return {
processedQuery: [{ text: initialQueryText || query }],
shouldProceed: true,
};
return { processedQuery: [{ text: initialQueryText || query }] };
}
const processedQueryParts: PartListUnion = [{ text: initialQueryText }];
@@ -494,7 +495,16 @@ export async function handleAtCommand({
>,
userMessageTimestamp,
);
return { processedQuery: null, shouldProceed: false };
// Find the first error to report
const firstError = resourceReadDisplays.find(
(d) => d.status === ToolCallStatus.Error,
)!;
const errorMessages = resourceReadDisplays
.filter((d) => d.status === ToolCallStatus.Error)
.map((d) => d.resultDisplay);
console.error(errorMessages);
const errorMsg = `Exiting due to an error processing the @ command: ${firstError.resultDisplay}`;
return { processedQuery: null, error: errorMsg };
}
if (pathSpecsToRead.length === 0) {
@@ -507,7 +517,7 @@ export async function handleAtCommand({
userMessageTimestamp,
);
}
return { processedQuery: processedQueryParts, shouldProceed: true };
return { processedQuery: processedQueryParts };
}
const toolArgs = {
@@ -593,7 +603,7 @@ export async function handleAtCommand({
userMessageTimestamp,
);
}
return { processedQuery: processedQueryParts, shouldProceed: true };
return { processedQuery: processedQueryParts };
} catch (error: unknown) {
readManyFilesDisplay = {
callId: `client-read-${userMessageTimestamp}`,
@@ -612,7 +622,10 @@ export async function handleAtCommand({
} as Omit<HistoryItem, 'id'>,
userMessageTimestamp,
);
return { processedQuery: null, shouldProceed: false };
return {
processedQuery: null,
error: `Exiting due to an error processing the @ command: ${readManyFilesDisplay.resultDisplay}`,
};
}
}

3
packages/cli/src/ui/hooks/useGeminiStream.ts
View File

@@ -483,7 +483,8 @@ export const useGeminiStream = (
userMessageTimestamp,
);
if (!atCommandResult.shouldProceed) {
if (atCommandResult.error) {
onDebugMessage(atCommandResult.error);
return { queryToSend: null, shouldProceed: false };
}
localQueryToSendToGemini = atCommandResult.processedQuery;