mirror of
https://github.com/openai/codex.git
synced 2026-10-23 18:08:45 +00:00
Treat model-not-found provider errors as terminal failures (#56445)
## Why Provider errors with code `model_not_found` should end the turn immediately and preserve the provider's message instead of triggering retries. ## What changed - Recognize `model_not_found` in HTTP 400, 403, and 404 responses and in streamed `error` and `response.failed` events. - Preserve the provider message and any available HTTP status, and expose `modelNotFound` through the app-server protocol and generated schemas and SDK types. ## Testing Add coverage for HTTP classification, statusless stream errors, and terminal failures without retries across HTTP, SSE, and WebSocket responses. Verify that `thread/read` preserves the error reported by the live notification. GitOrigin-RevId: 88f824ac03c8c9669915b1e40c67b76c08326fbe
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
use crate::TransportError;
|
||||
use crate::error::ApiError;
|
||||
use crate::error::parse_flex_unavailable;
|
||||
use crate::error::parse_terminal_provider_error;
|
||||
use crate::rate_limits::parse_promo_message;
|
||||
use crate::rate_limits::parse_rate_limit_for_limit;
|
||||
use crate::rate_limits::parse_rate_limit_reached_type;
|
||||
@@ -36,6 +36,7 @@ pub fn map_api_error(err: ApiError) -> CodexErr {
|
||||
| ApiError::FlexUnavailable
|
||||
| ApiError::RateLimit(_)
|
||||
| ApiError::InvalidRequest { .. }
|
||||
| ApiError::ModelNotFound { .. }
|
||||
| ApiError::InvalidPrompt { .. }
|
||||
| ApiError::CyberPolicy { .. }
|
||||
| ApiError::BioPolicy { .. }
|
||||
@@ -78,6 +79,9 @@ fn map_api_error_details(err: ApiError) -> CodexErr {
|
||||
})
|
||||
}
|
||||
ApiError::InvalidRequest { message } => CodexErr::InvalidRequest(message),
|
||||
ApiError::ModelNotFound { message, status } => {
|
||||
CodexErr::new(CodexErrorDetails::ModelNotFound { message, status })
|
||||
}
|
||||
ApiError::InvalidPrompt { message } => {
|
||||
CodexErr::new(CodexErrorDetails::InvalidPrompt { message })
|
||||
}
|
||||
@@ -101,10 +105,14 @@ fn map_api_error_details(err: ApiError) -> CodexErr {
|
||||
..
|
||||
} => {
|
||||
let body_text = body.unwrap_or_default();
|
||||
let parsed_body = serde_json::from_str::<Value>(&body_text).ok();
|
||||
let provider_error = parsed_body.as_ref().and_then(|body| body.get("error"));
|
||||
if let Some(error) = parse_terminal_provider_error(provider_error, Some(status)) {
|
||||
return map_api_error_details(error);
|
||||
}
|
||||
|
||||
if status == http::StatusCode::SERVICE_UNAVAILABLE
|
||||
&& let Ok(value) = serde_json::from_str::<serde_json::Value>(&body_text)
|
||||
&& let Some(error) = value.get("error")
|
||||
&& let Some(error) = provider_error
|
||||
{
|
||||
match error.get("code").and_then(Value::as_str) {
|
||||
Some("server_is_overloaded") => return CodexErr::ServerOverloaded,
|
||||
@@ -123,8 +131,7 @@ fn map_api_error_details(err: ApiError) -> CodexErr {
|
||||
|
||||
if (status == http::StatusCode::BAD_REQUEST
|
||||
|| status == http::StatusCode::FORBIDDEN)
|
||||
&& let Ok(parsed) = serde_json::from_str::<Value>(&body_text)
|
||||
&& let Some(error) = parsed.get("error")
|
||||
&& let Some(error) = provider_error
|
||||
&& error.get("code").and_then(Value::as_str)
|
||||
== Some(MISALIGNMENT_POLICY_VIOLATION_ERROR_CODE)
|
||||
{
|
||||
@@ -145,8 +152,7 @@ fn map_api_error_details(err: ApiError) -> CodexErr {
|
||||
}
|
||||
|
||||
if status == http::StatusCode::BAD_REQUEST {
|
||||
if let Ok(parsed) = serde_json::from_str::<Value>(&body_text)
|
||||
&& let Some(error) = parsed.get("error")
|
||||
if let Some(error) = provider_error
|
||||
&& let Some(
|
||||
code @ (CYBER_POLICY_ERROR_CODE
|
||||
| BIO_POLICY_ERROR_CODE
|
||||
@@ -183,11 +189,6 @@ fn map_api_error_details(err: ApiError) -> CodexErr {
|
||||
} else if status == http::StatusCode::INTERNAL_SERVER_ERROR {
|
||||
CodexErr::InternalServerError
|
||||
} else if status == http::StatusCode::TOO_MANY_REQUESTS {
|
||||
if let Ok(body) = serde_json::from_str::<Value>(&body_text)
|
||||
&& let Some(error) = body.get("error").and_then(parse_flex_unavailable)
|
||||
{
|
||||
return map_api_error(error);
|
||||
}
|
||||
if let Ok(err) = serde_json::from_str::<UsageErrorResponse>(&body_text) {
|
||||
if err.error.error_type.as_deref() == Some("usage_limit_reached") {
|
||||
let limit_id = extract_header(headers.as_ref(), ACTIVE_LIMIT_HEADER);
|
||||
|
||||
@@ -125,31 +125,49 @@ async fn http_retry_deadline_survives_mapping_and_respects_terminal_errors() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn map_api_error_distinguishes_capacity_from_slow_down() {
|
||||
for (code, expected, retryable) in [
|
||||
fn map_http_error_classifies_provider_codes() {
|
||||
use CodexErrorInfo::ModelNotFound;
|
||||
|
||||
for (status, code, expected, retryable) in [
|
||||
(
|
||||
503,
|
||||
"server_is_overloaded",
|
||||
CodexErrorInfo::ServerOverloaded,
|
||||
false,
|
||||
),
|
||||
("slow_down", CodexErrorInfo::RateLimitExceeded, true),
|
||||
(503, "slow_down", CodexErrorInfo::RateLimitExceeded, true),
|
||||
(
|
||||
503,
|
||||
"unknown_error",
|
||||
CodexErrorInfo::HttpConnectionFailed {
|
||||
http_status_code: Some(503),
|
||||
},
|
||||
true,
|
||||
),
|
||||
(400, "model_not_found", ModelNotFound, false),
|
||||
(403, "model_not_found", ModelNotFound, false),
|
||||
(404, "model_not_found", ModelNotFound, false),
|
||||
(
|
||||
404,
|
||||
"resource_not_found",
|
||||
CodexErrorInfo::HttpConnectionFailed {
|
||||
http_status_code: Some(404),
|
||||
},
|
||||
true,
|
||||
),
|
||||
] {
|
||||
let err = map_api_error(ApiError::Transport(TransportError::Http {
|
||||
retry_after: None,
|
||||
status: http::StatusCode::SERVICE_UNAVAILABLE,
|
||||
status: http::StatusCode::from_u16(status).unwrap(),
|
||||
url: None,
|
||||
headers: None,
|
||||
body: Some(
|
||||
serde_json::json!({"error": {"code": code, "message": "retry later"}}).to_string(),
|
||||
),
|
||||
}));
|
||||
if expected == ModelNotFound {
|
||||
assert_eq!(err.http_status_code_value(), Some(status));
|
||||
}
|
||||
assert_eq!(
|
||||
(
|
||||
err.to_codex_protocol_error(),
|
||||
|
||||
@@ -39,6 +39,11 @@ pub enum ApiError {
|
||||
RateLimit(String),
|
||||
#[error("invalid request: {message}")]
|
||||
InvalidRequest { message: String },
|
||||
#[error("{message}")]
|
||||
ModelNotFound {
|
||||
message: String,
|
||||
status: Option<StatusCode>,
|
||||
},
|
||||
#[error("invalid prompt: {message}")]
|
||||
InvalidPrompt { message: String },
|
||||
#[error("cyber policy: {message}")]
|
||||
@@ -62,9 +67,30 @@ impl From<RateLimitError> for ApiError {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn parse_flex_unavailable(error: &Value) -> Option<ApiError> {
|
||||
(error.get("code").and_then(Value::as_str) == Some("flex_unavailable"))
|
||||
.then_some(ApiError::FlexUnavailable)
|
||||
pub(crate) fn parse_terminal_provider_error(
|
||||
error: Option<&Value>,
|
||||
status: Option<StatusCode>,
|
||||
) -> Option<ApiError> {
|
||||
let error = error?;
|
||||
let code = error.get("code")?.as_str()?;
|
||||
match (code, status) {
|
||||
("flex_unavailable", None | Some(StatusCode::TOO_MANY_REQUESTS)) => {
|
||||
Some(ApiError::FlexUnavailable)
|
||||
}
|
||||
(
|
||||
"model_not_found",
|
||||
None | Some(StatusCode::BAD_REQUEST | StatusCode::FORBIDDEN | StatusCode::NOT_FOUND),
|
||||
) => Some(ApiError::ModelNotFound {
|
||||
message: error
|
||||
.get("message")
|
||||
.and_then(Value::as_str)
|
||||
.filter(|message| !message.trim().is_empty())
|
||||
.unwrap_or(code)
|
||||
.to_owned(),
|
||||
status,
|
||||
}),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
impl From<crate::AuthError> for ApiError {
|
||||
|
||||
@@ -4,7 +4,7 @@ use crate::common::ResponseStream;
|
||||
use crate::common::SafetyBuffering;
|
||||
use crate::common::SafetyBufferingTreatment;
|
||||
use crate::error::ApiError;
|
||||
use crate::error::parse_flex_unavailable;
|
||||
use crate::error::parse_terminal_provider_error;
|
||||
use crate::rate_limits::parse_all_rate_limits;
|
||||
use crate::safety_buffering::treatment_from_headers;
|
||||
use crate::telemetry::SseTelemetry;
|
||||
@@ -347,7 +347,9 @@ pub fn process_responses_event(
|
||||
) -> std::result::Result<Option<ResponseEvent>, ResponsesEventError> {
|
||||
match event.kind.as_str() {
|
||||
"error" => {
|
||||
if let Some(error) = event.error.as_ref().and_then(parse_flex_unavailable) {
|
||||
if let Some(error) =
|
||||
parse_terminal_provider_error(event.error.as_ref(), /*status*/ None)
|
||||
{
|
||||
return Err(ResponsesEventError::Api(error));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
//! Only rate-limit codes derive retry delays from plaintext messages.
|
||||
|
||||
use crate::error::ApiError;
|
||||
use crate::error::parse_flex_unavailable;
|
||||
use crate::error::parse_terminal_provider_error;
|
||||
use crate::responses_headers::json_headers_to_http_headers;
|
||||
use codex_http_client::RetryAfter;
|
||||
use codex_protocol::protocol::MisalignmentErrorDetails;
|
||||
@@ -29,7 +29,7 @@ pub(super) fn parse_failed_response(response: Option<Value>) -> ApiError {
|
||||
.and_then(|error| error.get("headers"))
|
||||
.and_then(Value::as_object)
|
||||
.and_then(|headers| RetryAfter::from_headers(&json_headers_to_http_headers(headers)));
|
||||
if let Some(error) = error.and_then(parse_flex_unavailable) {
|
||||
if let Some(error) = parse_terminal_provider_error(error, /*status*/ None) {
|
||||
return error;
|
||||
}
|
||||
let Some(error) = error.and_then(|error| serde_json::from_value::<Error>(error.clone()).ok())
|
||||
|
||||
@@ -61,6 +61,25 @@ async fn flex_failure_with_retry_header_ends_stream_immediately() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn statusless_model_errors_have_no_http_status() {
|
||||
let error = json!({"code": "model_not_found"});
|
||||
for event in [
|
||||
json!({"type": "error", "error": error}),
|
||||
json!({"type": "response.failed", "response": {"error": error}}),
|
||||
] {
|
||||
let event = serde_json::from_value(event).unwrap();
|
||||
let error = map_api_error(process_responses_event(event).unwrap_err().into_api_error());
|
||||
assert_eq!(
|
||||
(
|
||||
error.to_codex_protocol_error(),
|
||||
error.http_status_code_value()
|
||||
),
|
||||
(CodexErrorInfo::ModelNotFound, None)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Error headers beat rate-limit message advice; invalid ones fall back.
|
||||
#[tokio::test(start_paused = true)]
|
||||
async fn rate_limit_plaintext_loses_to_valid_error_header() {
|
||||
|
||||
Reference in New Issue
Block a user