mirror of
https://github.com/danny-avila/LibreChat.git
synced 2025-12-16 16:30:15 +01:00
🛡️ fix: Handle Null MCPManager In OAuthReconnectionManager (#9740)
This commit is contained in:
parent
a5195a57a4
commit
9d2aba5df5
2 changed files with 53 additions and 8 deletions
|
|
@ -290,5 +290,33 @@ describe('OAuthReconnectionManager', () => {
|
||||||
expect(reconnectionTracker.isActive(userId, 'server1')).toBe(false);
|
expect(reconnectionTracker.isActive(userId, 'server1')).toBe(false);
|
||||||
expect(mockMCPManager.disconnectUserConnection).toHaveBeenCalledWith(userId, 'server1');
|
expect(mockMCPManager.disconnectUserConnection).toHaveBeenCalledWith(userId, 'server1');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should handle MCPManager not available gracefully', async () => {
|
||||||
|
const userId = 'user-123';
|
||||||
|
|
||||||
|
// Reset singleton first
|
||||||
|
(OAuthReconnectionManager as unknown as { instance: null }).instance = null;
|
||||||
|
|
||||||
|
// Mock MCPManager.getInstance to throw (simulating no MCP manager available)
|
||||||
|
(MCPManager.getInstance as jest.Mock).mockImplementation(() => {
|
||||||
|
throw new Error('MCPManager has not been initialized.');
|
||||||
|
});
|
||||||
|
|
||||||
|
// Create a reconnection manager without MCPManager available
|
||||||
|
const reconnectionTracker = new OAuthReconnectionTracker();
|
||||||
|
const reconnectionManagerWithoutMCP = await OAuthReconnectionManager.createInstance(
|
||||||
|
flowManager,
|
||||||
|
tokenMethods,
|
||||||
|
reconnectionTracker,
|
||||||
|
);
|
||||||
|
|
||||||
|
// Verify that the method does not throw and completes successfully
|
||||||
|
await expect(reconnectionManagerWithoutMCP.reconnectServers(userId)).resolves.toBeUndefined();
|
||||||
|
|
||||||
|
// Verify that the method returns early without attempting any reconnections
|
||||||
|
expect(tokenMethods.findToken).not.toHaveBeenCalled();
|
||||||
|
expect(mockMCPManager.getUserConnection).not.toHaveBeenCalled();
|
||||||
|
expect(mockMCPManager.disconnectUserConnection).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -13,6 +13,7 @@ export class OAuthReconnectionManager {
|
||||||
|
|
||||||
protected readonly flowManager: FlowStateManager<MCPOAuthTokens | null>;
|
protected readonly flowManager: FlowStateManager<MCPOAuthTokens | null>;
|
||||||
protected readonly tokenMethods: TokenMethods;
|
protected readonly tokenMethods: TokenMethods;
|
||||||
|
private readonly mcpManager: MCPManager | null;
|
||||||
|
|
||||||
private readonly reconnectionsTracker: OAuthReconnectionTracker;
|
private readonly reconnectionsTracker: OAuthReconnectionTracker;
|
||||||
|
|
||||||
|
|
@ -46,6 +47,12 @@ export class OAuthReconnectionManager {
|
||||||
this.flowManager = flowManager;
|
this.flowManager = flowManager;
|
||||||
this.tokenMethods = tokenMethods;
|
this.tokenMethods = tokenMethods;
|
||||||
this.reconnectionsTracker = reconnections ?? new OAuthReconnectionTracker();
|
this.reconnectionsTracker = reconnections ?? new OAuthReconnectionTracker();
|
||||||
|
|
||||||
|
try {
|
||||||
|
this.mcpManager = MCPManager.getInstance();
|
||||||
|
} catch {
|
||||||
|
this.mcpManager = null;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
public isReconnecting(userId: string, serverName: string): boolean {
|
public isReconnecting(userId: string, serverName: string): boolean {
|
||||||
|
|
@ -53,11 +60,17 @@ export class OAuthReconnectionManager {
|
||||||
}
|
}
|
||||||
|
|
||||||
public async reconnectServers(userId: string) {
|
public async reconnectServers(userId: string) {
|
||||||
const mcpManager = MCPManager.getInstance();
|
// Check if MCPManager is available
|
||||||
|
if (this.mcpManager == null) {
|
||||||
|
logger.warn(
|
||||||
|
'[OAuthReconnectionManager] MCPManager not available, skipping OAuth MCP server reconnection',
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// 1. derive the servers to reconnect
|
// 1. derive the servers to reconnect
|
||||||
const serversToReconnect = [];
|
const serversToReconnect = [];
|
||||||
for (const serverName of mcpManager.getOAuthServers() ?? []) {
|
for (const serverName of this.mcpManager.getOAuthServers() ?? []) {
|
||||||
const canReconnect = await this.canReconnect(userId, serverName);
|
const canReconnect = await this.canReconnect(userId, serverName);
|
||||||
if (canReconnect) {
|
if (canReconnect) {
|
||||||
serversToReconnect.push(serverName);
|
serversToReconnect.push(serverName);
|
||||||
|
|
@ -81,23 +94,25 @@ export class OAuthReconnectionManager {
|
||||||
}
|
}
|
||||||
|
|
||||||
private async tryReconnect(userId: string, serverName: string) {
|
private async tryReconnect(userId: string, serverName: string) {
|
||||||
const mcpManager = MCPManager.getInstance();
|
if (this.mcpManager == null) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
const logPrefix = `[tryReconnectOAuthMCPServer][User: ${userId}][${serverName}]`;
|
const logPrefix = `[tryReconnectOAuthMCPServer][User: ${userId}][${serverName}]`;
|
||||||
|
|
||||||
logger.info(`${logPrefix} Attempting reconnection`);
|
logger.info(`${logPrefix} Attempting reconnection`);
|
||||||
|
|
||||||
const config = mcpManager.getRawConfig(serverName);
|
const config = this.mcpManager.getRawConfig(serverName);
|
||||||
|
|
||||||
const cleanupOnFailedReconnect = () => {
|
const cleanupOnFailedReconnect = () => {
|
||||||
this.reconnectionsTracker.setFailed(userId, serverName);
|
this.reconnectionsTracker.setFailed(userId, serverName);
|
||||||
this.reconnectionsTracker.removeActive(userId, serverName);
|
this.reconnectionsTracker.removeActive(userId, serverName);
|
||||||
mcpManager.disconnectUserConnection(userId, serverName);
|
this.mcpManager?.disconnectUserConnection(userId, serverName);
|
||||||
};
|
};
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// attempt to get connection (this will use existing tokens and refresh if needed)
|
// attempt to get connection (this will use existing tokens and refresh if needed)
|
||||||
const connection = await mcpManager.getUserConnection({
|
const connection = await this.mcpManager.getUserConnection({
|
||||||
serverName,
|
serverName,
|
||||||
user: { id: userId } as TUser,
|
user: { id: userId } as TUser,
|
||||||
flowManager: this.flowManager,
|
flowManager: this.flowManager,
|
||||||
|
|
@ -125,7 +140,9 @@ export class OAuthReconnectionManager {
|
||||||
}
|
}
|
||||||
|
|
||||||
private async canReconnect(userId: string, serverName: string) {
|
private async canReconnect(userId: string, serverName: string) {
|
||||||
const mcpManager = MCPManager.getInstance();
|
if (this.mcpManager == null) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
// if the server has failed reconnection, don't attempt to reconnect
|
// if the server has failed reconnection, don't attempt to reconnect
|
||||||
if (this.reconnectionsTracker.isFailed(userId, serverName)) {
|
if (this.reconnectionsTracker.isFailed(userId, serverName)) {
|
||||||
|
|
@ -133,7 +150,7 @@ export class OAuthReconnectionManager {
|
||||||
}
|
}
|
||||||
|
|
||||||
// if the server is already connected, don't attempt to reconnect
|
// if the server is already connected, don't attempt to reconnect
|
||||||
const existingConnections = mcpManager.getUserConnections(userId);
|
const existingConnections = this.mcpManager.getUserConnections(userId);
|
||||||
if (existingConnections?.has(serverName)) {
|
if (existingConnections?.has(serverName)) {
|
||||||
const isConnected = await existingConnections.get(serverName)?.isConnected();
|
const isConnected = await existingConnections.get(serverName)?.isConnected();
|
||||||
if (isConnected) {
|
if (isConnected) {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue