Add fetch timeouts to MCP server HTTP calls#1111
Conversation
|
I took a look at this PR and I think we should go a step further on the auth timeouts and error handling.
Suggested changes for // For validateApiKey and validateOAuthToken
try {
const sessionResponse = await fetch(`${apiUrl}/v3/session`, {
// ...
signal: AbortSignal.timeout(2000), // Tighten to 2s
})
// ...
} catch (error: any) {
if (error.name === 'TimeoutError' || error.name === 'AbortError') {
throw new Error("Authentication request timed out. Please check your connection or try again.");
}
console.error("API key validation error:", error)
return null;
}And in private handleError(error: unknown): never {
if (error instanceof Error && (error.name === 'TimeoutError' || error.name === 'AbortError')) {
throw new Error("Request timed out. The backend might be overloaded, please try again in a moment.");
}
// ... rest of handler
}Let's make these errors explicit so the user knows exactly why the request failed! |
ishaanxgupta
left a comment
There was a problem hiding this comment.
The timeout handling in validateApiKey/validateOAuthToken should preserve the existing auth failure contract instead of throwing out of the validator. Before this change all validation failures returned null, letting callers respond through the normal unauthorized path; now a timeout throws Authentication request timed out..., which can bubble as a server error depending on the transport caller. Please convert timeout/abort into the same handled auth result, or update every caller to catch this new exception and return the intended MCP auth response.
|
Suggest refactoring This preserves the existing contract by avoiding |
5 fetch() calls in the MCP server were missing timeouts — auth validation, project listing, document fetching, OAuth metadata proxy. In CF Workers a hanging request blocks the Durable Object and eats CPU time.
Added AbortSignal.timeout() to each one. 10s for lightweight calls (auth, projects, metadata), 30s for document listing. Standard Web API, no new deps. Existing try/catch handles the TimeoutError that gets thrown.