mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
Merge pull request #372 from kfiramar/fix/clear-provider-error-state
Thanks @kfiramar! 🎉 Critical fix — stale error metadata on recovered provider accounts was preventing valid accounts from being selected properly after recovery.
Improvement added on top: documented the two valid success-check patterns (`result.success` for open-sse handlers vs `response?.ok` for fetch-based handlers) to address the kilo-code-bot review warning — both patterns are correct by design, now explicitly documented.
5 commits total, 2 test files (+168 lines of coverage). Merged!
This commit is contained in:
@@ -1,6 +1,11 @@
|
||||
import { CORS_ORIGIN } from "@/shared/utils/cors";
|
||||
import { handleAudioSpeech } from "@omniroute/open-sse/handlers/audioSpeech.ts";
|
||||
import { getProviderCredentials, extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import {
|
||||
getProviderCredentials,
|
||||
clearRecoveredProviderState,
|
||||
extractApiKey,
|
||||
isValidApiKey,
|
||||
} from "@/sse/services/auth";
|
||||
import { parseSpeechModel, getSpeechProvider } from "@omniroute/open-sse/config/audioRegistry.ts";
|
||||
import { errorResponse } from "@omniroute/open-sse/utils/error.ts";
|
||||
import { HTTP_STATUS } from "@omniroute/open-sse/config/constants.ts";
|
||||
@@ -70,5 +75,9 @@ export async function POST(request) {
|
||||
}
|
||||
}
|
||||
|
||||
return handleAudioSpeech({ body, credentials });
|
||||
const response = await handleAudioSpeech({ body, credentials });
|
||||
if (response?.ok) {
|
||||
await clearRecoveredProviderState(credentials);
|
||||
}
|
||||
return response;
|
||||
}
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { CORS_ORIGIN } from "@/shared/utils/cors";
|
||||
import { handleAudioTranscription } from "@omniroute/open-sse/handlers/audioTranscription.ts";
|
||||
import { getProviderCredentials, extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import {
|
||||
getProviderCredentials,
|
||||
clearRecoveredProviderState,
|
||||
extractApiKey,
|
||||
isValidApiKey,
|
||||
} from "@/sse/services/auth";
|
||||
import { parseTranscriptionModel, getTranscriptionProvider } from "@omniroute/open-sse/config/audioRegistry.ts";
|
||||
import { errorResponse } from "@omniroute/open-sse/utils/error.ts";
|
||||
import { HTTP_STATUS } from "@omniroute/open-sse/config/constants.ts";
|
||||
@@ -68,5 +73,9 @@ export async function POST(request) {
|
||||
}
|
||||
}
|
||||
|
||||
return handleAudioTranscription({ formData, credentials });
|
||||
const response = await handleAudioTranscription({ formData, credentials });
|
||||
if (response?.ok) {
|
||||
await clearRecoveredProviderState(credentials);
|
||||
}
|
||||
return response;
|
||||
}
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { CORS_ORIGIN } from "@/shared/utils/cors";
|
||||
import { handleEmbedding } from "@omniroute/open-sse/handlers/embeddings.ts";
|
||||
import { getProviderCredentials, extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import {
|
||||
getProviderCredentials,
|
||||
clearRecoveredProviderState,
|
||||
extractApiKey,
|
||||
isValidApiKey,
|
||||
} from "@/sse/services/auth";
|
||||
import {
|
||||
parseEmbeddingModel,
|
||||
getAllEmbeddingModels,
|
||||
@@ -126,6 +131,7 @@ export async function POST(request) {
|
||||
const result = await handleEmbedding({ body, credentials, log });
|
||||
|
||||
if (result.success) {
|
||||
await clearRecoveredProviderState(credentials);
|
||||
return new Response(JSON.stringify(result.data), {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { CORS_ORIGIN } from "@/shared/utils/cors";
|
||||
import { handleImageGeneration } from "@omniroute/open-sse/handlers/imageGeneration.ts";
|
||||
import { getProviderCredentials, extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import {
|
||||
getProviderCredentials,
|
||||
clearRecoveredProviderState,
|
||||
extractApiKey,
|
||||
isValidApiKey,
|
||||
} from "@/sse/services/auth";
|
||||
import {
|
||||
parseImageModel,
|
||||
getAllImageModels,
|
||||
@@ -170,6 +175,7 @@ export async function POST(request) {
|
||||
});
|
||||
|
||||
if (result.success) {
|
||||
await clearRecoveredProviderState(credentials);
|
||||
return new Response(JSON.stringify((result as any).data), {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { CORS_ORIGIN } from "@/shared/utils/cors";
|
||||
import { handleModeration } from "@omniroute/open-sse/handlers/moderations.ts";
|
||||
import { getProviderCredentials, extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import {
|
||||
getProviderCredentials,
|
||||
clearRecoveredProviderState,
|
||||
extractApiKey,
|
||||
isValidApiKey,
|
||||
} from "@/sse/services/auth";
|
||||
import { parseModerationModel } from "@omniroute/open-sse/config/moderationRegistry.ts";
|
||||
import { errorResponse } from "@omniroute/open-sse/utils/error.ts";
|
||||
import { HTTP_STATUS } from "@omniroute/open-sse/config/constants.ts";
|
||||
@@ -64,5 +69,9 @@ export async function POST(request) {
|
||||
);
|
||||
}
|
||||
|
||||
return handleModeration({ body: { ...body, model }, credentials });
|
||||
const response = await handleModeration({ body: { ...body, model }, credentials });
|
||||
if (response?.ok) {
|
||||
await clearRecoveredProviderState(credentials);
|
||||
}
|
||||
return response;
|
||||
}
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { CORS_ORIGIN } from "@/shared/utils/cors";
|
||||
import { handleMusicGeneration } from "@omniroute/open-sse/handlers/musicGeneration.ts";
|
||||
import { getProviderCredentials, extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import {
|
||||
getProviderCredentials,
|
||||
clearRecoveredProviderState,
|
||||
extractApiKey,
|
||||
isValidApiKey,
|
||||
} from "@/sse/services/auth";
|
||||
import {
|
||||
parseMusicModel,
|
||||
getAllMusicModels,
|
||||
@@ -110,6 +115,7 @@ export async function POST(request) {
|
||||
const result = await handleMusicGeneration({ body, credentials, log });
|
||||
|
||||
if (result.success) {
|
||||
await clearRecoveredProviderState(credentials);
|
||||
return new Response(JSON.stringify((result as any).data), {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
|
||||
@@ -2,7 +2,12 @@ import { CORS_ORIGIN } from "@/shared/utils/cors";
|
||||
import { errorResponse } from "@omniroute/open-sse/utils/error.ts";
|
||||
import { HTTP_STATUS } from "@omniroute/open-sse/config/constants.ts";
|
||||
import { getRegistryEntry } from "@omniroute/open-sse/config/providerRegistry.ts";
|
||||
import { getProviderCredentials, extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import {
|
||||
getProviderCredentials,
|
||||
clearRecoveredProviderState,
|
||||
extractApiKey,
|
||||
isValidApiKey,
|
||||
} from "@/sse/services/auth";
|
||||
import { handleEmbedding } from "@omniroute/open-sse/handlers/embeddings.ts";
|
||||
import * as log from "@/sse/utils/logger";
|
||||
import { enforceApiKeyPolicy } from "@/shared/utils/apiKeyPolicy";
|
||||
@@ -84,6 +89,7 @@ export async function POST(request, { params }) {
|
||||
const result = await handleEmbedding({ body, credentials, log });
|
||||
|
||||
if (result.success) {
|
||||
await clearRecoveredProviderState(credentials);
|
||||
return new Response(JSON.stringify(result.data), {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
|
||||
@@ -2,7 +2,12 @@ import { CORS_ORIGIN } from "@/shared/utils/cors";
|
||||
import { handleImageGeneration } from "@omniroute/open-sse/handlers/imageGeneration.ts";
|
||||
import { errorResponse } from "@omniroute/open-sse/utils/error.ts";
|
||||
import { HTTP_STATUS } from "@omniroute/open-sse/config/constants.ts";
|
||||
import { getProviderCredentials, extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import {
|
||||
getProviderCredentials,
|
||||
clearRecoveredProviderState,
|
||||
extractApiKey,
|
||||
isValidApiKey,
|
||||
} from "@/sse/services/auth";
|
||||
import { getImageProvider } from "@omniroute/open-sse/config/imageRegistry.ts";
|
||||
import * as log from "@/sse/utils/logger";
|
||||
import { toJsonErrorPayload } from "@/shared/utils/upstreamError";
|
||||
@@ -84,6 +89,7 @@ export async function POST(request, { params }) {
|
||||
const result = await handleImageGeneration({ body, credentials, log });
|
||||
|
||||
if (result.success) {
|
||||
await clearRecoveredProviderState(credentials);
|
||||
return new Response(JSON.stringify((result as any).data), {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { CORS_ORIGIN } from "@/shared/utils/cors";
|
||||
import { handleRerank } from "@omniroute/open-sse/handlers/rerank.ts";
|
||||
import { getProviderCredentials, extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import {
|
||||
getProviderCredentials,
|
||||
clearRecoveredProviderState,
|
||||
extractApiKey,
|
||||
isValidApiKey,
|
||||
} from "@/sse/services/auth";
|
||||
import { parseRerankModel } from "@omniroute/open-sse/config/rerankRegistry.ts";
|
||||
import { errorResponse } from "@omniroute/open-sse/utils/error.ts";
|
||||
import { HTTP_STATUS } from "@omniroute/open-sse/config/constants.ts";
|
||||
@@ -66,7 +71,7 @@ export async function POST(request) {
|
||||
return errorResponse(HTTP_STATUS.BAD_REQUEST, `No credentials for provider: ${provider}`);
|
||||
}
|
||||
|
||||
return handleRerank({
|
||||
const response = await handleRerank({
|
||||
model: body.model,
|
||||
query: body.query,
|
||||
documents: body.documents,
|
||||
@@ -74,4 +79,8 @@ export async function POST(request) {
|
||||
return_documents: body.return_documents,
|
||||
credentials,
|
||||
});
|
||||
if (response?.ok) {
|
||||
await clearRecoveredProviderState(credentials);
|
||||
}
|
||||
return response;
|
||||
}
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { CORS_ORIGIN } from "@/shared/utils/cors";
|
||||
import { handleVideoGeneration } from "@omniroute/open-sse/handlers/videoGeneration.ts";
|
||||
import { getProviderCredentials, extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import {
|
||||
getProviderCredentials,
|
||||
clearRecoveredProviderState,
|
||||
extractApiKey,
|
||||
isValidApiKey,
|
||||
} from "@/sse/services/auth";
|
||||
import {
|
||||
parseVideoModel,
|
||||
getAllVideoModels,
|
||||
@@ -110,6 +115,7 @@ export async function POST(request) {
|
||||
const result = await handleVideoGeneration({ body, credentials, log });
|
||||
|
||||
if (result.success) {
|
||||
await clearRecoveredProviderState(credentials);
|
||||
return new Response(JSON.stringify((result as any).data), {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
|
||||
@@ -35,10 +35,22 @@ interface ProviderConnectionView {
|
||||
consecutiveUseCount: number;
|
||||
priority: number;
|
||||
lastError: string | null;
|
||||
lastErrorType: string | null;
|
||||
lastErrorSource: string | null;
|
||||
errorCode: string | number | null;
|
||||
backoffLevel: number;
|
||||
}
|
||||
|
||||
interface RecoverableConnectionState {
|
||||
connectionId: string;
|
||||
testStatus?: string | null;
|
||||
lastError?: string | null;
|
||||
rateLimitedUntil?: string | null;
|
||||
errorCode?: string | number | null;
|
||||
lastErrorType?: string | null;
|
||||
lastErrorSource?: string | null;
|
||||
}
|
||||
|
||||
const CODEX_QUOTA_THRESHOLD_PERCENT = 90;
|
||||
|
||||
function asRecord(value: unknown): JsonRecord {
|
||||
@@ -76,6 +88,8 @@ function toProviderConnection(value: unknown): ProviderConnectionView {
|
||||
consecutiveUseCount: toNumber(row.consecutiveUseCount, 0),
|
||||
priority: toNumber(row.priority, 999),
|
||||
lastError: toStringOrNull(row.lastError),
|
||||
lastErrorType: toStringOrNull(row.lastErrorType),
|
||||
lastErrorSource: toStringOrNull(row.lastErrorSource),
|
||||
errorCode:
|
||||
typeof row.errorCode === "string" || typeof row.errorCode === "number" ? row.errorCode : null,
|
||||
backoffLevel: toNumber(row.backoffLevel, 0),
|
||||
@@ -469,6 +483,9 @@ export async function getProviderCredentials(
|
||||
// Include current status for optimization check
|
||||
testStatus: connection.testStatus,
|
||||
lastError: connection.lastError,
|
||||
lastErrorType: connection.lastErrorType,
|
||||
lastErrorSource: connection.lastErrorSource,
|
||||
errorCode: connection.errorCode,
|
||||
rateLimitedUntil: connection.rateLimitedUntil,
|
||||
};
|
||||
} finally {
|
||||
@@ -569,12 +586,18 @@ export async function markAccountUnavailable(
|
||||
* Clear account error status (only if currently has error)
|
||||
* Optimized to avoid unnecessary DB updates
|
||||
*/
|
||||
export async function clearAccountError(connectionId: string, currentConnection: any) {
|
||||
export async function clearAccountError(
|
||||
connectionId: string,
|
||||
currentConnection: Partial<RecoverableConnectionState>
|
||||
) {
|
||||
// Only update if currently has error status
|
||||
const hasError =
|
||||
currentConnection.testStatus === "unavailable" ||
|
||||
(currentConnection.testStatus && currentConnection.testStatus !== "active") ||
|
||||
currentConnection.lastError ||
|
||||
currentConnection.rateLimitedUntil;
|
||||
currentConnection.rateLimitedUntil ||
|
||||
currentConnection.errorCode ||
|
||||
currentConnection.lastErrorType ||
|
||||
currentConnection.lastErrorSource;
|
||||
|
||||
if (!hasError) return; // Skip if already clean
|
||||
|
||||
@@ -582,12 +605,22 @@ export async function clearAccountError(connectionId: string, currentConnection:
|
||||
testStatus: "active",
|
||||
lastError: null,
|
||||
lastErrorAt: null,
|
||||
lastErrorType: null,
|
||||
lastErrorSource: null,
|
||||
errorCode: null,
|
||||
rateLimitedUntil: null,
|
||||
backoffLevel: 0,
|
||||
});
|
||||
log.info("AUTH", `Account ${connectionId.slice(0, 8)} error cleared`);
|
||||
}
|
||||
|
||||
export async function clearRecoveredProviderState(
|
||||
credentials: Partial<RecoverableConnectionState> | null
|
||||
) {
|
||||
if (!credentials?.connectionId) return;
|
||||
await clearAccountError(credentials.connectionId, credentials);
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract API key from request headers
|
||||
*/
|
||||
|
||||
59
tests/unit/auth-clear-account-error.test.mjs
Normal file
59
tests/unit/auth-clear-account-error.test.mjs
Normal file
@@ -0,0 +1,59 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-auth-clear-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
|
||||
const core = await import("../../src/lib/db/core.ts");
|
||||
const providersDb = await import("../../src/lib/db/providers.ts");
|
||||
const auth = await import("../../src/sse/services/auth.ts");
|
||||
|
||||
async function resetStorage() {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
||||
}
|
||||
|
||||
test.after(() => {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("clearAccountError clears stale provider error metadata after recovery", async () => {
|
||||
await resetStorage();
|
||||
core.getDbInstance().exec('ALTER TABLE provider_connections ADD COLUMN "group" TEXT');
|
||||
|
||||
const created = await providersDb.createProviderConnection({
|
||||
provider: "codex",
|
||||
authType: "oauth",
|
||||
email: "recover@example.com",
|
||||
accessToken: "access",
|
||||
refreshToken: "refresh",
|
||||
testStatus: "active",
|
||||
lastError: null,
|
||||
lastErrorType: "token_refresh_failed",
|
||||
lastErrorSource: "oauth",
|
||||
errorCode: "refresh_failed",
|
||||
rateLimitedUntil: null,
|
||||
backoffLevel: 2,
|
||||
});
|
||||
|
||||
const credentials = await auth.getProviderCredentials("codex");
|
||||
assert.equal(credentials.connectionId, created.id);
|
||||
assert.equal(credentials.errorCode, "refresh_failed");
|
||||
assert.equal(credentials.lastErrorType, "token_refresh_failed");
|
||||
assert.equal(credentials.lastErrorSource, "oauth");
|
||||
await auth.clearAccountError(created.id, credentials);
|
||||
|
||||
const updated = await providersDb.getProviderConnectionById(created.id);
|
||||
assert.equal(updated.testStatus, "active");
|
||||
assert.equal(updated.lastError, undefined);
|
||||
assert.equal(updated.lastErrorType, undefined);
|
||||
assert.equal(updated.lastErrorSource, undefined);
|
||||
assert.equal(updated.errorCode, undefined);
|
||||
assert.equal(updated.rateLimitedUntil, undefined);
|
||||
assert.equal(updated.backoffLevel, 0);
|
||||
});
|
||||
109
tests/unit/auth-clear-provider-routes.test.mjs
Normal file
109
tests/unit/auth-clear-provider-routes.test.mjs
Normal file
@@ -0,0 +1,109 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-auth-routes-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
|
||||
const core = await import("../../src/lib/db/core.ts");
|
||||
const providersDb = await import("../../src/lib/db/providers.ts");
|
||||
const moderationRoute = await import("../../src/app/api/v1/moderations/route.ts");
|
||||
const embeddingsRoute = await import("../../src/app/api/v1/embeddings/route.ts");
|
||||
|
||||
async function resetStorage() {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
||||
core.getDbInstance().exec('ALTER TABLE provider_connections ADD COLUMN "group" TEXT');
|
||||
}
|
||||
|
||||
async function seedOpenAIConnection(email) {
|
||||
return await providersDb.createProviderConnection({
|
||||
provider: "openai",
|
||||
authType: "apikey",
|
||||
email,
|
||||
name: email,
|
||||
apiKey: "sk-test",
|
||||
testStatus: "active",
|
||||
lastError: null,
|
||||
lastErrorType: "token_refresh_failed",
|
||||
lastErrorSource: "oauth",
|
||||
errorCode: "refresh_failed",
|
||||
rateLimitedUntil: null,
|
||||
backoffLevel: 2,
|
||||
});
|
||||
}
|
||||
|
||||
async function readConnection(id) {
|
||||
return await providersDb.getProviderConnectionById(id);
|
||||
}
|
||||
|
||||
test.after(() => {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("moderations route clears stale provider error metadata on success", async () => {
|
||||
await resetStorage();
|
||||
const created = await seedOpenAIConnection("moderation@example.com");
|
||||
|
||||
const originalFetch = globalThis.fetch;
|
||||
globalThis.fetch = async () =>
|
||||
Response.json({
|
||||
id: "modr-1",
|
||||
model: "omni-moderation-latest",
|
||||
results: [{ flagged: false }],
|
||||
});
|
||||
|
||||
try {
|
||||
const request = new Request("http://localhost/v1/moderations", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ input: "hello" }),
|
||||
});
|
||||
|
||||
const response = await moderationRoute.POST(request);
|
||||
assert.equal(response.status, 200);
|
||||
|
||||
const updated = await readConnection(created.id);
|
||||
assert.equal(updated.testStatus, "active");
|
||||
assert.equal(updated.errorCode, undefined);
|
||||
assert.equal(updated.lastErrorType, undefined);
|
||||
assert.equal(updated.lastErrorSource, undefined);
|
||||
} finally {
|
||||
globalThis.fetch = originalFetch;
|
||||
}
|
||||
});
|
||||
|
||||
test("embeddings route clears stale provider error metadata on success", async () => {
|
||||
await resetStorage();
|
||||
const created = await seedOpenAIConnection("embeddings@example.com");
|
||||
|
||||
const originalFetch = globalThis.fetch;
|
||||
globalThis.fetch = async () =>
|
||||
Response.json({
|
||||
data: [{ object: "embedding", index: 0, embedding: [0.1, 0.2] }],
|
||||
usage: { prompt_tokens: 3, total_tokens: 3 },
|
||||
});
|
||||
|
||||
try {
|
||||
const request = new Request("http://localhost/v1/embeddings", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ model: "openai/text-embedding-3-small", input: "hello" }),
|
||||
});
|
||||
|
||||
const response = await embeddingsRoute.POST(request);
|
||||
assert.equal(response.status, 200);
|
||||
|
||||
const updated = await readConnection(created.id);
|
||||
assert.equal(updated.testStatus, "active");
|
||||
assert.equal(updated.errorCode, undefined);
|
||||
assert.equal(updated.lastErrorType, undefined);
|
||||
assert.equal(updated.lastErrorSource, undefined);
|
||||
} finally {
|
||||
globalThis.fetch = originalFetch;
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user