-
Notifications
You must be signed in to change notification settings - Fork 132
fix: avoid filtered providers on moderation failure #1985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
152a8bd
e8d62de
9fe0c9b
290557d
1c140e6
54bb162
e26c302
77b91b8
d1ed69f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -364,9 +364,9 @@ function isContentFilterProvider(providerId: string): boolean { | |
|
|
||
| function getContentFilterRoutingDecision( | ||
| availableModelProviders: ProviderModelMapping[], | ||
| contentFilterMatched: boolean, | ||
| shouldAvoidContentFilterProviders: boolean, | ||
| ): ContentFilterRoutingDecision { | ||
| if (!contentFilterMatched) { | ||
| if (!shouldAvoidContentFilterProviders) { | ||
| return { | ||
| candidates: availableModelProviders, | ||
| excludedProviders: [], | ||
|
|
@@ -408,11 +408,12 @@ function getContentFilterRoutingDecision( | |
| function addContentFilterRoutingMetadata( | ||
| routingMetadata: RoutingMetadata, | ||
| contentFilterMatched: boolean, | ||
| contentFilterUnavailable: boolean, | ||
| excludedProviders: ProviderModelMapping[], | ||
| modelId: string | undefined, | ||
| metricsMap: Map<string, ProviderMetrics>, | ||
| ): RoutingMetadata { | ||
| if (!contentFilterMatched) { | ||
| if (!contentFilterMatched && !contentFilterUnavailable) { | ||
| return routingMetadata; | ||
| } | ||
|
|
||
|
|
@@ -438,15 +439,18 @@ function addContentFilterRoutingMetadata( | |
| throughput: metrics?.throughput ?? 0, | ||
| price: getProviderSelectionPrice(provider), | ||
| contentFilterProvider: true, | ||
| excludedByContentFilter: true, | ||
| ...(contentFilterMatched | ||
| ? { excludedByContentFilter: true } | ||
| : { excludedByModerationFailure: true }), | ||
| }; | ||
| }), | ||
| ...routingMetadata.providerScores, | ||
| ]; | ||
|
|
||
| return { | ||
| ...routingMetadata, | ||
| contentFilterMatched: true, | ||
| ...(contentFilterMatched ? { contentFilterMatched: true } : {}), | ||
| ...(contentFilterUnavailable ? { contentFilterUnavailable: true } : {}), | ||
| contentFilterRerouted: contentFilterExcludedProviders.length > 0, | ||
| contentFilterExcludedProviders: | ||
| contentFilterExcludedProviders.length > 0 | ||
|
|
@@ -1899,8 +1903,11 @@ chat.openapi(completions, async (c) => { | |
| const contentFilterMatched = | ||
| keywordContentFilterMatch !== null || | ||
| openAIContentFilterResult?.flagged === true; | ||
| const shouldRerouteContentFilter = | ||
| contentFilterMode === "enabled" && contentFilterMatched; | ||
| const contentFilterUnavailable = | ||
| openAIContentFilterResult?.unavailable === true; | ||
| const shouldAvoidContentFilterProviders = | ||
| contentFilterMode === "enabled" && | ||
| (contentFilterMatched || contentFilterUnavailable); | ||
| let contentFilterRoutingExcludedProviders: ProviderModelMapping[] = []; | ||
| let contentFilterRoutingApplied = false; | ||
|
|
||
|
|
@@ -2311,7 +2318,7 @@ chat.openapi(completions, async (c) => { | |
|
|
||
| const contentFilterRoutingDecision = getContentFilterRoutingDecision( | ||
| availableModelProviders, | ||
| shouldRerouteContentFilter, | ||
| shouldAvoidContentFilterProviders, | ||
| ); | ||
| const contentFilterPreferredProviders = | ||
| contentFilterRoutingDecision.candidates; | ||
|
|
@@ -2382,6 +2389,7 @@ chat.openapi(completions, async (c) => { | |
| ...(noFallback ? { noFallback: true } : {}), | ||
| }, | ||
| contentFilterMatched, | ||
| contentFilterUnavailable, | ||
| contentFilterRoutingExcludedProviders, | ||
| modelWithPricing.id, | ||
| metricsMap, | ||
|
|
@@ -2569,6 +2577,7 @@ chat.openapi(completions, async (c) => { | |
| ...(noFallback ? { noFallback: true } : {}), | ||
| }, | ||
| contentFilterMatched, | ||
| contentFilterUnavailable, | ||
| contentFilterRoutingExcludedProviders, | ||
| baseModelId, | ||
| metricsMap, | ||
|
|
@@ -2868,6 +2877,158 @@ chat.openapi(completions, async (c) => { | |
| ); | ||
| } | ||
|
|
||
| if (!usedToken) { | ||
| throw new HTTPException(500, { | ||
| message: `No token`, | ||
| }); | ||
| } | ||
|
|
||
| usedApiKeyHash = getApiKeyFingerprint(usedToken); | ||
| routingMetadata = withUsedApiKeyHash(routingMetadata, usedApiKeyHash); | ||
|
|
||
| const contentFilterBlocked = | ||
| contentFilterMode === "enabled" && | ||
| contentFilterMatched && | ||
| !contentFilterRoutingApplied; | ||
| const contentFilterSensitiveProviderBlocked = | ||
| contentFilterMode === "enabled" && | ||
| contentFilterUnavailable && | ||
| isContentFilterProvider(usedProvider); | ||
|
Comment on lines
+3252
to
+3255
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This new Useful? React with 👍 / 👎.
Comment on lines
+3252
to
+3255
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Useful? React with 👍 / 👎.
Comment on lines
+3252
to
+3255
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This outage gate blocks as soon as the selected provider is content-filter-sensitive, but for direct-provider requests we only evaluate alternative providers in the rate-limit/low-uptime branches. In the common case where the requested sensitive provider is healthy, Useful? React with 👍 / 👎. |
||
|
|
||
| // Preserve monitor tagging, and also tag successful reroutes triggered by a | ||
| // gateway content-filter match so the decision remains visible in logs. | ||
| const shouldTagContentFilter = | ||
| (contentFilterMode === "monitor" && contentFilterMatched) || | ||
| contentFilterRoutingApplied; | ||
| const gatewayContentFilterResponse = openAIContentFilterResult?.responses | ||
| .length | ||
| ? openAIContentFilterResult.responses | ||
| : null; | ||
| const insertLog = ( | ||
| logData: Parameters<typeof _insertLog>[0], | ||
| options?: Parameters<typeof _insertLog>[1], | ||
| ) => | ||
| _insertLog( | ||
| { | ||
| ...logData, | ||
| internalContentFilter: shouldTagContentFilter | ||
| ? true | ||
| : logData.internalContentFilter, | ||
| gatewayContentFilterResponse: | ||
| logData.gatewayContentFilterResponse ?? gatewayContentFilterResponse, | ||
| }, | ||
| options, | ||
| ); | ||
|
|
||
| if (contentFilterSensitiveProviderBlocked) { | ||
| const moderationOutageMessage = | ||
| "OpenAI moderation is unavailable and no eligible provider without provider-side content filtering is available."; | ||
| const baseLogEntry = createLogEntry( | ||
|
Comment on lines
+3282
to
+3285
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This new early-return path handles only Useful? React with 👍 / 👎. |
||
| 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, | ||
| null, | ||
| null, | ||
| undefined, | ||
| undefined, | ||
| ); | ||
|
|
||
| logger.warn( | ||
| "Blocking request because OpenAI moderation is unavailable and selected provider is content-filter-sensitive", | ||
| { | ||
| requestId, | ||
| organizationId: project.organizationId, | ||
| projectId: project.id, | ||
| apiKeyId: apiKey.id, | ||
| usedProvider, | ||
| usedModel, | ||
| requestedModel: initialRequestedModel, | ||
| routingMetadata, | ||
| }, | ||
| ); | ||
|
|
||
| try { | ||
| await insertLogEntry({ | ||
| ...baseLogEntry, | ||
| duration: 0, | ||
| 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: !!stream, | ||
| canceled: false, | ||
| errorDetails: { | ||
| statusCode: 503, | ||
| statusText: "Service Unavailable", | ||
| responseText: moderationOutageMessage, | ||
| }, | ||
| cachedInputCost: null, | ||
| requestCost: null, | ||
| webSearchCost: null, | ||
| imageInputTokens: null, | ||
| imageOutputTokens: null, | ||
| imageInputCost: null, | ||
| imageOutputCost: null, | ||
| estimatedCost: false, | ||
| discount: null, | ||
| dataStorageCost: "0", | ||
| cached: false, | ||
| toolResults: null, | ||
| unifiedFinishReason: "upstream_error", | ||
| }); | ||
| } catch (error) { | ||
| logger.error( | ||
| "Failed to persist moderation outage block log", | ||
| { | ||
| requestId, | ||
| organizationId: project.organizationId, | ||
| projectId: project.id, | ||
| apiKeyId: apiKey.id, | ||
| usedProvider, | ||
| }, | ||
| error as Error, | ||
| ); | ||
| } | ||
|
|
||
| throw new HTTPException(503, { | ||
| message: moderationOutageMessage, | ||
| }); | ||
| } | ||
|
|
||
| // Consume a rate-limit slot for the chosen provider (routing already filtered rate-limited ones) | ||
| { | ||
| const providerRateLimitResult = await checkProviderRateLimit( | ||
|
|
@@ -2971,52 +3132,13 @@ chat.openapi(completions, async (c) => { | |
| } | ||
| } | ||
|
|
||
| if (!usedToken) { | ||
| throw new HTTPException(500, { | ||
| message: `No token`, | ||
| }); | ||
| } | ||
|
|
||
| usedApiKeyHash = getApiKeyFingerprint(usedToken); | ||
| routingMetadata = withUsedApiKeyHash(routingMetadata, usedApiKeyHash); | ||
|
|
||
| const contentFilterBlocked = | ||
| contentFilterMode === "enabled" && | ||
| contentFilterMatched && | ||
| !contentFilterRoutingApplied; | ||
|
|
||
| // Preserve monitor tagging, and also tag successful reroutes triggered by a | ||
| // gateway content-filter match so the decision remains visible in logs. | ||
| const shouldTagContentFilter = | ||
| (contentFilterMode === "monitor" && contentFilterMatched) || | ||
| contentFilterRoutingApplied; | ||
| const gatewayContentFilterResponse = openAIContentFilterResult?.responses | ||
| .length | ||
| ? openAIContentFilterResult.responses | ||
| : null; | ||
| const insertLog = ( | ||
| logData: Parameters<typeof _insertLog>[0], | ||
| options?: Parameters<typeof _insertLog>[1], | ||
| ) => | ||
| _insertLog( | ||
| { | ||
| ...logData, | ||
| internalContentFilter: shouldTagContentFilter | ||
| ? true | ||
| : logData.internalContentFilter, | ||
| gatewayContentFilterResponse: | ||
| logData.gatewayContentFilterResponse ?? gatewayContentFilterResponse, | ||
| }, | ||
| options, | ||
| ); | ||
|
|
||
| if (contentFilterBlocked) { | ||
| const contentFilterResponseId = `chatcmpl-${Date.now()}`; | ||
| const contentFilterCreated = Math.floor(Date.now() / 1000); | ||
|
|
||
| // Log the filtered request | ||
| try { | ||
| await insertLog({ | ||
| await insertLogEntry({ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Route this branch through the local Useful? React with 👍 / 👎. |
||
| ...createLogEntry( | ||
| requestId, | ||
| project, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,6 +53,7 @@ interface OpenAIModerationResult { | |
|
|
||
| export interface OpenAIContentFilterCheckResult { | ||
| flagged: boolean; | ||
| unavailable: boolean; | ||
| model: string; | ||
| upstreamRequestId: string | null; | ||
| results: OpenAIModerationResult[]; | ||
|
|
@@ -339,9 +340,11 @@ function buildModerationErrorDetails(error: unknown): Record<string, string> { | |
|
|
||
| function createFailedOpenAIContentFilterResult( | ||
| upstreamRequestId: string | null = null, | ||
| unavailable = true, | ||
| ): OpenAIContentFilterCheckResult { | ||
| return { | ||
| flagged: false, | ||
| unavailable, | ||
| model: OPENAI_MODERATION_MODEL, | ||
| upstreamRequestId, | ||
| results: [], | ||
|
|
@@ -446,6 +449,7 @@ async function runOpenAIContentFilterRequest( | |
| flagged: (moderationResponse.results ?? []).some((result) => | ||
| isOpenAIModerationResultFlagged(result), | ||
| ), | ||
| unavailable: false, | ||
| model: moderationResponse.model ?? OPENAI_MODERATION_MODEL, | ||
| upstreamRequestId, | ||
| results: moderationResponse.results ?? [], | ||
|
|
@@ -476,7 +480,7 @@ export async function checkOpenAIContentFilter( | |
| results: [], | ||
| }); | ||
|
|
||
| return createFailedOpenAIContentFilterResult(); | ||
| return createFailedOpenAIContentFilterResult(null, false); | ||
| } | ||
|
Comment on lines
470
to
484
|
||
|
|
||
| const signal = requestSignal | ||
|
|
@@ -530,6 +534,7 @@ export async function checkOpenAIContentFilter( | |
|
|
||
| return { | ||
| flagged, | ||
| unavailable: moderationResults.some((result) => !result.success), | ||
| model, | ||
| upstreamRequestId, | ||
| results, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,6 +85,7 @@ export function selectNextProvider( | |
| score: number; | ||
| region?: string; | ||
| excludedByContentFilter?: boolean; | ||
| excludedByModerationFailure?: boolean; | ||
| }>, | ||
| failedProviders: Set<string>, | ||
| modelProviders: Array<{ | ||
|
|
@@ -95,7 +96,7 @@ export function selectNextProvider( | |
| ): { providerId: string; modelName: string; region?: string } | null { | ||
| const sorted = [...providerScores].sort((a, b) => a.score - b.score); | ||
| for (const score of sorted) { | ||
| if (score.excludedByContentFilter) { | ||
| if (score.excludedByContentFilter || score.excludedByModerationFailure) { | ||
| continue; | ||
| } | ||
|
Comment on lines
97
to
101
|
||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.