Skip to content

Commit b4a98d0

Browse files
devarshishimpidevarshishimpi
committed
fix: enhance model handling and error logging in API routes and configuration
1 parent db8e2b6 commit b4a98d0

6 files changed

Lines changed: 101 additions & 33 deletions

File tree

‎src/server/core/config.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { defaultRepoConfig, normalizeRepoModelConfig, type RepoConfig } from '@shared/schema';
1+
import { defaultRepoConfig, normalizeRepoModelConfig, repoConfigSchema, type RepoConfig } from '@shared/schema';
22
import { REPO_CONFIG_CACHE_VERSION } from '@shared/config';
33
import type { AppBindings } from '@server/env';
44
import { getRepoConfigRecord, syncRepoConfig } from '@server/db/repo-configs';
@@ -38,7 +38,12 @@ function hasRepoModelOverride(existing: Awaited<ReturnType<typeof getRepoConfigR
3838

3939
export async function getGlobalConfig(env: Pick<AppBindings, 'APP_KV'>): Promise<RepoConfig['model']> {
4040
const cached = await env.APP_KV.get(GLOBAL_CONFIG_KEY, 'json');
41-
if (cached) return normalizeRepoModelConfig(cached as RepoConfig['model']);
41+
if (cached) {
42+
const parsed = repoConfigSchema.shape.model.safeParse(cached);
43+
if (parsed.success) {
44+
return normalizeRepoModelConfig(parsed.data);
45+
}
46+
}
4247

4348
return EMPTY_GLOBAL_CONFIG;
4449
}

‎src/server/core/review.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ export async function runReviewJob(env: AppBindings, message: ReviewJobMessage):
246246

247247
logger.error(`Review job failed: ${job.owner}/${job.repo} PR #${job.prNumber}`, error);
248248
await failJobAndCheckRun(env, job, github, messageText);
249+
await releaseJobLease(env, job.id, leaseOwner);
249250
return { action: 'ack' };
250251
}
251252
}

‎src/server/index.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@ export default {
1515

1616
async queue(batch: MessageBatch<unknown>, env: AppBindings, _ctx: ExecutionContext) {
1717
return runWithDb(env, async () => {
18-
await runBestEffortJobMaintenance(env);
18+
try {
19+
await runBestEffortJobMaintenance(env);
20+
} catch (error) {
21+
logger.error('Pre-batch maintenance task failed', error instanceof Error ? error : new Error(String(error)));
22+
}
1923

2024
for (const message of batch.messages) {
2125
const parseResult = reviewJobMessageSchema.safeParse(message.body);
@@ -42,7 +46,11 @@ export default {
4246
}
4347
}
4448

45-
await runBestEffortJobMaintenance(env);
49+
try {
50+
await runBestEffortJobMaintenance(env);
51+
} catch (error) {
52+
logger.error('Post-batch maintenance task failed', error instanceof Error ? error : new Error(String(error)));
53+
}
4654
});
4755
},
4856
} satisfies ExportedHandler<AppBindings>;

‎src/server/models/catalog.ts‎

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -65,29 +65,56 @@ const CLOUDFLARE_TEXT_GENERATION_MODELS = [
6565
'@cf/meta/llama-3.1-8b-instruct-fast',
6666
];
6767

68+
interface OpenAIModelsResponse {
69+
data?: Array<{ id?: unknown }>;
70+
}
71+
72+
interface AnthropicModelsResponse {
73+
data?: Array<{ id?: unknown }>;
74+
}
75+
76+
interface GeminiModelsResponse {
77+
models?: Array<{
78+
name?: unknown;
79+
supportedGenerationMethods?: unknown;
80+
}>;
81+
}
82+
83+
interface CloudflareModelItem {
84+
id?: unknown;
85+
name?: unknown;
86+
model?: unknown;
87+
model_id?: unknown;
88+
}
89+
90+
interface CloudflareModelsResponse {
91+
result?: CloudflareModelItem[] | { data?: CloudflareModelItem[] };
92+
data?: CloudflareModelItem[];
93+
}
94+
6895
function cleanGeminiModelName(name: string) {
6996
return name.startsWith('models/') ? name.slice('models/'.length) : name;
7097
}
7198

