mirror of
https://github.com/fawney19/Aether.git
synced 2026-09-02 09:20:22 +08:00
fix(gateway): reject empty Gemini success responses
This commit is contained in:
@@ -12,7 +12,7 @@ use crate::control::GatewayControlDecision;
|
||||
use crate::usage::spawn_sync_report;
|
||||
use crate::{usage::GatewaySyncReportRequest, AppState, GatewayError};
|
||||
use axum::body::Body;
|
||||
use axum::http::Response;
|
||||
use axum::http::{Response, StatusCode};
|
||||
use base64::Engine as _;
|
||||
use tracing::warn;
|
||||
|
||||
@@ -148,6 +148,74 @@ fn build_local_core_sync_finalize_fallback_response(
|
||||
build_local_sync_response_from_bytes(trace_id, decision, payload, Vec::new())
|
||||
}
|
||||
|
||||
fn maybe_build_invalid_provider_success_finalize_response(
|
||||
trace_id: &str,
|
||||
decision: &GatewayControlDecision,
|
||||
payload: &GatewaySyncReportRequest,
|
||||
) -> Result<Option<Response<Body>>, GatewayError> {
|
||||
if !local_core_sync_finalize_has_invalid_provider_success(payload)? {
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
let client_api_format = resolve_local_sync_client_api_format(payload);
|
||||
let message = "Provider returned HTTP 200 but the Gemini response did not contain visible model output; refusing to finalize it as a successful response.";
|
||||
let body_json = build_core_error_body_for_client_format(
|
||||
&client_api_format,
|
||||
message,
|
||||
Some("invalid_provider_success_response"),
|
||||
LocalCoreSyncErrorKind::ServerError,
|
||||
)
|
||||
.unwrap_or_else(|| {
|
||||
serde_json::json!({
|
||||
"error": {
|
||||
"message": message,
|
||||
"type": "server_error",
|
||||
"code": "invalid_provider_success_response"
|
||||
}
|
||||
})
|
||||
});
|
||||
|
||||
let mut response_headers = payload.headers.clone();
|
||||
response_headers.remove("content-encoding");
|
||||
response_headers.remove("content-length");
|
||||
response_headers.insert("content-type".to_string(), "application/json".to_string());
|
||||
let body_bytes =
|
||||
serde_json::to_vec(&body_json).map_err(|err| GatewayError::Internal(err.to_string()))?;
|
||||
response_headers.insert("content-length".to_string(), body_bytes.len().to_string());
|
||||
|
||||
Ok(Some(build_client_response_from_parts(
|
||||
StatusCode::BAD_GATEWAY.as_u16(),
|
||||
&response_headers,
|
||||
Body::from(body_bytes),
|
||||
trace_id,
|
||||
Some(decision),
|
||||
)?))
|
||||
}
|
||||
|
||||
fn local_core_sync_finalize_has_invalid_provider_success(
|
||||
payload: &GatewaySyncReportRequest,
|
||||
) -> Result<bool, GatewayError> {
|
||||
if payload.status_code >= 400 || !is_core_error_finalize_kind(payload.report_kind.as_str()) {
|
||||
return Ok(false);
|
||||
}
|
||||
let provider_api_format = resolve_local_sync_provider_api_format(payload);
|
||||
if aether_ai_formats::normalize_api_format_alias(&provider_api_format)
|
||||
!= "gemini:generate_content"
|
||||
{
|
||||
return Ok(false);
|
||||
}
|
||||
let Some(body_json) = resolve_local_sync_source_body_json(payload)? else {
|
||||
return Ok(false);
|
||||
};
|
||||
if has_nested_error(&body_json) {
|
||||
return Ok(false);
|
||||
}
|
||||
Ok(
|
||||
aether_ai_formats::formats::gemini::generate_content::response::from_raw(&body_json)
|
||||
.is_none(),
|
||||
)
|
||||
}
|
||||
|
||||
pub(crate) fn build_best_effort_local_core_error_body(
|
||||
payload: &GatewaySyncReportRequest,
|
||||
body_json: &serde_json::Value,
|
||||
@@ -283,6 +351,16 @@ fn resolve_local_sync_client_api_format(payload: &GatewaySyncReportRequest) -> S
|
||||
.to_ascii_lowercase()
|
||||
}
|
||||
|
||||
fn resolve_local_sync_provider_api_format(payload: &GatewaySyncReportRequest) -> String {
|
||||
payload
|
||||
.report_context
|
||||
.as_ref()
|
||||
.and_then(|value| value.get("provider_api_format"))
|
||||
.and_then(|value| value.as_str())
|
||||
.map(|value| value.trim().to_ascii_lowercase())
|
||||
.unwrap_or_else(|| resolve_local_sync_client_api_format(payload))
|
||||
}
|
||||
|
||||
pub(crate) fn resolve_core_error_background_report_kind(report_kind: &str) -> Option<String> {
|
||||
core_error_background_report_kind(report_kind).map(ToOwned::to_owned)
|
||||
}
|
||||
@@ -522,6 +600,10 @@ pub(crate) async fn submit_local_core_error_or_sync_finalize(
|
||||
maybe_compile_sync_finalize_response(trace_id, decision, &payload)?
|
||||
{
|
||||
response
|
||||
} else if let Some(response) =
|
||||
maybe_build_invalid_provider_success_finalize_response(trace_id, decision, &payload)?
|
||||
{
|
||||
response
|
||||
} else if let Some(response) =
|
||||
maybe_build_local_core_error_response(trace_id, decision, &payload)?
|
||||
{
|
||||
@@ -566,9 +648,10 @@ mod tests {
|
||||
use axum::body::to_bytes;
|
||||
use serde_json::json;
|
||||
|
||||
use super::maybe_build_local_core_error_response;
|
||||
use super::{maybe_build_local_core_error_response, submit_local_core_error_or_sync_finalize};
|
||||
use crate::control::GatewayControlDecision;
|
||||
use crate::usage::GatewaySyncReportRequest;
|
||||
use crate::AppState;
|
||||
|
||||
fn test_decision() -> GatewayControlDecision {
|
||||
GatewayControlDecision::synthetic(
|
||||
@@ -684,4 +767,59 @@ mod tests {
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn local_core_sync_finalize_rejects_gemini_http_200_without_visible_output() {
|
||||
let mut payload = core_finalize_payload(
|
||||
"openai_chat_sync_finalize",
|
||||
"openai:chat",
|
||||
"gemini:generate_content",
|
||||
200,
|
||||
json!({
|
||||
"candidates": [{
|
||||
"content": {"role": "model"},
|
||||
"finishReason": "MAX_TOKENS"
|
||||
}],
|
||||
"usageMetadata": {
|
||||
"promptTokenCount": 8,
|
||||
"candidatesTokenCount": 1,
|
||||
"thoughtsTokenCount": 25,
|
||||
"totalTokenCount": 34
|
||||
},
|
||||
"modelVersion": "gemini-3-flash-preview",
|
||||
"responseId": "resp-empty"
|
||||
}),
|
||||
);
|
||||
payload.report_context = Some(json!({
|
||||
"client_api_format": "openai:chat",
|
||||
"provider_api_format": "gemini:generate_content",
|
||||
"needs_conversion": true,
|
||||
"has_envelope": false
|
||||
}));
|
||||
|
||||
let state = AppState::new().expect("state should build");
|
||||
let response = submit_local_core_error_or_sync_finalize(
|
||||
&state,
|
||||
"trace-invalid-gemini-200",
|
||||
&test_decision(),
|
||||
payload,
|
||||
)
|
||||
.await
|
||||
.expect("response should build");
|
||||
|
||||
assert_eq!(response.status(), http::StatusCode::BAD_GATEWAY);
|
||||
let body: serde_json::Value = serde_json::from_slice(
|
||||
&to_bytes(response.into_body(), usize::MAX)
|
||||
.await
|
||||
.expect("body should read"),
|
||||
)
|
||||
.expect("body should decode");
|
||||
let message = body["error"]["message"]
|
||||
.as_str()
|
||||
.expect("error message should exist");
|
||||
assert!(
|
||||
message.contains("visible model output"),
|
||||
"unexpected message: {message}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,7 +3,10 @@ use std::io::Error as IoError;
|
||||
use std::sync::Arc;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use aether_contracts::{ExecutionPlan, ExecutionResult, ExecutionTelemetry};
|
||||
use aether_contracts::{
|
||||
ExecutionError, ExecutionErrorKind, ExecutionPhase, ExecutionPlan, ExecutionResult,
|
||||
ExecutionTelemetry,
|
||||
};
|
||||
use aether_data_contracts::repository::candidates::RequestCandidateStatus;
|
||||
use aether_scheduler_core::{
|
||||
execution_error_details, parse_request_candidate_report_context,
|
||||
@@ -26,8 +29,8 @@ use tokio::time::MissedTickBehavior;
|
||||
use tracing::{debug, warn};
|
||||
|
||||
use crate::ai_serving::api::{
|
||||
implicit_sync_finalize_report_kind, maybe_build_sync_finalize_outcome,
|
||||
LocalCoreSyncFinalizeOutcome,
|
||||
build_core_error_body_for_client_format, implicit_sync_finalize_report_kind,
|
||||
maybe_build_sync_finalize_outcome, LocalCoreSyncErrorKind, LocalCoreSyncFinalizeOutcome,
|
||||
};
|
||||
use crate::api::response::{
|
||||
attach_control_metadata_headers, build_client_response, build_client_response_from_parts,
|
||||
@@ -183,6 +186,55 @@ fn build_sync_report_payload(
|
||||
}
|
||||
}
|
||||
|
||||
fn invalid_gemini_provider_success_message(
|
||||
plan: &ExecutionPlan,
|
||||
report_context: Option<&Value>,
|
||||
status_code: u16,
|
||||
body_json: Option<&Value>,
|
||||
) -> Option<&'static str> {
|
||||
if status_code >= 400 {
|
||||
return None;
|
||||
}
|
||||
let provider_api_format = report_context
|
||||
.and_then(|value| value.get("provider_api_format"))
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or(plan.provider_api_format.as_str());
|
||||
if aether_ai_formats::normalize_api_format_alias(provider_api_format)
|
||||
!= "gemini:generate_content"
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let body_json = body_json?;
|
||||
if body_json
|
||||
.as_object()
|
||||
.is_some_and(|object| object.get("error").is_some_and(|error| !error.is_null()))
|
||||
{
|
||||
return None;
|
||||
}
|
||||
if aether_ai_formats::formats::gemini::generate_content::response::from_raw(body_json).is_some()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
Some("Provider returned HTTP 200 but the Gemini response did not contain visible model output; refusing to finalize it as a successful response.")
|
||||
}
|
||||
|
||||
fn build_invalid_provider_success_body(
|
||||
plan: &ExecutionPlan,
|
||||
report_context: Option<&Value>,
|
||||
message: &str,
|
||||
) -> Option<Value> {
|
||||
let client_api_format = report_context
|
||||
.and_then(|value| value.get("client_api_format"))
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or(plan.client_api_format.as_str());
|
||||
build_core_error_body_for_client_format(
|
||||
client_api_format,
|
||||
message,
|
||||
Some("invalid_provider_success_response"),
|
||||
LocalCoreSyncErrorKind::ServerError,
|
||||
)
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
struct OpenAiImageSyncProgressSnapshot {
|
||||
phase: &'static str,
|
||||
@@ -1337,19 +1389,37 @@ async fn execute_execution_runtime_sync_impl(
|
||||
local_failover_response_text,
|
||||
local_failover_analysis,
|
||||
) = loop {
|
||||
let result_body_json = result
|
||||
.body
|
||||
.as_ref()
|
||||
.and_then(|body| body.json_body.as_ref());
|
||||
let (result_error_type, result_error_message) =
|
||||
execution_error_details(result.error.as_ref(), result_body_json);
|
||||
let result_latency_ms = result
|
||||
.telemetry
|
||||
.as_ref()
|
||||
.and_then(|telemetry| telemetry.elapsed_ms);
|
||||
let mut headers = std::mem::take(&mut result.headers);
|
||||
let (body_bytes, body_json, body_base64) =
|
||||
let (body_bytes, mut body_json, body_base64) =
|
||||
decode_execution_result_body(result.body.take(), &mut headers)?;
|
||||
if let Some(message) = invalid_gemini_provider_success_message(
|
||||
&plan,
|
||||
report_context.as_ref(),
|
||||
result.status_code,
|
||||
body_json.as_ref(),
|
||||
) {
|
||||
result.status_code = StatusCode::BAD_GATEWAY.as_u16();
|
||||
result.error = Some(ExecutionError {
|
||||
kind: ExecutionErrorKind::Upstream5xx,
|
||||
phase: ExecutionPhase::Finalize,
|
||||
message: message.to_string(),
|
||||
upstream_status: Some(StatusCode::OK.as_u16()),
|
||||
retryable: false,
|
||||
failover_recommended: false,
|
||||
});
|
||||
if let Some(error_body) =
|
||||
build_invalid_provider_success_body(&plan, report_context.as_ref(), message)
|
||||
{
|
||||
body_json = Some(error_body);
|
||||
headers.insert("content-type".to_string(), "application/json".to_string());
|
||||
}
|
||||
}
|
||||
let (result_error_type, result_error_message) =
|
||||
execution_error_details(result.error.as_ref(), body_json.as_ref());
|
||||
let local_failover_response_text = local_failover_response_text(
|
||||
body_json.as_ref(),
|
||||
&body_bytes,
|
||||
@@ -2058,6 +2128,41 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
fn test_gemini_chat_plan() -> ExecutionPlan {
|
||||
let mut plan = test_openai_image_plan(false);
|
||||
plan.client_api_format = "openai:chat".to_string();
|
||||
plan.provider_api_format = "gemini:generate_content".to_string();
|
||||
plan.model_name = Some("gemini-3-flash-preview".to_string());
|
||||
plan
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_gemini_provider_success_uses_plan_format_when_context_is_missing() {
|
||||
let plan = test_gemini_chat_plan();
|
||||
let body = json!({
|
||||
"candidates": [{
|
||||
"content": {"role": "model"},
|
||||
"finishReason": "MAX_TOKENS"
|
||||
}],
|
||||
"usageMetadata": {
|
||||
"promptTokenCount": 8,
|
||||
"candidatesTokenCount": 1,
|
||||
"thoughtsTokenCount": 25,
|
||||
"totalTokenCount": 34
|
||||
}
|
||||
});
|
||||
|
||||
let message = invalid_gemini_provider_success_message(
|
||||
&plan,
|
||||
None,
|
||||
StatusCode::OK.as_u16(),
|
||||
Some(&body),
|
||||
)
|
||||
.expect("empty Gemini 200 response should be rejected from plan format");
|
||||
|
||||
assert!(message.contains("visible model output"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn json_whitespace_heartbeat_stream_prefixes_final_json() {
|
||||
let (tx, rx) = mpsc::channel::<Result<Bytes, IoError>>(1);
|
||||
|
||||
@@ -555,7 +555,6 @@ fn provider_query_build_test_request_body_with_model_policy(
|
||||
"content": provider_query_extract_message(payload)
|
||||
.unwrap_or_else(|| DEFAULT_PROVIDER_QUERY_TEST_MESSAGE.to_string())
|
||||
}],
|
||||
"max_tokens": 30,
|
||||
"temperature": 0.7,
|
||||
"stream": true,
|
||||
})
|
||||
@@ -1256,7 +1255,7 @@ fn provider_query_standard_execution_response_body(
|
||||
provider_api_format: &str,
|
||||
result: &aether_contracts::ExecutionResult,
|
||||
) -> Option<Value> {
|
||||
result
|
||||
let body = result
|
||||
.body
|
||||
.as_ref()
|
||||
.and_then(|body| body.json_body.clone())
|
||||
@@ -1264,7 +1263,15 @@ fn provider_query_standard_execution_response_body(
|
||||
provider_query_decode_execution_body(result).and_then(|body| {
|
||||
provider_query_aggregate_standard_stream_sync_response(provider_api_format, &body)
|
||||
})
|
||||
})
|
||||
})?;
|
||||
if result.status_code < 400
|
||||
&& provider_query_normalize_api_format_alias(provider_api_format)
|
||||
== "gemini:generate_content"
|
||||
&& aether_ai_formats::formats::gemini::generate_content::response::from_raw(&body).is_none()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
Some(body)
|
||||
}
|
||||
|
||||
fn provider_query_extract_error_message(
|
||||
|
||||
@@ -28,6 +28,17 @@ fn provider_query_test_request_body_defaults_missing_model() {
|
||||
assert_eq!(body["model"], json!("fallback-model"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_query_default_test_request_body_does_not_set_max_tokens() {
|
||||
let body = provider_query_build_test_request_body(&json!({}), "fallback-model");
|
||||
|
||||
assert_eq!(body["model"], json!("fallback-model"));
|
||||
assert!(
|
||||
body.get("max_tokens").is_none(),
|
||||
"admin model test must not silently force a low max_tokens value"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_query_failover_request_body_overrides_custom_model() {
|
||||
let payload = json!({
|
||||
@@ -168,6 +179,40 @@ fn provider_query_standard_test_aggregates_responses_stream_body() {
|
||||
assert_eq!(body["output"][0]["content"][0]["text"], json!("Hello"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_query_standard_test_rejects_gemini_success_without_visible_output() {
|
||||
let result = aether_contracts::ExecutionResult {
|
||||
request_id: "provider-test".to_string(),
|
||||
candidate_id: Some("candidate-0".to_string()),
|
||||
status_code: 200,
|
||||
headers: BTreeMap::new(),
|
||||
body: Some(aether_contracts::ResponseBody {
|
||||
json_body: Some(json!({
|
||||
"candidates": [{
|
||||
"content": {"role": "model"},
|
||||
"finishReason": "MAX_TOKENS"
|
||||
}],
|
||||
"usageMetadata": {
|
||||
"promptTokenCount": 8,
|
||||
"candidatesTokenCount": 1,
|
||||
"thoughtsTokenCount": 25,
|
||||
"totalTokenCount": 34
|
||||
},
|
||||
"modelVersion": "gemini-3-flash-preview",
|
||||
"responseId": "resp-empty"
|
||||
})),
|
||||
body_bytes_b64: None,
|
||||
}),
|
||||
telemetry: None,
|
||||
error: None,
|
||||
};
|
||||
|
||||
assert!(
|
||||
provider_query_standard_execution_response_body("gemini:generate_content", &result)
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_query_test_adapter_routes_fixed_provider_endpoint_types() {
|
||||
assert_eq!(
|
||||
|
||||
@@ -66,6 +66,10 @@ pub fn from_raw(body_json: &Value) -> Option<CanonicalResponse> {
|
||||
extensions: Default::default(),
|
||||
});
|
||||
}
|
||||
outputs.retain(gemini_response_output_has_visible_content);
|
||||
if outputs.is_empty() {
|
||||
return None;
|
||||
}
|
||||
let content = outputs
|
||||
.first()
|
||||
.map(|output| output.content.clone())
|
||||
@@ -108,6 +112,18 @@ pub fn from_raw(body_json: &Value) -> Option<CanonicalResponse> {
|
||||
Some(canonical)
|
||||
}
|
||||
|
||||
fn gemini_response_output_has_visible_content(output: &CanonicalResponseOutput) -> bool {
|
||||
output.content.iter().any(|block| match block {
|
||||
CanonicalContentBlock::Text { text, .. } => !text.trim().is_empty(),
|
||||
CanonicalContentBlock::ToolUse { .. }
|
||||
| CanonicalContentBlock::ToolResult { .. }
|
||||
| CanonicalContentBlock::Image { .. }
|
||||
| CanonicalContentBlock::File { .. }
|
||||
| CanonicalContentBlock::Audio { .. } => true,
|
||||
CanonicalContentBlock::Thinking { .. } | CanonicalContentBlock::Unknown { .. } => false,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn to_raw(canonical: &CanonicalResponse, report_context: &Value) -> Option<Value> {
|
||||
let mut response = canonical_to_gemini_response(canonical, report_context)?;
|
||||
if let Some(object) = response.as_object_mut() {
|
||||
@@ -353,3 +369,72 @@ fn canonical_usage_to_gemini_usage_metadata(usage: &CanonicalUsage) -> Value {
|
||||
}
|
||||
Value::Object(out)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::CanonicalContentBlock;
|
||||
|
||||
#[test]
|
||||
fn gemini_response_without_visible_parts_is_not_success() {
|
||||
let body = json!({
|
||||
"candidates": [{
|
||||
"content": {"role": "model"},
|
||||
"finishReason": "MAX_TOKENS"
|
||||
}],
|
||||
"usageMetadata": {
|
||||
"promptTokenCount": 8,
|
||||
"candidatesTokenCount": 1,
|
||||
"thoughtsTokenCount": 25,
|
||||
"totalTokenCount": 34
|
||||
},
|
||||
"modelVersion": "gemini-3-flash-preview",
|
||||
"responseId": "resp-empty"
|
||||
});
|
||||
|
||||
assert!(from_raw(&body).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gemini_response_with_only_thought_parts_is_not_success() {
|
||||
let body = json!({
|
||||
"candidates": [{
|
||||
"content": {
|
||||
"role": "model",
|
||||
"parts": [{"text": "hidden plan", "thought": true}]
|
||||
},
|
||||
"finishReason": "MAX_TOKENS"
|
||||
}],
|
||||
"modelVersion": "gemini-3-flash-preview",
|
||||
"responseId": "resp-thought-only"
|
||||
});
|
||||
|
||||
assert!(from_raw(&body).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gemini_response_with_function_call_is_visible_output() {
|
||||
let body = json!({
|
||||
"candidates": [{
|
||||
"content": {
|
||||
"role": "model",
|
||||
"parts": [{
|
||||
"functionCall": {
|
||||
"name": "lookup",
|
||||
"args": {"query": "weather"}
|
||||
}
|
||||
}]
|
||||
},
|
||||
"finishReason": "STOP"
|
||||
}],
|
||||
"modelVersion": "gemini-3-flash-preview",
|
||||
"responseId": "resp-tool"
|
||||
});
|
||||
|
||||
let canonical = from_raw(&body).expect("function call should be visible output");
|
||||
assert!(matches!(
|
||||
canonical.content.first(),
|
||||
Some(CanonicalContentBlock::ToolUse { name, .. }) if name == "lookup"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user