migrating console.error to debugger for installationManager, oauth-provider, modifiable-tool (#12279)

This commit is contained in:
Sehoon Shon
2025-10-31 14:17:51 -04:00
committed by GitHub
parent c158923b27
commit ab013fb7e9
5 changed files with 26 additions and 11 deletions
+10 -2
View File
@@ -27,6 +27,11 @@ vi.mock('./oauth-token-storage.js', () => {
})), })),
}; };
}); });
vi.mock('../utils/events.js', () => ({
coreEvents: {
emitFeedback: vi.fn(),
},
}));
import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import * as http from 'node:http'; import * as http from 'node:http';
@@ -43,6 +48,7 @@ import type {
OAuthAuthorizationServerMetadata, OAuthAuthorizationServerMetadata,
OAuthProtectedResourceMetadata, OAuthProtectedResourceMetadata,
} from './oauth-utils.js'; } from './oauth-utils.js';
import { coreEvents } from '../utils/events.js';
// Mock fetch globally // Mock fetch globally
const mockFetch = vi.fn(); const mockFetch = vi.fn();
@@ -938,8 +944,10 @@ describe('MCPOAuthProvider', () => {
expect(tokenStorage.deleteCredentials).toHaveBeenCalledWith( expect(tokenStorage.deleteCredentials).toHaveBeenCalledWith(
'test-server', 'test-server',
); );
expect(console.error).toHaveBeenCalledWith( expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
expect.stringContaining('Failed to refresh token'), 'error',
expect.stringContaining('Failed to refresh auth token'),
expect.any(Error),
); );
}); });
+8 -3
View File
@@ -13,6 +13,7 @@ import type { OAuthToken } from './token-storage/types.js';
import { MCPOAuthTokenStorage } from './oauth-token-storage.js'; import { MCPOAuthTokenStorage } from './oauth-token-storage.js';
import { getErrorMessage } from '../utils/errors.js'; import { getErrorMessage } from '../utils/errors.js';
import { OAuthUtils } from './oauth-utils.js'; import { OAuthUtils } from './oauth-utils.js';
import { coreEvents } from '../utils/events.js';
import { debugLogger } from '../utils/debugLogger.js'; import { debugLogger } from '../utils/debugLogger.js';
export const OAUTH_DISPLAY_MESSAGE_EVENT = 'oauth-display-message' as const; export const OAUTH_DISPLAY_MESSAGE_EVENT = 'oauth-display-message' as const;
@@ -811,12 +812,12 @@ ${authUrl}
`✓ Token verification successful (fingerprint: ${tokenFingerprint})`, `✓ Token verification successful (fingerprint: ${tokenFingerprint})`,
); );
} else { } else {
console.error( debugLogger.warn(
'Token verification failed: token not found or invalid after save', 'Token verification failed: token not found or invalid after save',
); );
} }
} catch (saveError) { } catch (saveError) {
console.error(`Failed to save token: ${getErrorMessage(saveError)}`); debugLogger.error('Failed to save auth token.', saveError);
throw saveError; throw saveError;
} }
@@ -889,7 +890,11 @@ ${authUrl}
return newToken.accessToken; return newToken.accessToken;
} catch (error) { } catch (error) {
console.error(`Failed to refresh token: ${getErrorMessage(error)}`); coreEvents.emitFeedback(
'error',
'Failed to refresh auth token.',
error,
);
// Remove invalid token // Remove invalid token
await this.tokenStorage.deleteCredentials(serverName); await this.tokenStorage.deleteCredentials(serverName);
} }
+3 -2
View File
@@ -17,6 +17,7 @@ import type {
DeclarativeTool, DeclarativeTool,
ToolResult, ToolResult,
} from './tools.js'; } from './tools.js';
import { debugLogger } from '../utils/debugLogger.js';
/** /**
* A declarative tool that supports a modify operation. * A declarative tool that supports a modify operation.
@@ -128,13 +129,13 @@ function deleteTempFiles(oldPath: string, newPath: string): void {
try { try {
fs.unlinkSync(oldPath); fs.unlinkSync(oldPath);
} catch { } catch {
console.error(`Error deleting temp diff file: ${oldPath}`); debugLogger.error(`Error deleting temp diff file: ${oldPath}`);
} }
try { try {
fs.unlinkSync(newPath); fs.unlinkSync(newPath);
} catch { } catch {
console.error(`Error deleting temp diff file: ${newPath}`); debugLogger.error(`Error deleting temp diff file: ${newPath}`);
} }
} }
@@ -91,14 +91,14 @@ describe('InstallationManager', () => {
readSpy.mockImplementationOnce(() => { readSpy.mockImplementationOnce(() => {
throw new Error('Read error'); throw new Error('Read error');
}); });
const consoleErrorSpy = vi const consoleWarnSpy = vi
.spyOn(console, 'error') .spyOn(console, 'warn')
.mockImplementation(() => {}); .mockImplementation(() => {});
const id = installationManager.getInstallationId(); const id = installationManager.getInstallationId();
expect(id).toBe('123456789'); expect(id).toBe('123456789');
expect(consoleErrorSpy).toHaveBeenCalled(); expect(consoleWarnSpy).toHaveBeenCalled();
}); });
}); });
}); });
@@ -8,6 +8,7 @@ import * as fs from 'node:fs';
import { randomUUID } from 'node:crypto'; import { randomUUID } from 'node:crypto';
import * as path from 'node:path'; import * as path from 'node:path';
import { Storage } from '../config/storage.js'; import { Storage } from '../config/storage.js';
import { debugLogger } from './debugLogger.js';
export class InstallationManager { export class InstallationManager {
private getInstallationIdPath(): string { private getInstallationIdPath(): string {
@@ -48,7 +49,7 @@ export class InstallationManager {
return installationId; return installationId;
} catch (error) { } catch (error) {
console.error( debugLogger.warn(
'Error accessing installation ID file, generating ephemeral ID:', 'Error accessing installation ID file, generating ephemeral ID:',
error, error,
); );