From 2a7e602356b769a5520fc4140e45f8b72ec3d51b Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Fri, 13 Mar 2026 15:40:29 +0000 Subject: [PATCH] refactor(cli): consolidate getErrorMessage utility to core (#22190) --- packages/cli/src/acp/commands/extensions.ts | 7 +++++-- .../src/commands/extensions/disable.test.ts | 4 ++-- .../cli/src/commands/extensions/disable.ts | 3 +-- .../cli/src/commands/extensions/install.ts | 2 +- .../cli/src/commands/extensions/link.test.ts | 12 +++++------ packages/cli/src/commands/extensions/link.ts | 2 +- .../cli/src/commands/extensions/list.test.ts | 14 +++++-------- packages/cli/src/commands/extensions/list.ts | 3 +-- .../src/commands/extensions/uninstall.test.ts | 4 ++-- .../cli/src/commands/extensions/uninstall.ts | 3 +-- .../cli/src/commands/extensions/update.ts | 7 +++++-- .../cli/src/commands/extensions/validate.ts | 3 +-- .../cli/src/commands/skills/install.test.ts | 3 +++ packages/cli/src/commands/skills/install.ts | 7 +++++-- packages/cli/src/commands/skills/link.test.ts | 3 +++ packages/cli/src/commands/skills/link.ts | 3 +-- .../cli/src/commands/skills/uninstall.test.ts | 3 +++ packages/cli/src/commands/skills/uninstall.ts | 3 +-- packages/cli/src/config/extensions/github.ts | 2 +- packages/cli/src/config/extensions/update.ts | 7 +++++-- .../cli/src/ui/commands/extensionsCommand.ts | 2 +- packages/cli/src/ui/commands/skillsCommand.ts | 3 +-- .../cli/src/ui/hooks/useExtensionUpdates.ts | 2 +- packages/cli/src/utils/errors.test.ts | 20 ------------------- packages/cli/src/utils/errors.ts | 8 +------- 25 files changed, 56 insertions(+), 74 deletions(-) diff --git a/packages/cli/src/acp/commands/extensions.ts b/packages/cli/src/acp/commands/extensions.ts index d9342d647c..c2bd0e7190 100644 --- a/packages/cli/src/acp/commands/extensions.ts +++ b/packages/cli/src/acp/commands/extensions.ts @@ -4,13 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { listExtensions, type Config } from '@google/gemini-cli-core'; +import { + listExtensions, + type Config, + getErrorMessage, +} from '@google/gemini-cli-core'; import { SettingScope } from '../../config/settings.js'; import { ExtensionManager, inferInstallMetadata, } from '../../config/extension-manager.js'; -import { getErrorMessage } from '../../utils/errors.js'; import { McpServerEnablementManager } from '../../config/mcp/mcpServerEnablement.js'; import { stat } from 'node:fs/promises'; import type { diff --git a/packages/cli/src/commands/extensions/disable.test.ts b/packages/cli/src/commands/extensions/disable.test.ts index 341fbaf9f0..47fc1190c0 100644 --- a/packages/cli/src/commands/extensions/disable.test.ts +++ b/packages/cli/src/commands/extensions/disable.test.ts @@ -22,7 +22,7 @@ import { SettingScope, type LoadedSettings, } from '../../config/settings.js'; -import { getErrorMessage } from '../../utils/errors.js'; +import { getErrorMessage } from '@google/gemini-cli-core'; // Mock dependencies const emitConsoleLog = vi.hoisted(() => vi.fn()); @@ -44,12 +44,12 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { emitConsoleLog, }, debugLogger, + getErrorMessage: vi.fn(), }; }); vi.mock('../../config/extension-manager.js'); vi.mock('../../config/settings.js'); -vi.mock('../../utils/errors.js'); vi.mock('../../config/extensions/consent.js', () => ({ requestConsentNonInteractive: vi.fn(), })); diff --git a/packages/cli/src/commands/extensions/disable.ts b/packages/cli/src/commands/extensions/disable.ts index cdbc6a0ed4..dae97ea584 100644 --- a/packages/cli/src/commands/extensions/disable.ts +++ b/packages/cli/src/commands/extensions/disable.ts @@ -6,8 +6,7 @@ import { type CommandModule } from 'yargs'; import { loadSettings, SettingScope } from '../../config/settings.js'; -import { getErrorMessage } from '../../utils/errors.js'; -import { debugLogger } from '@google/gemini-cli-core'; +import { debugLogger, getErrorMessage } from '@google/gemini-cli-core'; import { ExtensionManager } from '../../config/extension-manager.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js'; diff --git a/packages/cli/src/commands/extensions/install.ts b/packages/cli/src/commands/extensions/install.ts index 1886444b88..eea7679c00 100644 --- a/packages/cli/src/commands/extensions/install.ts +++ b/packages/cli/src/commands/extensions/install.ts @@ -11,8 +11,8 @@ import { debugLogger, FolderTrustDiscoveryService, getRealPath, + getErrorMessage, } from '@google/gemini-cli-core'; -import { getErrorMessage } from '../../utils/errors.js'; import { INSTALL_WARNING_MESSAGE, promptForConsentNonInteractive, diff --git a/packages/cli/src/commands/extensions/link.test.ts b/packages/cli/src/commands/extensions/link.test.ts index 67351a5456..d54b81e083 100644 --- a/packages/cli/src/commands/extensions/link.test.ts +++ b/packages/cli/src/commands/extensions/link.test.ts @@ -13,26 +13,24 @@ import { afterEach, type Mock, } from 'vitest'; -import { coreEvents } from '@google/gemini-cli-core'; +import { coreEvents, getErrorMessage } from '@google/gemini-cli-core'; import { type Argv } from 'yargs'; import { handleLink, linkCommand } from './link.js'; import { ExtensionManager } from '../../config/extension-manager.js'; import { loadSettings, type LoadedSettings } from '../../config/settings.js'; -import { getErrorMessage } from '../../utils/errors.js'; vi.mock('@google/gemini-cli-core', async (importOriginal) => { const { mockCoreDebugLogger } = await import( '../../test-utils/mockDebugLogger.js' ); - return mockCoreDebugLogger( - await importOriginal(), - { stripAnsi: true }, - ); + const actual = + await importOriginal(); + const mocked = mockCoreDebugLogger(actual, { stripAnsi: true }); + return { ...mocked, getErrorMessage: vi.fn() }; }); vi.mock('../../config/extension-manager.js'); vi.mock('../../config/settings.js'); -vi.mock('../../utils/errors.js'); vi.mock('../../config/extensions/consent.js', () => ({ requestConsentNonInteractive: vi.fn(), })); diff --git a/packages/cli/src/commands/extensions/link.ts b/packages/cli/src/commands/extensions/link.ts index d7c5f2fd5c..0f419c5cad 100644 --- a/packages/cli/src/commands/extensions/link.ts +++ b/packages/cli/src/commands/extensions/link.ts @@ -8,10 +8,10 @@ import type { CommandModule } from 'yargs'; import chalk from 'chalk'; import { debugLogger, + getErrorMessage, type ExtensionInstallMetadata, } from '@google/gemini-cli-core'; -import { getErrorMessage } from '../../utils/errors.js'; import { INSTALL_WARNING_MESSAGE, requestConsentNonInteractive, diff --git a/packages/cli/src/commands/extensions/list.test.ts b/packages/cli/src/commands/extensions/list.test.ts index f0f0168f79..b65cfdaf3e 100644 --- a/packages/cli/src/commands/extensions/list.test.ts +++ b/packages/cli/src/commands/extensions/list.test.ts @@ -5,27 +5,23 @@ */ import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { coreEvents } from '@google/gemini-cli-core'; +import { coreEvents, getErrorMessage } from '@google/gemini-cli-core'; import { handleList, listCommand } from './list.js'; import { ExtensionManager } from '../../config/extension-manager.js'; import { loadSettings, type LoadedSettings } from '../../config/settings.js'; -import { getErrorMessage } from '../../utils/errors.js'; vi.mock('@google/gemini-cli-core', async (importOriginal) => { const { mockCoreDebugLogger } = await import( '../../test-utils/mockDebugLogger.js' ); - return mockCoreDebugLogger( - await importOriginal(), - { - stripAnsi: false, - }, - ); + const actual = + await importOriginal(); + const mocked = mockCoreDebugLogger(actual, { stripAnsi: false }); + return { ...mocked, getErrorMessage: vi.fn() }; }); vi.mock('../../config/extension-manager.js'); vi.mock('../../config/settings.js'); -vi.mock('../../utils/errors.js'); vi.mock('../../config/extensions/consent.js', () => ({ requestConsentNonInteractive: vi.fn(), })); diff --git a/packages/cli/src/commands/extensions/list.ts b/packages/cli/src/commands/extensions/list.ts index 9b4789ca55..e477ce3c21 100644 --- a/packages/cli/src/commands/extensions/list.ts +++ b/packages/cli/src/commands/extensions/list.ts @@ -5,8 +5,7 @@ */ import type { CommandModule } from 'yargs'; -import { getErrorMessage } from '../../utils/errors.js'; -import { debugLogger } from '@google/gemini-cli-core'; +import { debugLogger, getErrorMessage } from '@google/gemini-cli-core'; import { ExtensionManager } from '../../config/extension-manager.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; import { loadSettings } from '../../config/settings.js'; diff --git a/packages/cli/src/commands/extensions/uninstall.test.ts b/packages/cli/src/commands/extensions/uninstall.test.ts index 65aed446c5..341c0f7a7e 100644 --- a/packages/cli/src/commands/extensions/uninstall.test.ts +++ b/packages/cli/src/commands/extensions/uninstall.test.ts @@ -18,7 +18,7 @@ import { type Argv } from 'yargs'; import { handleUninstall, uninstallCommand } from './uninstall.js'; import { ExtensionManager } from '../../config/extension-manager.js'; import { loadSettings, type LoadedSettings } from '../../config/settings.js'; -import { getErrorMessage } from '../../utils/errors.js'; +import { getErrorMessage } from '@google/gemini-cli-core'; // NOTE: This file uses vi.hoisted() mocks to enable testing of sequential // mock behaviors (mockResolvedValueOnce/mockRejectedValueOnce chaining). @@ -66,11 +66,11 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { emitConsoleLog, }, debugLogger, + getErrorMessage: vi.fn(), }; }); vi.mock('../../config/settings.js'); -vi.mock('../../utils/errors.js'); vi.mock('../../config/extensions/consent.js', () => ({ requestConsentNonInteractive: vi.fn(), })); diff --git a/packages/cli/src/commands/extensions/uninstall.ts b/packages/cli/src/commands/extensions/uninstall.ts index b78b9510df..3a63602149 100644 --- a/packages/cli/src/commands/extensions/uninstall.ts +++ b/packages/cli/src/commands/extensions/uninstall.ts @@ -5,8 +5,7 @@ */ import type { CommandModule } from 'yargs'; -import { getErrorMessage } from '../../utils/errors.js'; -import { debugLogger } from '@google/gemini-cli-core'; +import { debugLogger, getErrorMessage } from '@google/gemini-cli-core'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; import { ExtensionManager } from '../../config/extension-manager.js'; import { loadSettings } from '../../config/settings.js'; diff --git a/packages/cli/src/commands/extensions/update.ts b/packages/cli/src/commands/extensions/update.ts index 4e5f593518..2459b5d7c4 100644 --- a/packages/cli/src/commands/extensions/update.ts +++ b/packages/cli/src/commands/extensions/update.ts @@ -12,9 +12,12 @@ import { updateExtension, } from '../../config/extensions/update.js'; import { checkForExtensionUpdate } from '../../config/extensions/github.js'; -import { getErrorMessage } from '../../utils/errors.js'; import { ExtensionUpdateState } from '../../ui/state/extensions.js'; -import { coreEvents, debugLogger } from '@google/gemini-cli-core'; +import { + coreEvents, + debugLogger, + getErrorMessage, +} from '@google/gemini-cli-core'; import { ExtensionManager } from '../../config/extension-manager.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; import { loadSettings } from '../../config/settings.js'; diff --git a/packages/cli/src/commands/extensions/validate.ts b/packages/cli/src/commands/extensions/validate.ts index 1385871219..e122b279dc 100644 --- a/packages/cli/src/commands/extensions/validate.ts +++ b/packages/cli/src/commands/extensions/validate.ts @@ -5,11 +5,10 @@ */ import type { CommandModule } from 'yargs'; -import { debugLogger } from '@google/gemini-cli-core'; +import { debugLogger, getErrorMessage } from '@google/gemini-cli-core'; import * as fs from 'node:fs'; import * as path from 'node:path'; import semver from 'semver'; -import { getErrorMessage } from '../../utils/errors.js'; import type { ExtensionConfig } from '../../config/extension.js'; import { ExtensionManager } from '../../config/extension-manager.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; diff --git a/packages/cli/src/commands/skills/install.test.ts b/packages/cli/src/commands/skills/install.test.ts index faaa7f31c6..db2548950d 100644 --- a/packages/cli/src/commands/skills/install.test.ts +++ b/packages/cli/src/commands/skills/install.test.ts @@ -28,6 +28,9 @@ const { debugLogger, emitConsoleLog } = await vi.hoisted(async () => { vi.mock('@google/gemini-cli-core', () => ({ debugLogger, + getErrorMessage: vi.fn((e: unknown) => + e instanceof Error ? e.message : String(e), + ), })); import { handleInstall, installCommand } from './install.js'; diff --git a/packages/cli/src/commands/skills/install.ts b/packages/cli/src/commands/skills/install.ts index 70ee094ae5..75dad58f0f 100644 --- a/packages/cli/src/commands/skills/install.ts +++ b/packages/cli/src/commands/skills/install.ts @@ -5,8 +5,11 @@ */ import type { CommandModule } from 'yargs'; -import { debugLogger, type SkillDefinition } from '@google/gemini-cli-core'; -import { getErrorMessage } from '../../utils/errors.js'; +import { + debugLogger, + type SkillDefinition, + getErrorMessage, +} from '@google/gemini-cli-core'; import { exitCli } from '../utils.js'; import { installSkill } from '../../utils/skillUtils.js'; import chalk from 'chalk'; diff --git a/packages/cli/src/commands/skills/link.test.ts b/packages/cli/src/commands/skills/link.test.ts index 24c3d3ff64..e661440952 100644 --- a/packages/cli/src/commands/skills/link.test.ts +++ b/packages/cli/src/commands/skills/link.test.ts @@ -24,6 +24,9 @@ const { debugLogger } = await vi.hoisted(async () => { vi.mock('@google/gemini-cli-core', () => ({ debugLogger, + getErrorMessage: vi.fn((e: unknown) => + e instanceof Error ? e.message : String(e), + ), })); vi.mock('../../config/extensions/consent.js', () => ({ diff --git a/packages/cli/src/commands/skills/link.ts b/packages/cli/src/commands/skills/link.ts index 60bf364bf4..3a03b93e6b 100644 --- a/packages/cli/src/commands/skills/link.ts +++ b/packages/cli/src/commands/skills/link.ts @@ -5,10 +5,9 @@ */ import type { CommandModule } from 'yargs'; -import { debugLogger } from '@google/gemini-cli-core'; +import { debugLogger, getErrorMessage } from '@google/gemini-cli-core'; import chalk from 'chalk'; -import { getErrorMessage } from '../../utils/errors.js'; import { exitCli } from '../utils.js'; import { requestConsentNonInteractive, diff --git a/packages/cli/src/commands/skills/uninstall.test.ts b/packages/cli/src/commands/skills/uninstall.test.ts index ab51db5b53..e12bda5353 100644 --- a/packages/cli/src/commands/skills/uninstall.test.ts +++ b/packages/cli/src/commands/skills/uninstall.test.ts @@ -21,6 +21,9 @@ const { debugLogger, emitConsoleLog } = await vi.hoisted(async () => { vi.mock('@google/gemini-cli-core', () => ({ debugLogger, + getErrorMessage: vi.fn((e: unknown) => + e instanceof Error ? e.message : String(e), + ), })); import { handleUninstall, uninstallCommand } from './uninstall.js'; diff --git a/packages/cli/src/commands/skills/uninstall.ts b/packages/cli/src/commands/skills/uninstall.ts index d5f030e1d2..cfcb67da21 100644 --- a/packages/cli/src/commands/skills/uninstall.ts +++ b/packages/cli/src/commands/skills/uninstall.ts @@ -5,8 +5,7 @@ */ import type { CommandModule } from 'yargs'; -import { debugLogger } from '@google/gemini-cli-core'; -import { getErrorMessage } from '../../utils/errors.js'; +import { debugLogger, getErrorMessage } from '@google/gemini-cli-core'; import { exitCli } from '../utils.js'; import { uninstallSkill } from '../../utils/skillUtils.js'; import chalk from 'chalk'; diff --git a/packages/cli/src/config/extensions/github.ts b/packages/cli/src/config/extensions/github.ts index 0141ffcc0e..156fe78309 100644 --- a/packages/cli/src/config/extensions/github.ts +++ b/packages/cli/src/config/extensions/github.ts @@ -5,9 +5,9 @@ */ import { simpleGit } from 'simple-git'; -import { getErrorMessage } from '../../utils/errors.js'; import { debugLogger, + getErrorMessage, type ExtensionInstallMetadata, type GeminiCLIExtension, } from '@google/gemini-cli-core'; diff --git a/packages/cli/src/config/extensions/update.ts b/packages/cli/src/config/extensions/update.ts index b1139d7143..4a91907d8f 100644 --- a/packages/cli/src/config/extensions/update.ts +++ b/packages/cli/src/config/extensions/update.ts @@ -11,9 +11,12 @@ import { } from '../../ui/state/extensions.js'; import { loadInstallMetadata } from '../extension.js'; import { checkForExtensionUpdate } from './github.js'; -import { debugLogger, type GeminiCLIExtension } from '@google/gemini-cli-core'; +import { + debugLogger, + getErrorMessage, + type GeminiCLIExtension, +} from '@google/gemini-cli-core'; import * as fs from 'node:fs'; -import { getErrorMessage } from '../../utils/errors.js'; import { copyExtension, type ExtensionManager } from '../extension-manager.js'; import { ExtensionStorage } from './storage.js'; diff --git a/packages/cli/src/ui/commands/extensionsCommand.ts b/packages/cli/src/ui/commands/extensionsCommand.ts index 6693d36b18..8fe206bfc4 100644 --- a/packages/cli/src/ui/commands/extensionsCommand.ts +++ b/packages/cli/src/ui/commands/extensionsCommand.ts @@ -7,10 +7,10 @@ import { debugLogger, listExtensions, + getErrorMessage, type ExtensionInstallMetadata, } from '@google/gemini-cli-core'; import type { ExtensionUpdateInfo } from '../../config/extension.js'; -import { getErrorMessage } from '../../utils/errors.js'; import { emptyIcon, MessageType, diff --git a/packages/cli/src/ui/commands/skillsCommand.ts b/packages/cli/src/ui/commands/skillsCommand.ts index 714f206f36..6f1672208d 100644 --- a/packages/cli/src/ui/commands/skillsCommand.ts +++ b/packages/cli/src/ui/commands/skillsCommand.ts @@ -16,9 +16,8 @@ import { MessageType, } from '../types.js'; import { disableSkill, enableSkill } from '../../utils/skillSettings.js'; -import { getErrorMessage } from '../../utils/errors.js'; -import { getAdminErrorMessage } from '@google/gemini-cli-core'; +import { getAdminErrorMessage, getErrorMessage } from '@google/gemini-cli-core'; import { linkSkill, renderSkillActionFeedback, diff --git a/packages/cli/src/ui/hooks/useExtensionUpdates.ts b/packages/cli/src/ui/hooks/useExtensionUpdates.ts index b46d3a4dee..52f39cde9f 100644 --- a/packages/cli/src/ui/hooks/useExtensionUpdates.ts +++ b/packages/cli/src/ui/hooks/useExtensionUpdates.ts @@ -7,9 +7,9 @@ import { debugLogger, checkExhaustive, + getErrorMessage, type GeminiCLIExtension, } from '@google/gemini-cli-core'; -import { getErrorMessage } from '../../utils/errors.js'; import { ExtensionUpdateState, extensionUpdatesReducer, diff --git a/packages/cli/src/utils/errors.test.ts b/packages/cli/src/utils/errors.test.ts index 38ee059bbe..a173711724 100644 --- a/packages/cli/src/utils/errors.test.ts +++ b/packages/cli/src/utils/errors.test.ts @@ -21,7 +21,6 @@ import { coreEvents, } from '@google/gemini-cli-core'; import { - getErrorMessage, handleError, handleToolError, handleCancellationError, @@ -152,25 +151,6 @@ describe('errors', () => { processExitSpy.mockRestore(); }); - describe('getErrorMessage', () => { - it('should return error message for Error instances', () => { - const error = new Error('Test error message'); - expect(getErrorMessage(error)).toBe('Test error message'); - }); - - it('should convert non-Error values to strings', () => { - expect(getErrorMessage('string error')).toBe('string error'); - expect(getErrorMessage(123)).toBe('123'); - expect(getErrorMessage(null)).toBe('null'); - expect(getErrorMessage(undefined)).toBe('undefined'); - }); - - it('should handle objects', () => { - const obj = { message: 'test' }; - expect(getErrorMessage(obj)).toBe('[object Object]'); - }); - }); - describe('handleError', () => { describe('in text mode', () => { beforeEach(() => { diff --git a/packages/cli/src/utils/errors.ts b/packages/cli/src/utils/errors.ts index 89c0fe6b22..9d4789b7e4 100644 --- a/packages/cli/src/utils/errors.ts +++ b/packages/cli/src/utils/errors.ts @@ -18,16 +18,10 @@ import { isFatalToolError, debugLogger, coreEvents, + getErrorMessage, } from '@google/gemini-cli-core'; import { runSyncCleanup } from './cleanup.js'; -export function getErrorMessage(error: unknown): string { - if (error instanceof Error) { - return error.message; - } - return String(error); -} - interface ErrorWithCode extends Error { exitCode?: number; code?: string | number;