Skip to content

Commit 10ae848

Browse files
authored
Migrate console to coreEvents.emitFeedback or debugLogger (#15219)
1 parent dcd2449 commit 10ae848

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

66 files changed

+564
-425
lines changed

GEMINI.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,15 @@ When working in the `/docs` directory, follow the guidelines in this section:
391391
Only write high-value comments if at all. Avoid talking to the user through
392392
comments.
393393

394+
## Logging and Error Handling
395+
396+
- **Avoid Console Statements:** Do not use `console.log`, `console.error`, or
397+
similar methods directly.
398+
- **Non-User-Facing Logs:** For developer-facing debug messages, use
399+
`debugLogger` (from `@google/gemini-cli-core`).
400+
- **User-Facing Feedback:** To surface errors or warnings to the user, use
401+
`coreEvents.emitFeedback` (from `@google/gemini-cli-core`).
402+
394403
## General requirements
395404

396405
- If there is something you do not understand or is ambiguous, seek confirmation

eslint.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ export default tseslint.config(
164164
'prefer-arrow-callback': 'error',
165165
'prefer-const': ['error', { destructuring: 'all' }],
166166
radix: 'error',
167+
'no-console': 'error',
167168
'default-case': 'error',
168169
'@typescript-eslint/await-thenable': ['error'],
169170
'@typescript-eslint/no-floating-promises': ['error'],

packages/a2a-server/src/commands/command-registry.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7+
import { debugLogger } from '@google/gemini-cli-core';
78
import { ExtensionsCommand } from './extensions.js';
89
import { InitCommand } from './init.js';
910
import { RestoreCommand } from './restore.js';
@@ -20,7 +21,7 @@ class CommandRegistry {
2021

2122
register(command: Command) {
2223
if (this.commands.has(command.name)) {
23-
console.warn(`Command ${command.name} already registered. Skipping.`);
24+
debugLogger.warn(`Command ${command.name} already registered. Skipping.`);
2425
return;
2526
}
2627

packages/a2a-server/src/config/settings.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import * as fs from 'node:fs';
99
import * as path from 'node:path';
1010
import * as os from 'node:os';
1111
import { loadSettings, USER_SETTINGS_PATH } from './settings.js';
12+
import { debugLogger } from '@google/gemini-cli-core';
1213

1314
const mocks = vi.hoisted(() => {
1415
const suffix = Math.random().toString(36).slice(2);
@@ -75,7 +76,7 @@ describe('loadSettings', () => {
7576
fs.rmSync(mockWorkspaceDir, { recursive: true, force: true });
7677
}
7778
} catch (e) {
78-
console.error('Failed to cleanup temp dirs', e);
79+
debugLogger.error('Failed to cleanup temp dirs', e);
7980
}
8081
vi.restoreAllMocks();
8182
});

packages/a2a-server/src/http/app.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { loadConfig, loadEnvironment, setTargetDir } from '../config/config.js';
2525
import { loadSettings } from '../config/settings.js';
2626
import { loadExtensions } from '../config/extension.js';
2727
import { commandRegistry } from '../commands/command-registry.js';
28-
import { SimpleExtensionLoader } from '@google/gemini-cli-core';
28+
import { debugLogger, SimpleExtensionLoader } from '@google/gemini-cli-core';
2929
import type { Command, CommandArgument } from '../commands/types.js';
3030
import { GitService } from '@google/gemini-cli-core';
3131

@@ -239,7 +239,7 @@ export async function createApp() {
239239
): CommandResponse | undefined => {
240240
const commandName = command.name;
241241
if (visited.includes(commandName)) {
242-
console.warn(
242+
debugLogger.warn(
243243
`Command ${commandName} already inserted in the response, skipping`,
244244
);
245245
return undefined;

packages/cli/src/gemini.test.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
type Config,
3030
type ResumedSessionData,
3131
debugLogger,
32+
coreEvents,
3233
} from '@google/gemini-cli-core';
3334
import { act } from 'react';
3435
import { type InitializationResult } from './core/initializer.js';
@@ -819,9 +820,7 @@ describe('gemini.tsx main function kitty protocol', () => {
819820
.mockImplementation((code) => {
820821
throw new MockProcessExitError(code);
821822
});
822-
const consoleErrorSpy = vi
823-
.spyOn(console, 'error')
824-
.mockImplementation(() => {});
823+
const emitFeedbackSpy = vi.spyOn(coreEvents, 'emitFeedback');
825824

826825
vi.mocked(loadSettings).mockReturnValue({
827826
merged: { advanced: {}, security: { auth: {} }, ui: { theme: 'test' } },
@@ -875,12 +874,13 @@ describe('gemini.tsx main function kitty protocol', () => {
875874
if (!(e instanceof MockProcessExitError)) throw e;
876875
}
877876

878-
expect(consoleErrorSpy).toHaveBeenCalledWith(
877+
expect(emitFeedbackSpy).toHaveBeenCalledWith(
878+
'error',
879879
expect.stringContaining('Error resuming session: Session not found'),
880880
);
881881
expect(processExitSpy).toHaveBeenCalledWith(42);
882882
processExitSpy.mockRestore();
883-
consoleErrorSpy.mockRestore();
883+
emitFeedbackSpy.mockRestore();
884884
});
885885

886886
it.skip('should log error when cleanupExpiredSessions fails', async () => {

packages/cli/src/gemini.tsx

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
type ResumedSessionData,
4242
type OutputPayload,
4343
type ConsoleLogPayload,
44+
type UserFeedbackPayload,
4445
sessionId,
4546
logUserPrompt,
4647
AuthType,
@@ -597,7 +598,8 @@ export async function main() {
597598
// Use the existing session ID to continue recording to the same session
598599
config.setSessionId(resumedSessionData.conversation.sessionId);
599600
} catch (error) {
600-
console.error(
601+
coreEvents.emitFeedback(
602+
'error',
601603
`Error resuming session: ${error instanceof Error ? error.message : 'Unknown error'}`,
602604
);
603605
await runExitCleanup();
@@ -719,13 +721,25 @@ export function initializeOutputListenersAndFlush() {
719721
}
720722
});
721723

722-
coreEvents.on(CoreEvent.ConsoleLog, (payload: ConsoleLogPayload) => {
723-
if (payload.type === 'error' || payload.type === 'warn') {
724-
writeToStderr(payload.content);
725-
} else {
726-
writeToStdout(payload.content);
727-
}
728-
});
724+
if (coreEvents.listenerCount(CoreEvent.ConsoleLog) === 0) {
725+
coreEvents.on(CoreEvent.ConsoleLog, (payload: ConsoleLogPayload) => {
726+
if (payload.type === 'error' || payload.type === 'warn') {
727+
writeToStderr(payload.content);
728+
} else {
729+
writeToStdout(payload.content);
730+
}
731+
});
732+
}
733+
734+
if (coreEvents.listenerCount(CoreEvent.UserFeedback) === 0) {
735+
coreEvents.on(CoreEvent.UserFeedback, (payload: UserFeedbackPayload) => {
736+
if (payload.severity === 'error' || payload.severity === 'warning') {
737+
writeToStderr(payload.message);
738+
} else {
739+
writeToStdout(payload.message);
740+
}
741+
});
742+
}
729743
}
730744
coreEvents.drainBacklogs();
731745
}

packages/cli/src/nonInteractiveCli.test.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ const mockCoreEvents = vi.hoisted(() => ({
4444
off: vi.fn(),
4545
drainBacklogs: vi.fn(),
4646
emit: vi.fn(),
47+
emitFeedback: vi.fn(),
4748
}));
4849

4950
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
@@ -785,11 +786,6 @@ describe('runNonInteractive', () => {
785786
throw testError;
786787
});
787788

788-
// Mock console.error to capture JSON error output
789-
const consoleErrorJsonSpy = vi
790-
.spyOn(console, 'error')
791-
.mockImplementation(() => {});
792-
793789
let thrownError: Error | null = null;
794790
try {
795791
await runNonInteractive({
@@ -807,7 +803,8 @@ describe('runNonInteractive', () => {
807803
// Should throw because of mocked process.exit
808804
expect(thrownError?.message).toBe('process.exit(1) called');
809805

810-
expect(consoleErrorJsonSpy).toHaveBeenCalledWith(
806+
expect(mockCoreEvents.emitFeedback).toHaveBeenCalledWith(
807+
'error',
811808
JSON.stringify(
812809
{
813810
session_id: 'test-session-id',
@@ -831,11 +828,6 @@ describe('runNonInteractive', () => {
831828
throw fatalError;
832829
});
833830

834-
// Mock console.error to capture JSON error output
835-
const consoleErrorJsonSpy = vi
836-
.spyOn(console, 'error')
837-
.mockImplementation(() => {});
838-
839831
let thrownError: Error | null = null;
840832
try {
841833
await runNonInteractive({
@@ -853,7 +845,8 @@ describe('runNonInteractive', () => {
853845
// Should throw because of mocked process.exit with custom exit code
854846
expect(thrownError?.message).toBe('process.exit(42) called');
855847

856-
expect(consoleErrorJsonSpy).toHaveBeenCalledWith(
848+
expect(mockCoreEvents.emitFeedback).toHaveBeenCalledWith(
849+
'error',
857850
JSON.stringify(
858851
{
859852
session_id: 'test-session-id',

packages/cli/src/services/FileCommandLoader.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import toml from '@iarna/toml';
1010
import { glob } from 'glob';
1111
import { z } from 'zod';
1212
import type { Config } from '@google/gemini-cli-core';
13-
import { Storage } from '@google/gemini-cli-core';
13+
import { Storage, coreEvents } from '@google/gemini-cli-core';
1414
import type { ICommandLoader } from './types.js';
1515
import type {
1616
CommandContext,
@@ -126,7 +126,8 @@ export class FileCommandLoader implements ICommandLoader {
126126
!signal.aborted &&
127127
(error as { code?: string })?.code !== 'ENOENT'
128128
) {
129-
console.error(
129+
coreEvents.emitFeedback(
130+
'error',
130131
`[FileCommandLoader] Error loading commands from ${dirInfo.path}:`,
131132
error,
132133
);
@@ -189,7 +190,8 @@ export class FileCommandLoader implements ICommandLoader {
189190
try {
190191
fileContent = await fs.readFile(filePath, 'utf-8');
191192
} catch (error: unknown) {
192-
console.error(
193+
coreEvents.emitFeedback(
194+
'error',
193195
`[FileCommandLoader] Failed to read file ${filePath}:`,
194196
error instanceof Error ? error.message : String(error),
195197
);
@@ -200,7 +202,8 @@ export class FileCommandLoader implements ICommandLoader {
200202
try {
201203
parsed = toml.parse(fileContent);
202204
} catch (error: unknown) {
203-
console.error(
205+
coreEvents.emitFeedback(
206+
'error',
204207
`[FileCommandLoader] Failed to parse TOML file ${filePath}:`,
205208
error instanceof Error ? error.message : String(error),
206209
);
@@ -210,7 +213,8 @@ export class FileCommandLoader implements ICommandLoader {
210213
const validationResult = TomlCommandDefSchema.safeParse(parsed);
211214

212215
if (!validationResult.success) {
213-
console.error(
216+
coreEvents.emitFeedback(
217+
'error',
214218
`[FileCommandLoader] Skipping invalid command file: ${filePath}. Validation errors:`,
215219
validationResult.error.flatten(),
216220
);
@@ -278,7 +282,8 @@ export class FileCommandLoader implements ICommandLoader {
278282
_args: string,
279283
): Promise<SlashCommandActionReturn> => {
280284
if (!context.invocation) {
281-
console.error(
285+
coreEvents.emitFeedback(
286+
'error',
282287
`[FileCommandLoader] Critical error: Command '${baseCommandName}' was executed without invocation context.`,
283288
);
284289
return {

packages/cli/src/ui/AppContainer.tsx

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -842,16 +842,8 @@ Logging in with Google... Restarting Gemini CLI to continue.
842842
const handleClearScreen = useCallback(() => {
843843
historyManager.clearItems();
844844
clearConsoleMessagesState();
845-
if (!isAlternateBuffer) {
846-
console.clear();
847-
}
848845
refreshStatic();
849-
}, [
850-
historyManager,
851-
clearConsoleMessagesState,
852-
refreshStatic,
853-
isAlternateBuffer,
854-
]);
846+
}, [historyManager, clearConsoleMessagesState, refreshStatic]);
855847

856848
const { handleInput: vimHandleInput } = useVim(buffer, handleFinalSubmit);
857849

0 commit comments

Comments
 (0)