Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
278 changes: 175 additions & 103 deletions apps/gateway/src/chat/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4818,6 +4818,7 @@ chat.openapi(completions, async (c) => {
let isTimeoutFetchError = false;
let res: Response | undefined;
let duration = 0;
let json: any;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

let json: any violates the no-any guideline — prefer unknown

res.json() returns any, but the outer binding can be narrowed safely:

🔧 Proposed fix
-	let json: any;
+	let json: unknown;

As per coding guidelines: "Never use any or as any type assertions in TypeScript code unless absolutely necessary."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let json: any;
let json: unknown;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/gateway/src/chat/chat.ts` at line 4849, The binding "let json: any"
should use a safer type: change it to "let json: unknown" and then narrow it
after calling res.json() before using it (e.g., with a type guard, instanceof
checks, or explicit validation) so downstream code operates on a known shape;
update the code around the res.json() call (where "json" is assigned) to perform
the necessary runtime checks/casts and only then access properties or cast to a
specific interface.

const finalLogId = shortid();
for (let retryAttempt = 0; retryAttempt <= MAX_RETRIES; retryAttempt++) {
const perAttemptStartTime = Date.now();
Expand Down Expand Up @@ -5275,6 +5276,19 @@ chat.openapi(completions, async (c) => {
status: res.status,
});

// Check if we should retry before logging so we can mark the log as retried
const willRetryBodyTimeoutNonStreaming = shouldRetryRequest({
requestedProvider,
noFallback,
statusCode: res.status,
retryCount: retryAttempt,
remainingProviders:
(routingMetadata?.providerScores.length ?? 0) -
failedProviderIds.size -
1,
usedProvider,
});

const bodyTimeoutPluginIds = plugins?.map((p) => p.id) ?? [];
const baseLogEntry = createLogEntry(
requestId,
Expand Down Expand Up @@ -5346,8 +5360,29 @@ chat.openapi(completions, async (c) => {
dataStorageCost: "0",
cached: false,
toolResults: null,
retried: willRetryBodyTimeoutNonStreaming,
retriedByLogId: willRetryBodyTimeoutNonStreaming
? finalLogId
: null,
});

// Report key health for environment-based tokens
if (envVarName !== undefined) {
reportKeyError(envVarName, configIndex, res.status);
}

if (willRetryBodyTimeoutNonStreaming) {
routingAttempts.push({
provider: usedProvider,
model: usedModel,
status_code: res.status,
error_type: getErrorType(res.status),
succeeded: false,
});
failedProviderIds.add(usedProvider);
continue;
}

return c.json(
{
error: {
Expand Down Expand Up @@ -5577,7 +5612,145 @@ chat.openapi(completions, async (c) => {
);
}

break; // Fetch succeeded, exit retry loop
// At this point, res must be defined and ok (otherwise we would have continued/returned above)
if (!res || !res.ok) {
throw new Error("Response not ok after error handling");
}

// Parse response body before exiting retry loop so we can retry on timeout
// Body read can throw TimeoutError if the abort signal fires during consumption
try {
json = await res.json();
} catch (bodyError) {
if (isTimeoutError(bodyError)) {
const errorMessage =
bodyError instanceof Error
? bodyError.message
: "Timeout reading response body";
logger.warn("Timeout reading response body", {
usedProvider,
usedModel,
initialRequestedModel,
});

// Check if we should retry before logging so we can mark the log as retried
const willRetrySuccessBodyTimeoutNonStreaming = shouldRetryRequest({
requestedProvider,
noFallback,
statusCode: res.status,
retryCount: retryAttempt,
remainingProviders:
(routingMetadata?.providerScores.length ?? 0) -
failedProviderIds.size -
1,
usedProvider,
});
Comment on lines +5637 to +5647

Copilot AI Feb 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential bug: When a timeout occurs while reading the body of a successful response (2xx status code), shouldRetryRequest will always return false because isRetryableError does not consider 2xx status codes as retryable. This means willRetrySuccessBodyTimeoutNonStreaming will always be false, so the retry logic will never execute and the error will be returned immediately with no retry attempt. Consider passing a different status code (like 0 for timeout errors, or 504) to shouldRetryRequest to ensure the timeout is treated as retryable.

Copilot uses AI. Check for mistakes.

const bodyTimeoutPluginIds = plugins?.map((p) => p.id) ?? [];
const baseLogEntry = createLogEntry(
requestId,
project,
apiKey,
providerKey?.id,
usedModelFormatted,
usedModelMapping,
usedProvider,
initialRequestedModel,
requestedProvider,
messages,
temperature,
max_tokens,
top_p,
frequency_penalty,
presence_penalty,
reasoning_effort,
reasoning_max_tokens,
effort,
response_format,
tools,
tool_choice,
source,
customHeaders,
debugMode,
userAgent,
image_config,
routingMetadata,
rawBody,
null,
requestBody,
null,
bodyTimeoutPluginIds,
undefined,
);

await insertLog({
...baseLogEntry,
duration: Date.now() - perAttemptStartTime,
timeToFirstToken: null,
timeToFirstReasoningToken: null,
responseSize: 0,
content: null,
reasoningContent: null,
finishReason: "upstream_error",
promptTokens: null,
completionTokens: null,
totalTokens: null,
reasoningTokens: null,
cachedTokens: null,
hasError: true,
streamed: false,
canceled: false,
errorDetails: {
statusCode: 0,
statusText: "TimeoutError",
responseText: errorMessage,
},
cachedInputCost: null,
requestCost: null,
webSearchCost: null,
imageInputTokens: null,
imageOutputTokens: null,
imageInputCost: null,
imageOutputCost: null,
estimatedCost: false,
discount: null,
dataStorageCost: "0",
cached: false,
toolResults: null,
retried: willRetrySuccessBodyTimeoutNonStreaming,
retriedByLogId: willRetrySuccessBodyTimeoutNonStreaming
? finalLogId
: null,
});

if (willRetrySuccessBodyTimeoutNonStreaming) {
routingAttempts.push({
provider: usedProvider,
model: usedModel,
status_code: 0,
error_type: getErrorType(0),
succeeded: false,
});
failedProviderIds.add(usedProvider);
continue;
}

return c.json(
{
error: {
message: `Upstream provider timeout: ${errorMessage}`,
type: "upstream_timeout",
param: null,
code: "timeout",
},
},
504,
);
}
throw bodyError;
}

break; // Fetch and body parse succeeded, exit retry loop
Comment on lines +5615 to +5753

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find shouldRetryRequest function definition
echo "=== Searching for shouldRetryRequest function ==="
rg -n "function shouldRetryRequest|const shouldRetryRequest|export.*shouldRetryRequest" --type ts -A 20 | head -100

echo ""
echo "=== Searching for getErrorType function ==="
rg -n "function getErrorType|const getErrorType|export.*getErrorType" --type ts -A 10 | head -80

Repository: theopenco/llmgateway

Length of output: 3192


🏁 Script executed:

#!/bin/bash
# Get context around the shouldRetryRequest calls in the changed lines
echo "=== Context around shouldRetryRequest call at line 5662-5672 (success body timeout) ==="
sed -n '5660,5675p' apps/gateway/src/chat/chat.ts

echo ""
echo "=== Context around shouldRetryRequest call at line 5307 (error body timeout) ==="
sed -n '5305,5320p' apps/gateway/src/chat/chat.ts

Repository: theopenco/llmgateway

Length of output: 1227


🏁 Script executed:

#!/bin/bash
# Search for where shouldRetryRequest is defined or imported
echo "=== Import or definition of shouldRetryRequest ==="
rg -n "shouldRetryRequest" apps/gateway/src/chat/chat.ts | head -20

Repository: theopenco/llmgateway

Length of output: 588


🏁 Script executed:

#!/bin/bash
# Find isRetryableError implementation
echo "=== Searching for isRetryableError function ==="
rg -n "function isRetryableError|const isRetryableError|export.*isRetryableError" --type ts -A 20

Repository: theopenco/llmgateway

Length of output: 1919


🏁 Script executed:

#!/bin/bash
# Also check if isRetryableError is imported from elsewhere
echo "=== Check imports and all references to isRetryableError ==="
rg -n "isRetryableError" apps/gateway/src/chat/tools/retry-with-fallback.ts

Repository: theopenco/llmgateway

Length of output: 237


Pass statusCode: 0 for body-read timeouts to enable actual retries

The success-response body timeout block passes statusCode: res.status (200) to shouldRetryRequest, but isRetryableError() only considers 429, 5xx, and 0 as retryable. Status 200 fails the check, preventing retries — defeating the PR's goal.

Similarly, getErrorType(200) returns "upstream_error", misclassifying a timeout as a provider error.

Apply the proposed fix: pass statusCode: 0 for timeout errors (matching the fetch-timeout pattern at line 5029) to signal transport-level failure:

Proposed changes
 			const willRetrySuccessBodyTimeoutNonStreaming = shouldRetryRequest({
 				requestedProvider,
 				noFallback,
-				statusCode: res.status,
+				statusCode: 0,
 				retryCount: retryAttempt,
 				remainingProviders:
 					(routingMetadata?.providerScores.length ?? 0) -
 					failedProviderIds.size -
 					1,
 				usedProvider,
 			});
 			if (willRetrySuccessBodyTimeoutNonStreaming) {
 				routingAttempts.push({
 					provider: usedProvider,
 					model: usedModel,
-					status_code: res.status,
-					error_type: getErrorType(res.status),
+					status_code: 0,
+					error_type: getErrorType(0),
 					succeeded: false,
 				});

Also update the error tracking (around line 5729 and 5753):

-				reportKeyError(envVarName, configIndex, res.status);
+				reportKeyError(envVarName, configIndex, 0);
 				errorDetails: {
-					statusCode: res.status,
+					statusCode: 0,
 					statusText: "TimeoutError",
 					responseText: errorMessage,
 				},
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// At this point, res must be defined and ok (otherwise we would have continued/returned above)
if (!res || !res.ok) {
throw new Error("Response not ok after error handling");
}
// Parse response body before exiting retry loop so we can retry on timeout
// Body read can throw TimeoutError if the abort signal fires during consumption
try {
json = await res.json();
} catch (bodyError) {
if (isTimeoutError(bodyError)) {
const errorMessage =
bodyError instanceof Error
? bodyError.message
: "Timeout reading response body";
logger.warn("Timeout reading response body", {
usedProvider,
usedModel,
initialRequestedModel,
});
// Check if we should retry before logging so we can mark the log as retried
const willRetrySuccessBodyTimeoutNonStreaming = shouldRetryRequest({
requestedProvider,
noFallback,
statusCode: res.status,
retryCount: retryAttempt,
remainingProviders:
(routingMetadata?.providerScores.length ?? 0) -
failedProviderIds.size -
1,
usedProvider,
});
const bodyTimeoutPluginIds = plugins?.map((p) => p.id) || [];
const baseLogEntry = createLogEntry(
requestId,
project,
apiKey,
providerKey?.id,
usedModelFormatted,
usedModelMapping,
usedProvider,
initialRequestedModel,
requestedProvider,
messages,
temperature,
max_tokens,
top_p,
frequency_penalty,
presence_penalty,
reasoning_effort,
reasoning_max_tokens,
effort,
response_format,
tools,
tool_choice,
source,
customHeaders,
debugMode,
userAgent,
image_config,
routingMetadata,
rawBody,
null,
requestBody,
null,
bodyTimeoutPluginIds,
undefined,
);
await insertLog({
...baseLogEntry,
duration: Date.now() - perAttemptStartTime,
timeToFirstToken: null,
timeToFirstReasoningToken: null,
responseSize: 0,
content: null,
reasoningContent: null,
finishReason: "upstream_error",
promptTokens: null,
completionTokens: null,
totalTokens: null,
reasoningTokens: null,
cachedTokens: null,
hasError: true,
streamed: false,
canceled: false,
errorDetails: {
statusCode: res.status,
statusText: "TimeoutError",
responseText: errorMessage,
},
cachedInputCost: null,
requestCost: null,
webSearchCost: null,
imageInputTokens: null,
imageOutputTokens: null,
imageInputCost: null,
imageOutputCost: null,
estimatedCost: false,
discount: null,
dataStorageCost: "0",
cached: false,
toolResults: null,
retried: willRetrySuccessBodyTimeoutNonStreaming,
retriedByLogId: willRetrySuccessBodyTimeoutNonStreaming
? finalLogId
: null,
});
// Report key health for environment-based tokens
if (envVarName !== undefined) {
reportKeyError(envVarName, configIndex, res.status);
}
if (willRetrySuccessBodyTimeoutNonStreaming) {
routingAttempts.push({
provider: usedProvider,
model: usedModel,
status_code: res.status,
error_type: getErrorType(res.status),
succeeded: false,
});
failedProviderIds.add(usedProvider);
continue;
}
return c.json(
{
error: {
message: `Upstream provider timeout: ${errorMessage}`,
type: "upstream_timeout",
param: null,
code: "timeout",
},
},
504,
);
}
throw bodyError;
}
break; // Fetch and body parse succeeded, exit retry loop
// At this point, res must be defined and ok (otherwise we would have continued/returned above)
if (!res || !res.ok) {
throw new Error("Response not ok after error handling");
}
// Parse response body before exiting retry loop so we can retry on timeout
// Body read can throw TimeoutError if the abort signal fires during consumption
try {
json = await res.json();
} catch (bodyError) {
if (isTimeoutError(bodyError)) {
const errorMessage =
bodyError instanceof Error
? bodyError.message
: "Timeout reading response body";
logger.warn("Timeout reading response body", {
usedProvider,
usedModel,
initialRequestedModel,
});
// Check if we should retry before logging so we can mark the log as retried
const willRetrySuccessBodyTimeoutNonStreaming = shouldRetryRequest({
requestedProvider,
noFallback,
statusCode: 0,
retryCount: retryAttempt,
remainingProviders:
(routingMetadata?.providerScores.length ?? 0) -
failedProviderIds.size -
1,
usedProvider,
});
const bodyTimeoutPluginIds = plugins?.map((p) => p.id) || [];
const baseLogEntry = createLogEntry(
requestId,
project,
apiKey,
providerKey?.id,
usedModelFormatted,
usedModelMapping,
usedProvider,
initialRequestedModel,
requestedProvider,
messages,
temperature,
max_tokens,
top_p,
frequency_penalty,
presence_penalty,
reasoning_effort,
reasoning_max_tokens,
effort,
response_format,
tools,
tool_choice,
source,
customHeaders,
debugMode,
userAgent,
image_config,
routingMetadata,
rawBody,
null,
requestBody,
null,
bodyTimeoutPluginIds,
undefined,
);
await insertLog({
...baseLogEntry,
duration: Date.now() - perAttemptStartTime,
timeToFirstToken: null,
timeToFirstReasoningToken: null,
responseSize: 0,
content: null,
reasoningContent: null,
finishReason: "upstream_error",
promptTokens: null,
completionTokens: null,
totalTokens: null,
reasoningTokens: null,
cachedTokens: null,
hasError: true,
streamed: false,
canceled: false,
errorDetails: {
statusCode: 0,
statusText: "TimeoutError",
responseText: errorMessage,
},
cachedInputCost: null,
requestCost: null,
webSearchCost: null,
imageInputTokens: null,
imageOutputTokens: null,
imageInputCost: null,
imageOutputCost: null,
estimatedCost: false,
discount: null,
dataStorageCost: "0",
cached: false,
toolResults: null,
retried: willRetrySuccessBodyTimeoutNonStreaming,
retriedByLogId: willRetrySuccessBodyTimeoutNonStreaming
? finalLogId
: null,
});
// Report key health for environment-based tokens
if (envVarName !== undefined) {
reportKeyError(envVarName, configIndex, 0);
}
if (willRetrySuccessBodyTimeoutNonStreaming) {
routingAttempts.push({
provider: usedProvider,
model: usedModel,
status_code: 0,
error_type: getErrorType(0),
succeeded: false,
});
failedProviderIds.add(usedProvider);
continue;
}
return c.json(
{
error: {
message: `Upstream provider timeout: ${errorMessage}`,
type: "upstream_timeout",
param: null,
code: "timeout",
},
},
504,
);
}
throw bodyError;
}
break; // Fetch and body parse succeeded, exit retry loop
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/gateway/src/chat/chat.ts` around lines 5640 - 5783, The body-read
timeout is using res.status (e.g., 200) which prevents retries and misclassifies
the error; change places that currently pass res.status for timeouts to use
statusCode: 0 instead: call shouldRetryRequest with statusCode: 0, set
errorDetails.statusCode to 0 in the insertLog payload, set the routingAttempts
entry's status_code to 0 and use getErrorType(0) for error_type, and when
calling reportKeyError pass 0 as the status; update any other uses in this block
that rely on res.status for error classification to use 0 so transport-level
timeout is treated as retryable/transport error.

} // End of retry for loop
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Add the final attempt (successful or last failed) to routing
Expand Down Expand Up @@ -5635,108 +5808,7 @@ chat.openapi(completions, async (c) => {
throw new Error("No provider context after retry loop");
}

let json: any;
try {
json = await res.json();
} catch (bodyError) {
if (isTimeoutError(bodyError)) {
const errorMessage =
bodyError instanceof Error
? bodyError.message
: "Timeout reading response body";
logger.warn("Timeout reading response body", {
usedProvider,
usedModel,
initialRequestedModel,
});

const bodyTimeoutPluginIds = plugins?.map((p) => p.id) ?? [];
const baseLogEntry = createLogEntry(
requestId,
project,
apiKey,
providerKey?.id,
usedModelFormatted!,
usedModelMapping,
usedProvider,
initialRequestedModel,
requestedProvider,
messages,
temperature,
max_tokens,
top_p,
frequency_penalty,
presence_penalty,
reasoning_effort,
reasoning_max_tokens,
effort,
response_format,
tools,
tool_choice,
source,
customHeaders,
debugMode,
userAgent,
image_config,
routingMetadata,
rawBody,
null,
requestBody,
null,
bodyTimeoutPluginIds,
undefined,
);

await insertLog({
...baseLogEntry,
duration: Date.now() - startTime,
timeToFirstToken: null,
timeToFirstReasoningToken: null,
responseSize: 0,
content: null,
reasoningContent: null,
finishReason: "upstream_error",
promptTokens: null,
completionTokens: null,
totalTokens: null,
reasoningTokens: null,
cachedTokens: null,
hasError: true,
streamed: false,
canceled: false,
errorDetails: {
statusCode: res.status,
statusText: "TimeoutError",
responseText: errorMessage,
},
cachedInputCost: null,
requestCost: null,
webSearchCost: null,
imageInputTokens: null,
imageOutputTokens: null,
imageInputCost: null,
imageOutputCost: null,
estimatedCost: false,
discount: null,
dataStorageCost: "0",
cached: false,
toolResults: null,
});

return c.json(
{
error: {
message: `Upstream provider timeout: ${errorMessage}`,
type: "upstream_timeout",
param: null,
code: "timeout",
},
},
504,
);
}
throw bodyError;
}
// json variable is already set from inside the retry loop
if (process.env.NODE_ENV !== "production") {
logger.debug("API response", { response: json });
}
Expand Down
27 changes: 22 additions & 5 deletions apps/gateway/src/chat/tools/get-finish-reason-from-error.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,34 @@ describe("getFinishReasonFromError", () => {
).toBe("client_error");
});

it("returns upstream_error for 401/403 auth errors", () => {
expect(getFinishReasonFromError(401)).toBe("upstream_error");
expect(getFinishReasonFromError(403)).toBe("upstream_error");
});

it("returns upstream_error for provider billing errors at 400", () => {
expect(
getFinishReasonFromError(
400,
'{"type":"error","error":{"type":"invalid_request_error","message":"Your credit balance is too low to access the Anthropic API."}}',
),
).toBe("upstream_error");

expect(getFinishReasonFromError(400, "insufficient_quota")).toBe(
"upstream_error",
);

expect(
getFinishReasonFromError(400, "Please check your billing settings"),
).toBe("upstream_error");
});

it("returns gateway_error for other 400 errors", () => {
expect(getFinishReasonFromError(400, "some other error")).toBe(
"gateway_error",
);
});

it("returns gateway_error for 401/403 auth errors", () => {
expect(getFinishReasonFromError(401)).toBe("gateway_error");
expect(getFinishReasonFromError(403)).toBe("gateway_error");
});

it("returns gateway_error when no error text provided", () => {
expect(getFinishReasonFromError(400)).toBe("gateway_error");
});
Expand Down
21 changes: 20 additions & 1 deletion apps/gateway/src/chat/tools/get-finish-reason-from-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
* 5xx status codes indicate upstream provider errors
* 429 status codes indicate upstream rate limiting (treated as upstream error)
* 404 status codes indicate model/endpoint not found at provider (treated as upstream error)
* 401/403 status codes indicate authentication/authorization issues (gateway configuration errors)
* 401/403 status codes indicate authentication/authorization issues at the provider
* (e.g. expired key, insufficient credits) — treated as upstream errors so requests
* can be retried on other providers
* Other 4xx status codes indicate client/gateway errors
* Special client errors (like JSON format validation) are classified as client_error
*
Expand All @@ -29,6 +31,13 @@ export function getFinishReasonFromError(
return "upstream_error";
}

// 401/403 from upstream provider indicates auth/billing issues with the provider key
// (e.g. expired key, insufficient credits, revoked access)
// These are provider-specific and should be retried on other providers
if (statusCode === 401 || statusCode === 403) {
return "upstream_error";
}

// Azure OpenAI content filter (ResponsibleAIPolicyViolation)
if (errorText?.includes("ResponsibleAIPolicyViolation")) {
return "content_filter";
Expand All @@ -52,6 +61,16 @@ export function getFinishReasonFromError(
) {
return "client_error";
}

// Provider billing/credit errors returned as 400 (e.g. Anthropic "credit balance is too low")
// These are provider-specific issues, not client errors
if (
errorText.includes("credit balance") ||
errorText.includes("insufficient_quota") ||
errorText.includes("billing")
) {
return "upstream_error";
}
}

return "gateway_error";
Expand Down
Loading
Loading