fix(auth): Return empty map if token file does not exits, and refacto… (#12332)

Co-authored-by: Sandy Tao <sandytao520@icloud.com>
This commit is contained in:
Gal Zahavi
2025-11-03 15:07:22 -08:00
committed by GitHub
parent 7a515339ea
commit 2144d25885
6 changed files with 23 additions and 22 deletions

View File

@@ -433,6 +433,7 @@ Logging in with Google... Please restart Gemini CLI to continue.
const handleApiKeySubmit = useCallback(
async (apiKey: string) => {
try {
onAuthError(null);
if (!apiKey.trim() && apiKey.length > 1) {
onAuthError(
'API key cannot be empty string with length greater than 1.',

View File

@@ -79,4 +79,10 @@ describe('ApiKeyCredentialStorage', () => {
await clearApiKey();
expect(deleteCredentialsMock).toHaveBeenCalledWith('default-api-key');
});
it('should not throw when clearing an API key fails', async () => {
deleteCredentialsMock.mockRejectedValueOnce(new Error('Failed to delete'));
await expect(saveApiKey('')).resolves.not.toThrow();
expect(deleteCredentialsMock).toHaveBeenCalledWith('default-api-key');
});
});

View File

@@ -26,15 +26,6 @@ export async function loadApiKey(): Promise<string | null> {
return null;
} catch (error: unknown) {
// Ignore "file not found" error from FileTokenStorage, it just means no key is saved yet.
// This is common in fresh environments like e2e tests.
if (
error instanceof Error &&
error.message === 'Token file does not exist'
) {
return null;
}
// Log other errors but don't crash, just return null so user can re-enter key
debugLogger.error('Failed to load API key from storage:', error);
return null;
@@ -48,7 +39,12 @@ export async function saveApiKey(
apiKey: string | null | undefined,
): Promise<void> {
if (!apiKey || apiKey.trim() === '') {
await storage.deleteCredentials(DEFAULT_API_KEY_ENTRY);
try {
await storage.deleteCredentials(DEFAULT_API_KEY_ENTRY);
} catch (error: unknown) {
// Ignore errors when deleting, as it might not exist
debugLogger.warn('Failed to delete API key from storage:', error);
}
return;
}

View File

@@ -58,12 +58,11 @@ describe('FileTokenStorage', () => {
});
describe('getCredentials', () => {
it('should throw error when file does not exist', async () => {
it('should return null when file does not exist', async () => {
mockFs.readFile.mockRejectedValue({ code: 'ENOENT' });
await expect(storage.getCredentials('test-server')).rejects.toThrow(
'Token file does not exist',
);
const result = await storage.getCredentials('test-server');
expect(result).toBeNull();
});
it('should return null for expired tokens', async () => {
@@ -179,7 +178,7 @@ describe('FileTokenStorage', () => {
mockFs.readFile.mockRejectedValue({ code: 'ENOENT' });
await expect(storage.deleteCredentials('test-server')).rejects.toThrow(
'Token file does not exist',
'No credentials found for test-server',
);
});
@@ -246,12 +245,11 @@ describe('FileTokenStorage', () => {
});
describe('listServers', () => {
it('should throw error when file does not exist', async () => {
it('should return empty list when file does not exist', async () => {
mockFs.readFile.mockRejectedValue({ code: 'ENOENT' });
await expect(storage.listServers()).rejects.toThrow(
'Token file does not exist',
);
const result = await storage.listServers();
expect(result).toEqual([]);
});
it('should return list of server names', async () => {

View File

@@ -77,7 +77,7 @@ export class FileTokenStorage extends BaseTokenStorage {
} catch (error: unknown) {
const err = error as NodeJS.ErrnoException & { message?: string };
if (err.code === 'ENOENT') {
throw new Error('Token file does not exist');
return new Map();
}
if (
err.message?.includes('Invalid encrypted data format') ||

View File

@@ -46,8 +46,8 @@ export class KeychainTokenStorage
const moduleName = 'keytar';
const module = await import(moduleName);
this.keytarModule = module.default || module;
} catch (error) {
coreEvents.emitFeedback('error', "Failed to load 'keytar' module", error);
} catch (_) {
//Keytar is optional so we shouldn't raise an error of log anything.
}
return this.keytarModule;
}