mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-26 13:04:49 -07:00
feat(ux): Surface internal errors via unified event system (#11803)
This commit is contained in:
@@ -64,9 +64,12 @@ import {
|
||||
loadEnvironment,
|
||||
migrateDeprecatedSettings,
|
||||
SettingScope,
|
||||
saveSettings,
|
||||
type SettingsFile,
|
||||
} from './settings.js';
|
||||
import { FatalConfigError, GEMINI_DIR, Storage } from '@google/gemini-cli-core';
|
||||
import { ExtensionEnablementManager } from './extensions/extensionEnablement.js';
|
||||
import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js';
|
||||
|
||||
const MOCK_WORKSPACE_DIR = '/mock/workspace';
|
||||
// Use the (mocked) GEMINI_DIR for consistency
|
||||
@@ -96,6 +99,23 @@ vi.mock('fs', async (importOriginal) => {
|
||||
|
||||
vi.mock('./extension.js');
|
||||
|
||||
const mockCoreEvents = vi.hoisted(() => ({
|
||||
emitFeedback: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import('@google/gemini-cli-core')>();
|
||||
return {
|
||||
...actual,
|
||||
coreEvents: mockCoreEvents,
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock('../utils/commentJson.js', () => ({
|
||||
updateSettingsFilePreservingFormat: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('strip-json-comments', () => ({
|
||||
default: vi.fn((content) => content),
|
||||
}));
|
||||
@@ -2495,4 +2515,62 @@ describe('Settings Loading and Merging', () => {
|
||||
expect(setValueSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('saveSettings', () => {
|
||||
it('should save settings using updateSettingsFilePreservingFormat', () => {
|
||||
const mockUpdateSettings = vi.mocked(updateSettingsFilePreservingFormat);
|
||||
const settingsFile = {
|
||||
path: '/mock/settings.json',
|
||||
settings: { ui: { theme: 'dark' } },
|
||||
originalSettings: { ui: { theme: 'dark' } },
|
||||
} as unknown as SettingsFile;
|
||||
|
||||
saveSettings(settingsFile);
|
||||
|
||||
expect(mockUpdateSettings).toHaveBeenCalledWith('/mock/settings.json', {
|
||||
ui: { theme: 'dark' },
|
||||
});
|
||||
});
|
||||
|
||||
it('should create directory if it does not exist', () => {
|
||||
const mockFsExistsSync = vi.mocked(fs.existsSync);
|
||||
const mockFsMkdirSync = vi.mocked(fs.mkdirSync);
|
||||
mockFsExistsSync.mockReturnValue(false);
|
||||
|
||||
const settingsFile = {
|
||||
path: '/mock/new/dir/settings.json',
|
||||
settings: {},
|
||||
originalSettings: {},
|
||||
} as unknown as SettingsFile;
|
||||
|
||||
saveSettings(settingsFile);
|
||||
|
||||
expect(mockFsExistsSync).toHaveBeenCalledWith('/mock/new/dir');
|
||||
expect(mockFsMkdirSync).toHaveBeenCalledWith('/mock/new/dir', {
|
||||
recursive: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('should emit error feedback if saving fails', () => {
|
||||
const mockUpdateSettings = vi.mocked(updateSettingsFilePreservingFormat);
|
||||
const error = new Error('Write failed');
|
||||
mockUpdateSettings.mockImplementation(() => {
|
||||
throw error;
|
||||
});
|
||||
|
||||
const settingsFile = {
|
||||
path: '/mock/settings.json',
|
||||
settings: {},
|
||||
originalSettings: {},
|
||||
} as unknown as SettingsFile;
|
||||
|
||||
saveSettings(settingsFile);
|
||||
|
||||
expect(mockCoreEvents.emitFeedback).toHaveBeenCalledWith(
|
||||
'error',
|
||||
'There was an error saving your latest settings changes.',
|
||||
error,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
GEMINI_DIR,
|
||||
getErrorMessage,
|
||||
Storage,
|
||||
coreEvents,
|
||||
} from '@google/gemini-cli-core';
|
||||
import stripJsonComments from 'strip-json-comments';
|
||||
import { DefaultLight } from '../ui/themes/default-light.js';
|
||||
@@ -799,6 +800,10 @@ export function saveSettings(settingsFile: SettingsFile): void {
|
||||
settingsToSave as Record<string, unknown>,
|
||||
);
|
||||
} catch (error) {
|
||||
console.error('Error saving user settings file:', error);
|
||||
coreEvents.emitFeedback(
|
||||
'error',
|
||||
'There was an error saving your latest settings changes.',
|
||||
error,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ import type {
|
||||
SessionMetrics,
|
||||
AnyDeclarativeTool,
|
||||
AnyToolInvocation,
|
||||
UserFeedbackPayload,
|
||||
} from '@google/gemini-cli-core';
|
||||
import {
|
||||
executeToolCall,
|
||||
@@ -20,14 +21,32 @@ import {
|
||||
OutputFormat,
|
||||
uiTelemetryService,
|
||||
FatalInputError,
|
||||
CoreEvent,
|
||||
} from '@google/gemini-cli-core';
|
||||
import type { Part } from '@google/genai';
|
||||
import { runNonInteractive } from './nonInteractiveCli.js';
|
||||
import { vi, type Mock, type MockInstance } from 'vitest';
|
||||
import {
|
||||
describe,
|
||||
it,
|
||||
expect,
|
||||
beforeEach,
|
||||
afterEach,
|
||||
vi,
|
||||
type Mock,
|
||||
type MockInstance,
|
||||
} from 'vitest';
|
||||
import type { LoadedSettings } from './config/settings.js';
|
||||
|
||||
// Mock core modules
|
||||
vi.mock('./ui/hooks/atCommandProcessor.js');
|
||||
|
||||
const mockCoreEvents = vi.hoisted(() => ({
|
||||
on: vi.fn(),
|
||||
off: vi.fn(),
|
||||
drainFeedbackBacklog: vi.fn(),
|
||||
emit: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
|
||||
const original =
|
||||
await importOriginal<typeof import('@google/gemini-cli-core')>();
|
||||
@@ -48,6 +67,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
|
||||
uiTelemetryService: {
|
||||
getMetrics: vi.fn(),
|
||||
},
|
||||
coreEvents: mockCoreEvents,
|
||||
};
|
||||
});
|
||||
|
||||
@@ -70,6 +90,7 @@ describe('runNonInteractive', () => {
|
||||
let mockShutdownTelemetry: Mock;
|
||||
let consoleErrorSpy: MockInstance;
|
||||
let processStdoutSpy: MockInstance;
|
||||
let processStderrSpy: MockInstance;
|
||||
let mockGeminiClient: {
|
||||
sendMessageStream: Mock;
|
||||
getChatRecordingService: Mock;
|
||||
@@ -87,6 +108,9 @@ describe('runNonInteractive', () => {
|
||||
processStdoutSpy = vi
|
||||
.spyOn(process.stdout, 'write')
|
||||
.mockImplementation(() => true);
|
||||
processStderrSpy = vi
|
||||
.spyOn(process.stderr, 'write')
|
||||
.mockImplementation(() => true);
|
||||
vi.spyOn(process, 'exit').mockImplementation((code) => {
|
||||
throw new Error(`process.exit(${code}) called`);
|
||||
});
|
||||
@@ -1051,4 +1075,135 @@ describe('runNonInteractive', () => {
|
||||
);
|
||||
expect(processStdoutSpy).toHaveBeenCalledWith('file.txt');
|
||||
});
|
||||
|
||||
describe('CoreEvents Integration', () => {
|
||||
it('subscribes to UserFeedback and drains backlog on start', async () => {
|
||||
const events: ServerGeminiStreamEvent[] = [
|
||||
{
|
||||
type: GeminiEventType.Finished,
|
||||
value: { reason: undefined, usageMetadata: { totalTokenCount: 0 } },
|
||||
},
|
||||
];
|
||||
mockGeminiClient.sendMessageStream.mockReturnValue(
|
||||
createStreamFromEvents(events),
|
||||
);
|
||||
|
||||
await runNonInteractive(
|
||||
mockConfig,
|
||||
mockSettings,
|
||||
'test',
|
||||
'prompt-id-events',
|
||||
);
|
||||
|
||||
expect(mockCoreEvents.on).toHaveBeenCalledWith(
|
||||
CoreEvent.UserFeedback,
|
||||
expect.any(Function),
|
||||
);
|
||||
expect(mockCoreEvents.drainFeedbackBacklog).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('unsubscribes from UserFeedback on finish', async () => {
|
||||
const events: ServerGeminiStreamEvent[] = [
|
||||
{
|
||||
type: GeminiEventType.Finished,
|
||||
value: { reason: undefined, usageMetadata: { totalTokenCount: 0 } },
|
||||
},
|
||||
];
|
||||
mockGeminiClient.sendMessageStream.mockReturnValue(
|
||||
createStreamFromEvents(events),
|
||||
);
|
||||
|
||||
await runNonInteractive(
|
||||
mockConfig,
|
||||
mockSettings,
|
||||
'test',
|
||||
'prompt-id-events',
|
||||
);
|
||||
|
||||
expect(mockCoreEvents.off).toHaveBeenCalledWith(
|
||||
CoreEvent.UserFeedback,
|
||||
expect.any(Function),
|
||||
);
|
||||
});
|
||||
|
||||
it('logs to process.stderr when UserFeedback event is received', async () => {
|
||||
const events: ServerGeminiStreamEvent[] = [
|
||||
{
|
||||
type: GeminiEventType.Finished,
|
||||
value: { reason: undefined, usageMetadata: { totalTokenCount: 0 } },
|
||||
},
|
||||
];
|
||||
mockGeminiClient.sendMessageStream.mockReturnValue(
|
||||
createStreamFromEvents(events),
|
||||
);
|
||||
|
||||
await runNonInteractive(
|
||||
mockConfig,
|
||||
mockSettings,
|
||||
'test',
|
||||
'prompt-id-events',
|
||||
);
|
||||
|
||||
// Get the registered handler
|
||||
const handler = mockCoreEvents.on.mock.calls.find(
|
||||
(call: unknown[]) => call[0] === CoreEvent.UserFeedback,
|
||||
)?.[1];
|
||||
expect(handler).toBeDefined();
|
||||
|
||||
// Simulate an event
|
||||
const payload: UserFeedbackPayload = {
|
||||
severity: 'error',
|
||||
message: 'Test error message',
|
||||
};
|
||||
handler(payload);
|
||||
|
||||
expect(processStderrSpy).toHaveBeenCalledWith(
|
||||
'[ERROR] Test error message\n',
|
||||
);
|
||||
});
|
||||
|
||||
it('logs optional error object to process.stderr in debug mode', async () => {
|
||||
vi.mocked(mockConfig.getDebugMode).mockReturnValue(true);
|
||||
const events: ServerGeminiStreamEvent[] = [
|
||||
{
|
||||
type: GeminiEventType.Finished,
|
||||
value: { reason: undefined, usageMetadata: { totalTokenCount: 0 } },
|
||||
},
|
||||
];
|
||||
mockGeminiClient.sendMessageStream.mockReturnValue(
|
||||
createStreamFromEvents(events),
|
||||
);
|
||||
|
||||
await runNonInteractive(
|
||||
mockConfig,
|
||||
mockSettings,
|
||||
'test',
|
||||
'prompt-id-events',
|
||||
);
|
||||
|
||||
// Get the registered handler
|
||||
const handler = mockCoreEvents.on.mock.calls.find(
|
||||
(call: unknown[]) => call[0] === CoreEvent.UserFeedback,
|
||||
)?.[1];
|
||||
expect(handler).toBeDefined();
|
||||
|
||||
// Simulate an event with error object
|
||||
const errorObj = new Error('Original error');
|
||||
// Mock stack for deterministic testing
|
||||
errorObj.stack = 'Error: Original error\n at test';
|
||||
const payload: UserFeedbackPayload = {
|
||||
severity: 'warning',
|
||||
message: 'Test warning message',
|
||||
error: errorObj,
|
||||
};
|
||||
handler(payload);
|
||||
|
||||
expect(processStderrSpy).toHaveBeenCalledWith(
|
||||
'[WARNING] Test warning message\n',
|
||||
);
|
||||
expect(processStderrSpy).toHaveBeenCalledWith(
|
||||
'Error: Original error\n at test\n',
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,6 +8,7 @@ import type {
|
||||
Config,
|
||||
ToolCallRequestInfo,
|
||||
CompletedToolCall,
|
||||
UserFeedbackPayload,
|
||||
} from '@google/gemini-cli-core';
|
||||
import { isSlashCommand } from './ui/utils/commandUtils.js';
|
||||
import type { LoadedSettings } from './config/settings.js';
|
||||
@@ -24,6 +25,8 @@ import {
|
||||
JsonStreamEventType,
|
||||
uiTelemetryService,
|
||||
debugLogger,
|
||||
coreEvents,
|
||||
CoreEvent,
|
||||
} from '@google/gemini-cli-core';
|
||||
|
||||
import type { Content, Part } from '@google/genai';
|
||||
@@ -50,6 +53,18 @@ export async function runNonInteractive(
|
||||
debugMode: config.getDebugMode(),
|
||||
});
|
||||
|
||||
const handleUserFeedback = (payload: UserFeedbackPayload) => {
|
||||
const prefix = payload.severity.toUpperCase();
|
||||
process.stderr.write(`[${prefix}] ${payload.message}\n`);
|
||||
if (payload.error && config.getDebugMode()) {
|
||||
const errorToLog =
|
||||
payload.error instanceof Error
|
||||
? payload.error.stack || payload.error.message
|
||||
: String(payload.error);
|
||||
process.stderr.write(`${errorToLog}\n`);
|
||||
}
|
||||
};
|
||||
|
||||
const startTime = Date.now();
|
||||
const streamFormatter =
|
||||
config.getOutputFormat() === OutputFormat.STREAM_JSON
|
||||
@@ -58,6 +73,9 @@ export async function runNonInteractive(
|
||||
|
||||
try {
|
||||
consolePatcher.patch();
|
||||
coreEvents.on(CoreEvent.UserFeedback, handleUserFeedback);
|
||||
coreEvents.drainFeedbackBacklog();
|
||||
|
||||
// Handle EPIPE errors when the output is piped to a command that closes early.
|
||||
process.stdout.on('error', (err: NodeJS.ErrnoException) => {
|
||||
if (err.code === 'EPIPE') {
|
||||
@@ -287,6 +305,7 @@ export async function runNonInteractive(
|
||||
handleError(error, config);
|
||||
} finally {
|
||||
consolePatcher.cleanup();
|
||||
coreEvents.off(CoreEvent.UserFeedback, handleUserFeedback);
|
||||
if (isTelemetrySdkInitialized()) {
|
||||
await shutdownTelemetry(config);
|
||||
}
|
||||
|
||||
@@ -15,7 +15,29 @@ import {
|
||||
} from 'vitest';
|
||||
import { render, cleanup } from 'ink-testing-library';
|
||||
import { AppContainer } from './AppContainer.js';
|
||||
import { type Config, makeFakeConfig } from '@google/gemini-cli-core';
|
||||
import {
|
||||
type Config,
|
||||
makeFakeConfig,
|
||||
CoreEvent,
|
||||
type UserFeedbackPayload,
|
||||
} from '@google/gemini-cli-core';
|
||||
|
||||
// Mock coreEvents
|
||||
const mockCoreEvents = vi.hoisted(() => ({
|
||||
on: vi.fn(),
|
||||
off: vi.fn(),
|
||||
drainFeedbackBacklog: vi.fn(),
|
||||
emit: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import('@google/gemini-cli-core')>();
|
||||
return {
|
||||
...actual,
|
||||
coreEvents: mockCoreEvents,
|
||||
};
|
||||
});
|
||||
import type { LoadedSettings } from '../config/settings.js';
|
||||
import type { InitializationResult } from '../core/initializer.js';
|
||||
import { useQuotaAndFallback } from './hooks/useQuotaAndFallback.js';
|
||||
@@ -1293,4 +1315,73 @@ describe('AppContainer State Management', () => {
|
||||
expect(mockCloseModelDialog).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('CoreEvents Integration', () => {
|
||||
it('subscribes to UserFeedback and drains backlog on mount', () => {
|
||||
render(
|
||||
<AppContainer
|
||||
config={mockConfig}
|
||||
settings={mockSettings}
|
||||
version="1.0.0"
|
||||
initializationResult={mockInitResult}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(mockCoreEvents.on).toHaveBeenCalledWith(
|
||||
CoreEvent.UserFeedback,
|
||||
expect.any(Function),
|
||||
);
|
||||
expect(mockCoreEvents.drainFeedbackBacklog).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('unsubscribes from UserFeedback on unmount', () => {
|
||||
const { unmount } = render(
|
||||
<AppContainer
|
||||
config={mockConfig}
|
||||
settings={mockSettings}
|
||||
version="1.0.0"
|
||||
initializationResult={mockInitResult}
|
||||
/>,
|
||||
);
|
||||
|
||||
unmount();
|
||||
|
||||
expect(mockCoreEvents.off).toHaveBeenCalledWith(
|
||||
CoreEvent.UserFeedback,
|
||||
expect.any(Function),
|
||||
);
|
||||
});
|
||||
|
||||
it('adds history item when UserFeedback event is received', () => {
|
||||
render(
|
||||
<AppContainer
|
||||
config={mockConfig}
|
||||
settings={mockSettings}
|
||||
version="1.0.0"
|
||||
initializationResult={mockInitResult}
|
||||
/>,
|
||||
);
|
||||
|
||||
// Get the registered handler
|
||||
const handler = mockCoreEvents.on.mock.calls.find(
|
||||
(call: unknown[]) => call[0] === CoreEvent.UserFeedback,
|
||||
)?.[1];
|
||||
expect(handler).toBeDefined();
|
||||
|
||||
// Simulate an event
|
||||
const payload: UserFeedbackPayload = {
|
||||
severity: 'error',
|
||||
message: 'Test error message',
|
||||
};
|
||||
handler(payload);
|
||||
|
||||
expect(mockedUseHistory().addItem).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
type: 'error',
|
||||
text: 'Test error message',
|
||||
}),
|
||||
expect.any(Number),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -34,6 +34,7 @@ import {
|
||||
type IdeInfo,
|
||||
type IdeContext,
|
||||
type UserTierId,
|
||||
type UserFeedbackPayload,
|
||||
DEFAULT_GEMINI_FLASH_MODEL,
|
||||
IdeClient,
|
||||
ideContextStore,
|
||||
@@ -44,6 +45,8 @@ import {
|
||||
recordExitFail,
|
||||
ShellExecutionService,
|
||||
debugLogger,
|
||||
coreEvents,
|
||||
CoreEvent,
|
||||
} from '@google/gemini-cli-core';
|
||||
import { validateAuthMethod } from '../config/auth.js';
|
||||
import { loadHierarchicalGeminiMemory } from '../config/config.js';
|
||||
@@ -1066,6 +1069,53 @@ Logging in with Google... Please restart Gemini CLI to continue.
|
||||
stdout,
|
||||
]);
|
||||
|
||||
useEffect(() => {
|
||||
const handleUserFeedback = (payload: UserFeedbackPayload) => {
|
||||
let type: MessageType;
|
||||
switch (payload.severity) {
|
||||
case 'error':
|
||||
type = MessageType.ERROR;
|
||||
break;
|
||||
case 'warning':
|
||||
type = MessageType.WARNING;
|
||||
break;
|
||||
case 'info':
|
||||
type = MessageType.INFO;
|
||||
break;
|
||||
default:
|
||||
throw new Error(
|
||||
`Unexpected severity for user feedback: ${payload.severity}`,
|
||||
);
|
||||
}
|
||||
|
||||
historyManager.addItem(
|
||||
{
|
||||
type,
|
||||
text: payload.message,
|
||||
},
|
||||
Date.now(),
|
||||
);
|
||||
|
||||
// If there is an attached error object, log it to the debug drawer.
|
||||
if (payload.error) {
|
||||
debugLogger.warn(
|
||||
`[Feedback Details for "${payload.message}"]`,
|
||||
payload.error,
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
coreEvents.on(CoreEvent.UserFeedback, handleUserFeedback);
|
||||
|
||||
// Flush any messages that happened during startup before this component
|
||||
// mounted.
|
||||
coreEvents.drainFeedbackBacklog();
|
||||
|
||||
return () => {
|
||||
coreEvents.off(CoreEvent.UserFeedback, handleUserFeedback);
|
||||
};
|
||||
}, [historyManager]);
|
||||
|
||||
const filteredConsoleMessages = useMemo(() => {
|
||||
if (config.getDebugMode()) {
|
||||
return consoleMessages;
|
||||
|
||||
Reference in New Issue
Block a user