72-
function extractOpenAiModels(data: any) {
99+
function extractOpenAiModels(data: OpenAIModelsResponse) {
73100
return Array.isArray(data?.data)
74-
? data.data.map((item: any) => item?.id).filter((id: unknown): id is string => typeof id === 'string' && id.length > 0)
101+
? data.data.map((item) => item?.id).filter((id: unknown): id is string => typeof id === 'string' && id.length > 0)
75102
: [];
76103
}
77104

78-
function extractAnthropicModels(data: any) {
105+
function extractAnthropicModels(data: AnthropicModelsResponse) {
79106
return Array.isArray(data?.data)
80-
? data.data.map((item: any) => item?.id).filter((id: unknown): id is string => typeof id === 'string' && id.length > 0)
107+
? data.data.map((item) => item?.id).filter((id: unknown): id is string => typeof id === 'string' && id.length > 0)
81108
: [];
82109
}
83110

84-
function extractGeminiModels(data: any) {
111+
function extractGeminiModels(data: GeminiModelsResponse) {
85112
if (!Array.isArray(data?.models)) return [];
86113
return data.models
87-
.filter((model: any) => Array.isArray(model?.supportedGenerationMethods)
114+
.filter((model) => Array.isArray(model?.supportedGenerationMethods)
88115
? model.supportedGenerationMethods.includes('generateContent')
89116
: true)
90-
.map((model: any) => typeof model?.name === 'string' ? cleanGeminiModelName(model.name) : null)
117+
.map((model) => typeof model?.name === 'string' ? cleanGeminiModelName(model.name) : null)
91118
.filter((id: unknown): id is string => typeof id === 'string' && id.length > 0);
92119
}
93120

@@ -112,7 +139,7 @@ export async function listProviderModels(input: {
112139
}),
113140
);
114141
if (!response.ok) throw new Error(`OpenAI model list failed with ${response.status}: ${await limitedErrorBody(response)}`);
115-
return extractOpenAiModels(await response.json());
142+
return extractOpenAiModels(await response.json() as OpenAIModelsResponse);
116143
}
117144

118145
if (input.apiFormat === 'anthropic') {
@@ -128,18 +155,23 @@ export async function listProviderModels(input: {
128155
}),
129156
);
130157
if (!response.ok) throw new Error(`Anthropic model list failed with ${response.status}: ${await limitedErrorBody(response)}`);
131-
return extractAnthropicModels(await response.json());
158+
return extractAnthropicModels(await response.json() as AnthropicModelsResponse);
132159
}
133160

134161
if (input.apiFormat === 'gemini') {
135162
if (!input.apiKey) throw new Error('Google API key is required to list models.');
136163
const apiKey = input.apiKey;
137-
const url = `${baseUrl}/models?key=${encodeURIComponent(apiKey)}`;
164+
const url = `${baseUrl}/models`;
138165
const response = await withTimeout('Google model list', MODEL_LIST_TIMEOUT_MS, (signal) =>
139-
fetch(url, { signal }),
166+
fetch(url, {
167+
signal,
168+
headers: {
169+
'x-goog-api-key': apiKey,
170+
},
171+
}),
140172
);
141173
if (!response.ok) throw new Error(`Google model list failed with ${response.status}: ${await limitedErrorBody(response)}`);
142-
return extractGeminiModels(await response.json());
174+
return extractGeminiModels(await response.json() as GeminiModelsResponse);
143175
}
144176

145177
return listCloudflareModels(input.cloudflareAccountId, input.cloudflareApiToken);
@@ -165,21 +197,21 @@ async function listCloudflareModels(accountId?: string, apiToken?: string) {
165197
return CLOUDFLARE_TEXT_GENERATION_MODELS;
166198
}
167199
if (!response.ok) throw new Error(`Cloudflare model list failed with ${response.status}: ${await limitedErrorBody(response)}`);
168-
const models = extractCloudflareModels(await response.json());
200+
const models = extractCloudflareModels(await response.json() as CloudflareModelsResponse);
169201
return models.length > 0 ? models : CLOUDFLARE_TEXT_GENERATION_MODELS;
170202
}
171203

