Skip to content

Commit ce150db

Browse files
authored
Revert "[core] add optional status_code to error events (openai#6865)" (openai#6955)
This reverts commit c2ec477. # 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.
1 parent 2736e14 commit ce150db

File tree

11 files changed

+25
-105
lines changed

11 files changed

+25
-105
lines changed

codex-rs/core/src/codex.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ use crate::context_manager::ContextManager;
6666
use crate::environment_context::EnvironmentContext;
6767
use crate::error::CodexErr;
6868
use crate::error::Result as CodexResult;
69-
use crate::error::http_status_code_value;
7069
#[cfg(test)]
7170
use crate::exec::StreamOutput;
7271
use crate::mcp::auth::compute_auth_statuses;
@@ -80,6 +79,7 @@ use crate::protocol::ApplyPatchApprovalRequestEvent;
8079
use crate::protocol::AskForApproval;
8180
use crate::protocol::BackgroundEventEvent;
8281
use crate::protocol::DeprecationNoticeEvent;
82+
use crate::protocol::ErrorEvent;
8383
use crate::protocol::Event;
8484
use crate::protocol::EventMsg;
8585
use crate::protocol::ExecApprovalRequestEvent;
@@ -134,7 +134,6 @@ use codex_protocol::user_input::UserInput;
134134
use codex_utils_readiness::Readiness;
135135
use codex_utils_readiness::ReadinessFlag;
136136
use codex_utils_tokenizer::warm_model_cache;
137-
use reqwest::StatusCode;
138137

139138
/// The high-level interface to the Codex system.
140139
/// It operates as a queue pair where you send submissions and receive events.
@@ -1197,11 +1196,9 @@ impl Session {
11971196
&self,
11981197
turn_context: &TurnContext,
11991198
message: impl Into<String>,
1200-
http_status_code: Option<StatusCode>,
12011199
) {
12021200
let event = EventMsg::StreamError(StreamErrorEvent {
12031201
message: message.into(),
1204-
http_status_code: http_status_code_value(http_status_code),
12051202
});
12061203
self.send_event(turn_context, event).await;
12071204
}
@@ -1693,7 +1690,6 @@ mod handlers {
16931690
id: sub_id.clone(),
16941691
msg: EventMsg::Error(ErrorEvent {
16951692
message: "Failed to shutdown rollout recorder".to_string(),
1696-
http_status_code: None,
16971693
}),
16981694
};
16991695
sess.send_event_raw(event).await;
@@ -1948,8 +1944,10 @@ pub(crate) async fn run_task(
19481944
}
19491945
Err(e) => {
19501946
info!("Turn error: {e:#}");
1951-
sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None)))
1952-
.await;
1947+
let event = EventMsg::Error(ErrorEvent {
1948+
message: e.to_string(),
1949+
});
1950+
sess.send_event(&turn_context, event).await;
19531951
// let the user continue the conversation
19541952
break;
19551953
}
@@ -2073,7 +2071,6 @@ async fn run_turn(
20732071
sess.notify_stream_error(
20742072
&turn_context,
20752073
format!("Reconnecting... {retries}/{max_retries}"),
2076-
e.http_status_code(),
20772074
)
20782075
.await;
20792076

codex-rs/core/src/compact.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::error::Result as CodexResult;
1010
use crate::features::Feature;
1111
use crate::protocol::AgentMessageEvent;
1212
use crate::protocol::CompactedItem;
13+
use crate::protocol::ErrorEvent;
1314
use crate::protocol::EventMsg;
1415
use crate::protocol::TaskStartedEvent;
1516
use crate::protocol::TurnContextItem;
@@ -127,8 +128,10 @@ async fn run_compact_task_inner(
127128
continue;
128129
}
129130
sess.set_total_tokens_full(turn_context.as_ref()).await;
130-
sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None)))
131-
.await;
131+
let event = EventMsg::Error(ErrorEvent {
132+
message: e.to_string(),
133+
});
134+
sess.send_event(&turn_context, event).await;
132135
return;
133136
}
134137
Err(e) => {
@@ -138,14 +141,15 @@ async fn run_compact_task_inner(
138141
sess.notify_stream_error(
139142
turn_context.as_ref(),
140143
format!("Reconnecting... {retries}/{max_retries}"),
141-
e.http_status_code(),
142144
)
143145
.await;
144146
tokio::time::sleep(delay).await;
145147
continue;
146148
} else {
147-
sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None)))
148-
.await;
149+
let event = EventMsg::Error(ErrorEvent {
150+
message: e.to_string(),
151+
});
152+
sess.send_event(&turn_context, event).await;
149153
return;
150154
}
151155
}

codex-rs/core/src/compact_remote.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::codex::TurnContext;
66
use crate::error::Result as CodexResult;
77
use crate::protocol::AgentMessageEvent;
88
use crate::protocol::CompactedItem;
9+
use crate::protocol::ErrorEvent;
910
use crate::protocol::EventMsg;
1011
use crate::protocol::RolloutItem;
1112
use crate::protocol::TaskStartedEvent;
@@ -29,8 +30,10 @@ pub(crate) async fn run_remote_compact_task(sess: Arc<Session>, turn_context: Ar
2930

3031
async fn run_remote_compact_task_inner(sess: &Arc<Session>, turn_context: &Arc<TurnContext>) {
3132
if let Err(err) = run_remote_compact_task_inner_impl(sess, turn_context).await {
32-
let event = err.to_error_event(Some("Error running remote compact task".to_string()));
33-
sess.send_event(turn_context, EventMsg::Error(event)).await;
33+
let event = EventMsg::Error(ErrorEvent {
34+
message: format!("Error running remote compact task: {err}"),
35+
});
36+
sess.send_event(turn_context, event).await;
3437
}
3538
}
3639

codex-rs/core/src/error.rs

Lines changed: 0 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use chrono::Local;
1010
use chrono::Utc;
1111
use codex_async_utils::CancelErr;
1212
use codex_protocol::ConversationId;
13-
use codex_protocol::protocol::ErrorEvent;
1413
use codex_protocol::protocol::RateLimitSnapshot;
1514
use reqwest::StatusCode;
1615
use serde_json;
@@ -431,37 +430,6 @@ impl CodexErr {
431430
pub fn downcast_ref<T: std::any::Any>(&self) -> Option<&T> {
432431
(self as &dyn std::any::Any).downcast_ref::<T>()
433432
}
434-
435-
pub fn http_status_code(&self) -> Option<StatusCode> {
436-
match self {
437-
CodexErr::UnexpectedStatus(err) => Some(err.status),
438-
CodexErr::RetryLimit(err) => Some(err.status),
439-
CodexErr::UsageLimitReached(_) | CodexErr::UsageNotIncluded => {
440-
Some(StatusCode::TOO_MANY_REQUESTS)
441-
}
442-
CodexErr::InternalServerError => Some(StatusCode::INTERNAL_SERVER_ERROR),
443-
CodexErr::ResponseStreamFailed(err) => err.source.status(),
444-
CodexErr::ConnectionFailed(err) => err.source.status(),
445-
_ => None,
446-
}
447-
}
448-
449-
pub fn to_error_event(&self, message_prefix: Option<String>) -> ErrorEvent {
450-
let error_message = self.to_string();
451-
let message: String = match message_prefix {
452-
Some(prefix) => format!("{prefix}: {error_message}"),
453-
None => error_message,
454-
};
455-
456-
ErrorEvent {
457-
message,
458-
http_status_code: http_status_code_value(self.http_status_code()),
459-
}
460-
}
461-
}
462-
463-
pub fn http_status_code_value(http_status_code: Option<StatusCode>) -> Option<u16> {
464-
http_status_code.as_ref().map(StatusCode::as_u16)
465433
}
466434

467435
pub fn get_error_message_ui(e: &CodexErr) -> String {
@@ -807,43 +775,4 @@ mod tests {
807775
assert_eq!(err.to_string(), expected);
808776
});
809777
}
810-
811-
#[test]
812-
fn error_event_includes_http_status_code_when_available() {
813-
let err = CodexErr::UnexpectedStatus(UnexpectedResponseError {
814-
status: StatusCode::BAD_REQUEST,
815-
body: "oops".to_string(),
816-
request_id: Some("req-1".to_string()),
817-
});
818-
let event = err.to_error_event(None);
819-
820-
assert_eq!(
821-
event.message,
822-
"unexpected status 400 Bad Request: oops, request id: req-1"
823-
);
824-
assert_eq!(
825-
event.http_status_code,
826-
Some(StatusCode::BAD_REQUEST.as_u16())
827-
);
828-
}
829-
830-
#[test]
831-
fn error_event_omits_http_status_code_when_unknown() {
832-
let event = CodexErr::Fatal("boom".to_string()).to_error_event(None);
833-
834-
assert_eq!(event.message, "Fatal error: boom");
835-
assert_eq!(event.http_status_code, None);
836-
}
837-
838-
#[test]
839-
fn error_event_applies_message_wrapper() {
840-
let event = CodexErr::Fatal("boom".to_string())
841-
.to_error_event(Some("Error running remote compact task".to_string()));
842-
843-
assert_eq!(
844-
event.message,
845-
"Error running remote compact task: Fatal error: boom"
846-
);
847-
assert_eq!(event.http_status_code, None);
848-
}
849778
}

codex-rs/docs/protocol_v1.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ For complete documentation of the `Op` and `EventMsg` variants, refer to [protoc
7272
- `EventMsg::AgentMessage` – Messages from the `Model`
7373
- `EventMsg::ExecApprovalRequest` – Request approval from user to execute a command
7474
- `EventMsg::TaskComplete` – A task completed successfully
75-
- `EventMsg::Error` – A task stopped with an error (includes an optional `http_status_code` when available)
75+
- `EventMsg::Error` – A task stopped with an error
7676
- `EventMsg::Warning` – A non-fatal warning that the client should surface to the user
7777
- `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.
7878

codex-rs/exec/src/event_processor_with_human_output.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
161161
fn process_event(&mut self, event: Event) -> CodexStatus {
162162
let Event { id: _, msg } = event;
163163
match msg {
164-
EventMsg::Error(ErrorEvent { message, .. }) => {
164+
EventMsg::Error(ErrorEvent { message }) => {
165165
let prefix = "ERROR:".style(self.red);
166166
ts_msg!(self, "{prefix} {message}");
167167
}
@@ -221,7 +221,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
221221
EventMsg::BackgroundEvent(BackgroundEventEvent { message }) => {
222222
ts_msg!(self, "{}", message.style(self.dimmed));
223223
}
224-
EventMsg::StreamError(StreamErrorEvent { message, .. }) => {
224+
EventMsg::StreamError(StreamErrorEvent { message }) => {
225225
ts_msg!(self, "{}", message.style(self.dimmed));
226226
}
227227
EventMsg::TaskStarted(_) => {

codex-rs/exec/tests/event_processor_with_json_output.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,6 @@ fn error_event_produces_error() {
539539
"e1",
540540
EventMsg::Error(codex_core::protocol::ErrorEvent {
541541
message: "boom".to_string(),
542-
http_status_code: Some(500),
543542
}),
544543
));
545544
assert_eq!(
@@ -579,7 +578,6 @@ fn stream_error_event_produces_error() {
579578
"e1",
580579
EventMsg::StreamError(codex_core::protocol::StreamErrorEvent {
581580
message: "retrying".to_string(),
582-
http_status_code: Some(500),
583581
}),
584582
));
585583
assert_eq!(
@@ -598,7 +596,6 @@ fn error_followed_by_task_complete_produces_turn_failed() {
598596
"e1",
599597
EventMsg::Error(ErrorEvent {
600598
message: "boom".to_string(),
601-
http_status_code: Some(500),
602599
}),
603600
);
604601
assert_eq!(

codex-rs/protocol/src/protocol.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -686,8 +686,6 @@ pub struct ExitedReviewModeEvent {
686686
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
687687
pub struct ErrorEvent {
688688
pub message: String,
689-
#[serde(default)]
690-
pub http_status_code: Option<u16>,
691689
}
692690

693691
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
@@ -1365,8 +1363,6 @@ pub struct UndoCompletedEvent {
13651363
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
13661364
pub struct StreamErrorEvent {
13671365
pub message: String,
1368-
#[serde(default)]
1369-
pub http_status_code: Option<u16>,
13701366
}
13711367

13721368
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]

codex-rs/tui/src/chatwidget.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1654,7 +1654,7 @@ impl ChatWidget {
16541654
self.on_rate_limit_snapshot(ev.rate_limits);
16551655
}
16561656
EventMsg::Warning(WarningEvent { message }) => self.on_warning(message),
1657-
EventMsg::Error(ErrorEvent { message, .. }) => self.on_error(message),
1657+
EventMsg::Error(ErrorEvent { message }) => self.on_error(message),
16581658
EventMsg::McpStartupUpdate(ev) => self.on_mcp_startup_update(ev),
16591659
EventMsg::McpStartupComplete(ev) => self.on_mcp_startup_complete(ev),
16601660
EventMsg::TurnAborted(ev) => match ev.reason {
@@ -1697,9 +1697,7 @@ impl ChatWidget {
16971697
}
16981698
EventMsg::UndoStarted(ev) => self.on_undo_started(ev),
16991699
EventMsg::UndoCompleted(ev) => self.on_undo_completed(ev),
1700-
EventMsg::StreamError(StreamErrorEvent { message, .. }) => {
1701-
self.on_stream_error(message)
1702-
}
1700+
EventMsg::StreamError(StreamErrorEvent { message }) => self.on_stream_error(message),
17031701
EventMsg::UserMessage(ev) => {
17041702
if from_replay {
17051703
self.on_user_message_event(ev);

codex-rs/tui/src/chatwidget/agent.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,7 @@ pub(crate) fn spawn_agent(
3737
eprintln!("{message}");
3838
app_event_tx_clone.send(AppEvent::CodexEvent(Event {
3939
id: "".to_string(),
40-
msg: EventMsg::Error(ErrorEvent {
41-
message,
42-
http_status_code: None,
43-
}),
40+
msg: EventMsg::Error(ErrorEvent { message }),
4441
}));
4542
app_event_tx_clone.send(AppEvent::ExitRequest);
4643
tracing::error!("failed to initialize codex: {err}");

0 commit comments

Comments
 (0)