mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
fix: Release timed-out MCP refresh locks
This commit is contained in:
parent
7f60fdf38e
commit
243fa72d0e
2 changed files with 114 additions and 14 deletions
|
|
@ -86,6 +86,7 @@ export class MCPConnectionFactory {
|
|||
* connect budget for OAuth discovery, registration, and `oauthStart`.
|
||||
*/
|
||||
private static readonly SILENT_REFRESH_TIMEOUT_MS = 5_000;
|
||||
private static readonly SILENT_REFRESH_ABORT_GRACE_MS = 1_000;
|
||||
|
||||
/** Creates a new MCP connection with optional OAuth support */
|
||||
static async create(
|
||||
|
|
@ -534,10 +535,15 @@ export class MCPConnectionFactory {
|
|||
const timeoutMs = this.getSilentRefreshTimeoutMs();
|
||||
const abortController = new AbortController();
|
||||
let timeoutId: ReturnType<typeof setTimeout> | null = null;
|
||||
let abortGraceTimeoutId: ReturnType<typeof setTimeout> | null = null;
|
||||
const refreshPromise = this.runSilentRefresh(abortController.signal);
|
||||
const promise = new Promise<MCPOAuthTokens | null>((resolve) => {
|
||||
timeoutId = setTimeout(() => {
|
||||
abortController.abort();
|
||||
abortGraceTimeoutId = setTimeout(
|
||||
releaseLock,
|
||||
MCPConnectionFactory.SILENT_REFRESH_ABORT_GRACE_MS,
|
||||
);
|
||||
logger.info(
|
||||
`${this.logPrefix} Silent token refresh timed out after ${timeoutMs}ms, falling back to interactive OAuth`,
|
||||
);
|
||||
|
|
@ -556,19 +562,17 @@ export class MCPConnectionFactory {
|
|||
clearTimeout(timeoutId);
|
||||
}
|
||||
});
|
||||
function releaseLock() {
|
||||
if (abortGraceTimeoutId) {
|
||||
clearTimeout(abortGraceTimeoutId);
|
||||
abortGraceTimeoutId = null;
|
||||
}
|
||||
if (MCPConnectionFactory.inflightSilentRefreshes.get(lockKey) === promise) {
|
||||
MCPConnectionFactory.inflightSilentRefreshes.delete(lockKey);
|
||||
}
|
||||
}
|
||||
MCPConnectionFactory.inflightSilentRefreshes.set(lockKey, promise);
|
||||
void refreshPromise.then(
|
||||
() => {
|
||||
if (MCPConnectionFactory.inflightSilentRefreshes.get(lockKey) === promise) {
|
||||
MCPConnectionFactory.inflightSilentRefreshes.delete(lockKey);
|
||||
}
|
||||
},
|
||||
() => {
|
||||
if (MCPConnectionFactory.inflightSilentRefreshes.get(lockKey) === promise) {
|
||||
MCPConnectionFactory.inflightSilentRefreshes.delete(lockKey);
|
||||
}
|
||||
},
|
||||
);
|
||||
void refreshPromise.then(releaseLock, releaseLock);
|
||||
return await promise;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1323,8 +1323,9 @@ describe('MCPConnectionFactory', () => {
|
|||
|
||||
it('should keep the in-flight silent-refresh lock until the aborted refresh settles', async () => {
|
||||
// A timed-out caller should fall back to interactive OAuth immediately,
|
||||
// but the shared lock must remain until the underlying abortable refresh
|
||||
// settles so another 401 cannot redeem the same rotating refresh token.
|
||||
// but the shared lock must remain briefly while the underlying abortable
|
||||
// refresh settles so another 401 cannot immediately redeem the same
|
||||
// rotating refresh token.
|
||||
const sseConfig = {
|
||||
...mockServerConfig,
|
||||
url: 'https://api.example.com',
|
||||
|
|
@ -1427,6 +1428,101 @@ describe('MCPConnectionFactory', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('should drop timed-out silent-refresh locks after abort grace if refresh stays hung', async () => {
|
||||
const sseConfig = {
|
||||
...mockServerConfig,
|
||||
url: 'https://api.example.com',
|
||||
type: 'sse' as const,
|
||||
} as t.SSEOptions;
|
||||
|
||||
const basicOptions = {
|
||||
serverName: 'test-server',
|
||||
serverConfig: sseConfig,
|
||||
};
|
||||
|
||||
const oauthOptions = {
|
||||
useOAuth: true as const,
|
||||
user: mockUser,
|
||||
flowManager: mockFlowManager,
|
||||
oauthStart: jest.fn(),
|
||||
oauthEnd: jest.fn(),
|
||||
tokenMethods: {
|
||||
findToken: jest.fn(),
|
||||
createToken: jest.fn(),
|
||||
updateToken: jest.fn(),
|
||||
deleteTokens: jest.fn(),
|
||||
},
|
||||
};
|
||||
|
||||
mockProcessMCPEnv.mockReturnValue(sseConfig);
|
||||
mockMCPOAuthHandler.generateFlowId.mockReturnValue('flow123');
|
||||
let firstRefreshSignal: AbortSignal | undefined;
|
||||
let secondRefreshSignal: AbortSignal | undefined;
|
||||
mockMCPTokenStorage.forceRefreshTokens
|
||||
.mockImplementationOnce((params) => {
|
||||
firstRefreshSignal = params.signal;
|
||||
return new Promise<MCPOAuthTokens>(() => {});
|
||||
})
|
||||
.mockImplementationOnce((params) => {
|
||||
secondRefreshSignal = params.signal;
|
||||
return Promise.resolve(null as unknown as MCPOAuthTokens);
|
||||
});
|
||||
mockMCPOAuthHandler.initiateOAuthFlow.mockResolvedValue({
|
||||
authorizationUrl: 'https://auth.example.com',
|
||||
flowId: 'flow123',
|
||||
flowMetadata: {
|
||||
serverName: 'test-server',
|
||||
userId: 'user123',
|
||||
serverUrl: 'https://api.example.com',
|
||||
state: 'state-x',
|
||||
},
|
||||
});
|
||||
mockFlowManager.getFlowState.mockResolvedValue(null);
|
||||
mockFlowManager.createFlow.mockResolvedValue(null);
|
||||
mockConnectionInstance.connect.mockRejectedValue(new Error('OAuth authentication required'));
|
||||
mockConnectionInstance.isConnected.mockResolvedValue(false);
|
||||
|
||||
let oauthRequiredHandler: (data: Record<string, unknown>) => Promise<void>;
|
||||
mockConnectionInstance.on.mockImplementation((event, handler) => {
|
||||
if (event === 'oauthRequired') {
|
||||
oauthRequiredHandler = handler as (data: Record<string, unknown>) => Promise<void>;
|
||||
}
|
||||
return mockConnectionInstance;
|
||||
});
|
||||
|
||||
try {
|
||||
await MCPConnectionFactory.create(basicOptions, oauthOptions);
|
||||
} catch {
|
||||
// Expected
|
||||
}
|
||||
|
||||
jest.useFakeTimers();
|
||||
try {
|
||||
const firstAttempt = oauthRequiredHandler!({ serverUrl: 'https://api.example.com' });
|
||||
await jest.advanceTimersByTimeAsync(2_001);
|
||||
await firstAttempt;
|
||||
|
||||
const inflightMap = (
|
||||
MCPConnectionFactory as unknown as {
|
||||
inflightSilentRefreshes: Map<string, unknown>;
|
||||
}
|
||||
).inflightSilentRefreshes;
|
||||
expect(inflightMap.size).toBe(1);
|
||||
expect(firstRefreshSignal?.aborted).toBe(true);
|
||||
|
||||
await jest.advanceTimersByTimeAsync(1_001);
|
||||
expect(inflightMap.size).toBe(0);
|
||||
|
||||
const secondAttempt = oauthRequiredHandler!({ serverUrl: 'https://api.example.com' });
|
||||
await secondAttempt;
|
||||
|
||||
expect(mockMCPTokenStorage.forceRefreshTokens).toHaveBeenCalledTimes(2);
|
||||
expect(secondRefreshSignal).toBeDefined();
|
||||
} finally {
|
||||
jest.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('should reserve interactive OAuth fallback time when initTimeout is omitted', async () => {
|
||||
const sseConfig = {
|
||||
url: 'https://api.example.com',
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue