From 72a1453ac5b4ee60f19876542b7d7203e83cde62 Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Wed, 19 Nov 2025 17:26:14 -0800 Subject: [PATCH] Revert "[core] add optional status_code to error events (#6865)" (#6955) This reverts commit c2ec477d939af2a275255666b37fe9b0acd7492a. # External (non-OpenAI) Pull Request Requirements Before opening this Pull Request, please read the dedicated "Contributing" markdown file or your PR may be closed: https://github.com/openai/codex/blob/main/docs/contributing.md If your PR conforms to our contribution guidelines, replace this text with a detailed and high quality description of your changes. Include a link to a bug report or enhancement request. --- codex-rs/core/src/codex.rs | 13 ++-- codex-rs/core/src/compact.rs | 14 ++-- codex-rs/core/src/compact_remote.rs | 7 +- codex-rs/core/src/error.rs | 71 ------------------- codex-rs/docs/protocol_v1.md | 2 +- .../src/event_processor_with_human_output.rs | 4 +- .../tests/event_processor_with_json_output.rs | 3 - codex-rs/protocol/src/protocol.rs | 4 -- codex-rs/tui/src/chatwidget.rs | 6 +- codex-rs/tui/src/chatwidget/agent.rs | 5 +- codex-rs/tui/src/chatwidget/tests.rs | 1 - 11 files changed, 25 insertions(+), 105 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 897f1f294..c793eff35 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -66,7 +66,6 @@ use crate::context_manager::ContextManager; use crate::environment_context::EnvironmentContext; use crate::error::CodexErr; use crate::error::Result as CodexResult; -use crate::error::http_status_code_value; #[cfg(test)] use crate::exec::StreamOutput; use crate::mcp::auth::compute_auth_statuses; @@ -80,6 +79,7 @@ use crate::protocol::ApplyPatchApprovalRequestEvent; use crate::protocol::AskForApproval; use crate::protocol::BackgroundEventEvent; use crate::protocol::DeprecationNoticeEvent; +use crate::protocol::ErrorEvent; use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::ExecApprovalRequestEvent; @@ -134,7 +134,6 @@ use codex_protocol::user_input::UserInput; use codex_utils_readiness::Readiness; use codex_utils_readiness::ReadinessFlag; use codex_utils_tokenizer::warm_model_cache; -use reqwest::StatusCode; /// The high-level interface to the Codex system. /// It operates as a queue pair where you send submissions and receive events. @@ -1197,11 +1196,9 @@ impl Session { &self, turn_context: &TurnContext, message: impl Into, - http_status_code: Option, ) { let event = EventMsg::StreamError(StreamErrorEvent { message: message.into(), - http_status_code: http_status_code_value(http_status_code), }); self.send_event(turn_context, event).await; } @@ -1693,7 +1690,6 @@ mod handlers { id: sub_id.clone(), msg: EventMsg::Error(ErrorEvent { message: "Failed to shutdown rollout recorder".to_string(), - http_status_code: None, }), }; sess.send_event_raw(event).await; @@ -1948,8 +1944,10 @@ pub(crate) async fn run_task( } Err(e) => { info!("Turn error: {e:#}"); - sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None))) - .await; + let event = EventMsg::Error(ErrorEvent { + message: e.to_string(), + }); + sess.send_event(&turn_context, event).await; // let the user continue the conversation break; } @@ -2073,7 +2071,6 @@ async fn run_turn( sess.notify_stream_error( &turn_context, format!("Reconnecting... {retries}/{max_retries}"), - e.http_status_code(), ) .await; diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index 67b5b9342..8c38f9393 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -10,6 +10,7 @@ use crate::error::Result as CodexResult; use crate::features::Feature; use crate::protocol::AgentMessageEvent; use crate::protocol::CompactedItem; +use crate::protocol::ErrorEvent; use crate::protocol::EventMsg; use crate::protocol::TaskStartedEvent; use crate::protocol::TurnContextItem; @@ -127,8 +128,10 @@ async fn run_compact_task_inner( continue; } sess.set_total_tokens_full(turn_context.as_ref()).await; - sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None))) - .await; + let event = EventMsg::Error(ErrorEvent { + message: e.to_string(), + }); + sess.send_event(&turn_context, event).await; return; } Err(e) => { @@ -138,14 +141,15 @@ async fn run_compact_task_inner( sess.notify_stream_error( turn_context.as_ref(), format!("Reconnecting... {retries}/{max_retries}"), - e.http_status_code(), ) .await; tokio::time::sleep(delay).await; continue; } else { - sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None))) - .await; + let event = EventMsg::Error(ErrorEvent { + message: e.to_string(), + }); + sess.send_event(&turn_context, event).await; return; } } diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index aa32942dc..0d2e0f138 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -6,6 +6,7 @@ use crate::codex::TurnContext; use crate::error::Result as CodexResult; use crate::protocol::AgentMessageEvent; use crate::protocol::CompactedItem; +use crate::protocol::ErrorEvent; use crate::protocol::EventMsg; use crate::protocol::RolloutItem; use crate::protocol::TaskStartedEvent; @@ -29,8 +30,10 @@ pub(crate) async fn run_remote_compact_task(sess: Arc, turn_context: Ar async fn run_remote_compact_task_inner(sess: &Arc, turn_context: &Arc) { if let Err(err) = run_remote_compact_task_inner_impl(sess, turn_context).await { - let event = err.to_error_event(Some("Error running remote compact task".to_string())); - sess.send_event(turn_context, EventMsg::Error(event)).await; + let event = EventMsg::Error(ErrorEvent { + message: format!("Error running remote compact task: {err}"), + }); + sess.send_event(turn_context, event).await; } } diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index d20d5cfb4..b2027dc94 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -10,7 +10,6 @@ use chrono::Local; use chrono::Utc; use codex_async_utils::CancelErr; use codex_protocol::ConversationId; -use codex_protocol::protocol::ErrorEvent; use codex_protocol::protocol::RateLimitSnapshot; use reqwest::StatusCode; use serde_json; @@ -431,37 +430,6 @@ impl CodexErr { pub fn downcast_ref(&self) -> Option<&T> { (self as &dyn std::any::Any).downcast_ref::() } - - pub fn http_status_code(&self) -> Option { - match self { - CodexErr::UnexpectedStatus(err) => Some(err.status), - CodexErr::RetryLimit(err) => Some(err.status), - CodexErr::UsageLimitReached(_) | CodexErr::UsageNotIncluded => { - Some(StatusCode::TOO_MANY_REQUESTS) - } - CodexErr::InternalServerError => Some(StatusCode::INTERNAL_SERVER_ERROR), - CodexErr::ResponseStreamFailed(err) => err.source.status(), - CodexErr::ConnectionFailed(err) => err.source.status(), - _ => None, - } - } - - pub fn to_error_event(&self, message_prefix: Option) -> ErrorEvent { - let error_message = self.to_string(); - let message: String = match message_prefix { - Some(prefix) => format!("{prefix}: {error_message}"), - None => error_message, - }; - - ErrorEvent { - message, - http_status_code: http_status_code_value(self.http_status_code()), - } - } -} - -pub fn http_status_code_value(http_status_code: Option) -> Option { - http_status_code.as_ref().map(StatusCode::as_u16) } pub fn get_error_message_ui(e: &CodexErr) -> String { @@ -807,43 +775,4 @@ mod tests { assert_eq!(err.to_string(), expected); }); } - - #[test] - fn error_event_includes_http_status_code_when_available() { - let err = CodexErr::UnexpectedStatus(UnexpectedResponseError { - status: StatusCode::BAD_REQUEST, - body: "oops".to_string(), - request_id: Some("req-1".to_string()), - }); - let event = err.to_error_event(None); - - assert_eq!( - event.message, - "unexpected status 400 Bad Request: oops, request id: req-1" - ); - assert_eq!( - event.http_status_code, - Some(StatusCode::BAD_REQUEST.as_u16()) - ); - } - - #[test] - fn error_event_omits_http_status_code_when_unknown() { - let event = CodexErr::Fatal("boom".to_string()).to_error_event(None); - - assert_eq!(event.message, "Fatal error: boom"); - assert_eq!(event.http_status_code, None); - } - - #[test] - fn error_event_applies_message_wrapper() { - let event = CodexErr::Fatal("boom".to_string()) - .to_error_event(Some("Error running remote compact task".to_string())); - - assert_eq!( - event.message, - "Error running remote compact task: Fatal error: boom" - ); - assert_eq!(event.http_status_code, None); - } } diff --git a/codex-rs/docs/protocol_v1.md b/codex-rs/docs/protocol_v1.md index c6af7c926..f6405c9bc 100644 --- a/codex-rs/docs/protocol_v1.md +++ b/codex-rs/docs/protocol_v1.md @@ -72,7 +72,7 @@ For complete documentation of the `Op` and `EventMsg` variants, refer to [protoc - `EventMsg::AgentMessage` – Messages from the `Model` - `EventMsg::ExecApprovalRequest` – Request approval from user to execute a command - `EventMsg::TaskComplete` – A task completed successfully - - `EventMsg::Error` – A task stopped with an error (includes an optional `http_status_code` when available) + - `EventMsg::Error` – A task stopped with an error - `EventMsg::Warning` – A non-fatal warning that the client should surface to the user - `EventMsg::TurnComplete` – Contains a `response_id` bookmark for last `response_id` executed by the task. This can be used to continue the task at a later point in time, perhaps with additional user input. diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 86af82118..2d550fea4 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -161,7 +161,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { fn process_event(&mut self, event: Event) -> CodexStatus { let Event { id: _, msg } = event; match msg { - EventMsg::Error(ErrorEvent { message, .. }) => { + EventMsg::Error(ErrorEvent { message }) => { let prefix = "ERROR:".style(self.red); ts_msg!(self, "{prefix} {message}"); } @@ -221,7 +221,7 @@ impl EventProcessor for EventProcessorWithHumanOutput { EventMsg::BackgroundEvent(BackgroundEventEvent { message }) => { ts_msg!(self, "{}", message.style(self.dimmed)); } - EventMsg::StreamError(StreamErrorEvent { message, .. }) => { + EventMsg::StreamError(StreamErrorEvent { message }) => { ts_msg!(self, "{}", message.style(self.dimmed)); } EventMsg::TaskStarted(_) => { diff --git a/codex-rs/exec/tests/event_processor_with_json_output.rs b/codex-rs/exec/tests/event_processor_with_json_output.rs index 444d32304..5053f6192 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -539,7 +539,6 @@ fn error_event_produces_error() { "e1", EventMsg::Error(codex_core::protocol::ErrorEvent { message: "boom".to_string(), - http_status_code: Some(500), }), )); assert_eq!( @@ -579,7 +578,6 @@ fn stream_error_event_produces_error() { "e1", EventMsg::StreamError(codex_core::protocol::StreamErrorEvent { message: "retrying".to_string(), - http_status_code: Some(500), }), )); assert_eq!( @@ -598,7 +596,6 @@ fn error_followed_by_task_complete_produces_turn_failed() { "e1", EventMsg::Error(ErrorEvent { message: "boom".to_string(), - http_status_code: Some(500), }), ); assert_eq!( diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 0158ce9cf..1825d7636 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -686,8 +686,6 @@ pub struct ExitedReviewModeEvent { #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct ErrorEvent { pub message: String, - #[serde(default)] - pub http_status_code: Option, } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] @@ -1365,8 +1363,6 @@ pub struct UndoCompletedEvent { #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct StreamErrorEvent { pub message: String, - #[serde(default)] - pub http_status_code: Option, } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 3a79330b7..987a956fe 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1654,7 +1654,7 @@ impl ChatWidget { self.on_rate_limit_snapshot(ev.rate_limits); } EventMsg::Warning(WarningEvent { message }) => self.on_warning(message), - EventMsg::Error(ErrorEvent { message, .. }) => self.on_error(message), + EventMsg::Error(ErrorEvent { message }) => self.on_error(message), EventMsg::McpStartupUpdate(ev) => self.on_mcp_startup_update(ev), EventMsg::McpStartupComplete(ev) => self.on_mcp_startup_complete(ev), EventMsg::TurnAborted(ev) => match ev.reason { @@ -1697,9 +1697,7 @@ impl ChatWidget { } EventMsg::UndoStarted(ev) => self.on_undo_started(ev), EventMsg::UndoCompleted(ev) => self.on_undo_completed(ev), - EventMsg::StreamError(StreamErrorEvent { message, .. }) => { - self.on_stream_error(message) - } + EventMsg::StreamError(StreamErrorEvent { message }) => self.on_stream_error(message), EventMsg::UserMessage(ev) => { if from_replay { self.on_user_message_event(ev); diff --git a/codex-rs/tui/src/chatwidget/agent.rs b/codex-rs/tui/src/chatwidget/agent.rs index 0abddac50..3c326fed8 100644 --- a/codex-rs/tui/src/chatwidget/agent.rs +++ b/codex-rs/tui/src/chatwidget/agent.rs @@ -37,10 +37,7 @@ pub(crate) fn spawn_agent( eprintln!("{message}"); app_event_tx_clone.send(AppEvent::CodexEvent(Event { id: "".to_string(), - msg: EventMsg::Error(ErrorEvent { - message, - http_status_code: None, - }), + msg: EventMsg::Error(ErrorEvent { message }), })); app_event_tx_clone.send(AppEvent::ExitRequest); tracing::error!("failed to initialize codex: {err}"); diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index a4cef2241..d60a20b8f 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2596,7 +2596,6 @@ fn stream_error_updates_status_indicator() { id: "sub-1".into(), msg: EventMsg::StreamError(StreamErrorEvent { message: msg.to_string(), - http_status_code: None, }), });