mirror of
https://github.com/google-gemini/gemini-cli
synced 2026-04-21 13:37:17 +00:00
Merge fbec6ff979 into a38e2f0048
This commit is contained in:
commit
3dbe69b0ce
4 changed files with 160 additions and 6 deletions
|
|
@ -64,6 +64,7 @@ import {
|
|||
import { maybeRequestConsentOrFail } from './extensions/consent.js';
|
||||
import { resolveEnvVarsInObject } from '../utils/envVarResolver.js';
|
||||
import { ExtensionStorage } from './extensions/storage.js';
|
||||
import { removeDirectoryWithRetry } from '../utils/retry.js';
|
||||
import {
|
||||
EXTENSIONS_CONFIG_FILENAME,
|
||||
INSTALL_METADATA_FILENAME,
|
||||
|
|
@ -495,7 +496,7 @@ Would you like to attempt to install via "git clone" instead?`,
|
|||
}
|
||||
} finally {
|
||||
if (tempDir) {
|
||||
await fs.promises.rm(tempDir, { recursive: true, force: true });
|
||||
await removeDirectoryWithRetry(tempDir);
|
||||
}
|
||||
}
|
||||
return extension;
|
||||
|
|
@ -564,10 +565,7 @@ Would you like to attempt to install via "git clone" instead?`,
|
|||
: path.basename(extension.path),
|
||||
);
|
||||
|
||||
await fs.promises.rm(storage.getExtensionDir(), {
|
||||
recursive: true,
|
||||
force: true,
|
||||
});
|
||||
await removeDirectoryWithRetry(storage.getExtensionDir());
|
||||
|
||||
// The rest of the cleanup below here is only for true uninstalls, not
|
||||
// uninstalls related to updates.
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import {
|
|||
import * as fs from 'node:fs';
|
||||
import { copyExtension, type ExtensionManager } from '../extension-manager.js';
|
||||
import { ExtensionStorage } from './storage.js';
|
||||
import { removeDirectoryWithRetry } from '../../utils/retry.js';
|
||||
|
||||
export interface ExtensionUpdateInfo {
|
||||
name: string;
|
||||
|
|
@ -145,7 +146,7 @@ export async function updateExtension(
|
|||
await copyExtension(tempDir, extension.path);
|
||||
throw e;
|
||||
} finally {
|
||||
await fs.promises.rm(tempDir, { recursive: true, force: true });
|
||||
await removeDirectoryWithRetry(tempDir);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
87
packages/cli/src/utils/retry.test.ts
Normal file
87
packages/cli/src/utils/retry.test.ts
Normal file
|
|
@ -0,0 +1,87 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import * as fs from 'node:fs';
|
||||
import { removeDirectoryWithRetry } from './retry.js';
|
||||
|
||||
vi.mock('node:fs', async () => {
|
||||
const actual = await vi.importActual<typeof import('node:fs')>('node:fs');
|
||||
return {
|
||||
...actual,
|
||||
promises: {
|
||||
...actual.promises,
|
||||
rm: vi.fn(),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
describe('removeDirectoryWithRetry', () => {
|
||||
const mockedRm = vi.mocked(fs.promises.rm);
|
||||
|
||||
beforeEach(() => {
|
||||
mockedRm.mockClear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should succeed on first attempt', async () => {
|
||||
mockedRm.mockResolvedValue(undefined);
|
||||
|
||||
await removeDirectoryWithRetry('/test/path');
|
||||
|
||||
expect(mockedRm).toHaveBeenCalledTimes(1);
|
||||
expect(mockedRm).toHaveBeenCalledWith('/test/path', {
|
||||
recursive: true,
|
||||
force: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('should retry on EBUSY error', async () => {
|
||||
const ebusyError = Object.assign(new Error('EBUSY'), { code: 'EBUSY' });
|
||||
mockedRm
|
||||
.mockRejectedValueOnce(ebusyError)
|
||||
.mockRejectedValueOnce(ebusyError)
|
||||
.mockResolvedValue(undefined);
|
||||
|
||||
await removeDirectoryWithRetry('/test/path');
|
||||
|
||||
expect(mockedRm).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it('should retry on ENOTEMPTY error', async () => {
|
||||
const notEmptyError = Object.assign(new Error('ENOTEMPTY'), {
|
||||
code: 'ENOTEMPTY',
|
||||
});
|
||||
mockedRm.mockRejectedValueOnce(notEmptyError).mockResolvedValue(undefined);
|
||||
|
||||
await removeDirectoryWithRetry('/test/path');
|
||||
|
||||
expect(mockedRm).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should throw after max retries', async () => {
|
||||
const ebusyError = Object.assign(new Error('EBUSY'), { code: 'EBUSY' });
|
||||
mockedRm.mockRejectedValue(ebusyError);
|
||||
|
||||
await expect(removeDirectoryWithRetry('/test/path')).rejects.toThrow(
|
||||
'EBUSY',
|
||||
);
|
||||
expect(mockedRm).toHaveBeenCalledTimes(5);
|
||||
});
|
||||
|
||||
it('should throw immediately on non-retryable errors', async () => {
|
||||
const otherError = new Error('Some other error');
|
||||
mockedRm.mockRejectedValue(otherError);
|
||||
|
||||
await expect(removeDirectoryWithRetry('/test/path')).rejects.toThrow(
|
||||
'Some other error',
|
||||
);
|
||||
expect(mockedRm).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
68
packages/cli/src/utils/retry.ts
Normal file
68
packages/cli/src/utils/retry.ts
Normal file
|
|
@ -0,0 +1,68 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import * as fs from 'node:fs';
|
||||
import { debugLogger } from '@google/gemini-cli-core';
|
||||
|
||||
/**
|
||||
* Retries a file system operation with exponential backoff.
|
||||
* Useful for Windows where file locks may cause EBUSY errors.
|
||||
*/
|
||||
async function retryWithBackoff<T>(
|
||||
operation: () => Promise<T>,
|
||||
maxRetries: number = 5,
|
||||
initialDelayMs: number = 100,
|
||||
): Promise<T> {
|
||||
let lastError: Error | undefined;
|
||||
|
||||
if (maxRetries <= 0) {
|
||||
throw new Error('maxRetries must be a positive number.');
|
||||
}
|
||||
|
||||
for (let attempt = 0; attempt < maxRetries; attempt++) {
|
||||
try {
|
||||
return await operation();
|
||||
} catch (e) {
|
||||
if (!(e instanceof Error)) {
|
||||
throw e;
|
||||
}
|
||||
lastError = e;
|
||||
|
||||
const code = (e as NodeJS.ErrnoException).code;
|
||||
if (code !== 'EBUSY' && code !== 'ENOTEMPTY' && code !== 'EPERM') {
|
||||
throw e;
|
||||
}
|
||||
|
||||
if (attempt < maxRetries - 1) {
|
||||
const delay = initialDelayMs * Math.pow(2, attempt);
|
||||
debugLogger.debug(
|
||||
`Retry attempt ${attempt + 1}/${maxRetries} after ${delay}ms due to: ${lastError.message}`,
|
||||
);
|
||||
await new Promise((resolve) => setTimeout(resolve, delay));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
throw lastError;
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes a directory recursively with retries for Windows file lock issues.
|
||||
* On Windows, file handles may remain open briefly after operations like copy,
|
||||
* causing EBUSY errors. This function retries with exponential backoff.
|
||||
*/
|
||||
export async function removeDirectoryWithRetry(
|
||||
path: string,
|
||||
options: { recursive?: boolean; force?: boolean } = {},
|
||||
): Promise<void> {
|
||||
await retryWithBackoff(
|
||||
async () => {
|
||||
await fs.promises.rm(path, { ...options, recursive: true, force: true });
|
||||
},
|
||||
5,
|
||||
100,
|
||||
);
|
||||
}
|
||||
Loading…
Reference in a new issue