172-
function extractCloudflareModels(data: any) {
204+
function extractCloudflareModels(data: CloudflareModelsResponse) {
173205
const items = Array.isArray(data?.result)
174206
? data.result
175-
: Array.isArray(data?.result?.data)
176-
? data.result.data
207+
: typeof data?.result === 'object' && data.result !== null && 'data' in data.result && Array.isArray((data.result as { data?: unknown[] }).data)
208+
? (data.result as { data?: unknown[] }).data
177209
: Array.isArray(data?.data)
178210
? data.data
179211
: [];
180212

181213
return Array.from(new Set(
182-
items
214+
(items || [])
183215
.map((item: any) => normalizeCloudflareModelId(item?.id ?? item?.name ?? item?.model ?? item?.model_id))
184216
.filter((id: unknown): id is string => typeof id === 'string' && id.startsWith('@cf/')),
185217
));

‎src/server/models/openai.ts‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,22 @@ import { ProviderRequestError, providerErrorMessage, type ModelResponse } from '
55
const OPENAI_TIMEOUT_MS = 180_000;
66
const OPENAI_MAX_OUTPUT_TOKENS = 4096;
77

8-
function extractOpenAiText(data: any) {
8+
export interface OpenAIResponse {
9+
choices?: Array<{
10+
message?: {
11+
content?: string | Array<{ text?: string }>;
12+
};
13+
}>;
14+
output_text?: string;
15+
usage?: {
16+
prompt_tokens?: number;
17+
completion_tokens?: number;
18+
input_tokens?: number;
19+
output_tokens?: number;
20+
};
21+
}
22+
23+
function extractOpenAiText(data: OpenAIResponse) {
924
const messageContent = data?.choices?.[0]?.message?.content;
1025
if (typeof messageContent === 'string') return messageContent.trim();
1126
if (Array.isArray(messageContent)) {
@@ -80,7 +95,7 @@ export async function reviewWithOpenAI(
8095
throw new ProviderRequestError(config.providerName, response.status, providerErrorMessage(errorText));
8196
}
8297

83-
const data = await response.json() as any;
98+
const data = await response.json() as OpenAIResponse;
8499
const rawText = extractOpenAiText(data);
85100
if (!rawText) {
86101
throw new Error('OpenAI provider returned an empty response.');

‎src/server/routes/api/models.ts‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -337,16 +337,23 @@ export function createModelsRouter() {
337337
return jsonError(`Provider ${config.providerName} does not have a saved API key.`, 400);
338338
}
339339
const apiKey = await decryptLlmApiKey(c.env, config.encryptedApiKey);
340-
if (config.apiFormat === 'gemini') {
341-
response = await reviewWithGoogle({ apiKey, baseUrl: config.baseUrl, providerName: config.providerName }, config.modelName, input);
342-
} else if (config.apiFormat === 'openai') {
343-
response = await reviewWithOpenAI({
344-
apiKey,
345-
baseUrl: config.baseUrl || 'https://api.openai.com/v1',
346-
providerName: config.providerName,
347-
}, config.modelName, input);
348-
} else {
349-
response = await reviewWithAnthropic({ apiKey, baseUrl: config.baseUrl, providerName: config.providerName }, config.modelName, input);
340+
341+
switch (config.apiFormat) {
342+
case 'gemini':
343+
response = await reviewWithGoogle({ apiKey, baseUrl: config.baseUrl, providerName: config.providerName }, config.modelName, input);
344+
break;
345+
case 'openai':
346+
response = await reviewWithOpenAI({
347+
apiKey,
348+
baseUrl: config.baseUrl || 'https://api.openai.com/v1',
349+
providerName: config.providerName,
350+
}, config.modelName, input);
351+
break;
352+
case 'anthropic':
353+
response = await reviewWithAnthropic({ apiKey, baseUrl: config.baseUrl, providerName: config.providerName }, config.modelName, input);
354+
break;
355+
default:
356+
return jsonError(`Unsupported API format: ${config.apiFormat}`, 400);
350357
}
351358
}
352359

0 commit comments

Comments
 (0)