mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[core] add optional status_code to error events (#6865)
We want to better uncover error status code for clients. Add an optional
status_code to error events (thread error, error, stream error) so app
server could uncover the status code from the client side later.
in event log:
```
< {
< "method": "codex/event/stream_error",
< "params": {
< "conversationId": "019a9a32-f576-7292-9711-8e57e8063536",
< "id": "0",
< "msg": {
< "message": "Reconnecting... 5/5",
< "status_code": 401,
< "type": "stream_error"
< }
< }
< }
< {
< "method": "codex/event/error",
< "params": {
< "conversationId": "019a9a32-f576-7292-9711-8e57e8063536",
< "id": "0",
< "msg": {
< "message": "exceeded retry limit, last status: 401 Unauthorized, request id: 9a0cb03a485067f7-SJC",
< "status_code": 401,
< "type": "error"
< }
< }
< }
```
This commit is contained in:
committed by
GitHub
Unverified
parent
20982d5c6a
commit
c2ec477d93
@@ -66,6 +66,7 @@ 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;
|
||||
@@ -79,7 +80,6 @@ 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;
|
||||
@@ -133,6 +133,7 @@ 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.
|
||||
@@ -1186,9 +1187,11 @@ impl Session {
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
message: impl Into<String>,
|
||||
http_status_code: Option<StatusCode>,
|
||||
) {
|
||||
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;
|
||||
}
|
||||
@@ -1680,6 +1683,7 @@ 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;
|
||||
@@ -1933,10 +1937,8 @@ pub(crate) async fn run_task(
|
||||
}
|
||||
Err(e) => {
|
||||
info!("Turn error: {e:#}");
|
||||
let event = EventMsg::Error(ErrorEvent {
|
||||
message: e.to_string(),
|
||||
});
|
||||
sess.send_event(&turn_context, event).await;
|
||||
sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None)))
|
||||
.await;
|
||||
// let the user continue the conversation
|
||||
break;
|
||||
}
|
||||
@@ -2060,6 +2062,7 @@ async fn run_turn(
|
||||
sess.notify_stream_error(
|
||||
&turn_context,
|
||||
format!("Reconnecting... {retries}/{max_retries}"),
|
||||
e.http_status_code(),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
@@ -10,7 +10,6 @@ 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;
|
||||
@@ -128,10 +127,8 @@ async fn run_compact_task_inner(
|
||||
continue;
|
||||
}
|
||||
sess.set_total_tokens_full(turn_context.as_ref()).await;
|
||||
let event = EventMsg::Error(ErrorEvent {
|
||||
message: e.to_string(),
|
||||
});
|
||||
sess.send_event(&turn_context, event).await;
|
||||
sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None)))
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
Err(e) => {
|
||||
@@ -141,15 +138,14 @@ 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 {
|
||||
let event = EventMsg::Error(ErrorEvent {
|
||||
message: e.to_string(),
|
||||
});
|
||||
sess.send_event(&turn_context, event).await;
|
||||
sess.send_event(&turn_context, EventMsg::Error(e.to_error_event(None)))
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,7 +6,6 @@ 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;
|
||||
@@ -30,10 +29,8 @@ pub(crate) async fn run_remote_compact_task(sess: Arc<Session>, turn_context: Ar
|
||||
|
||||
async fn run_remote_compact_task_inner(sess: &Arc<Session>, turn_context: &Arc<TurnContext>) {
|
||||
if let Err(err) = run_remote_compact_task_inner_impl(sess, turn_context).await {
|
||||
let event = EventMsg::Error(ErrorEvent {
|
||||
message: format!("Error running remote compact task: {err}"),
|
||||
});
|
||||
sess.send_event(turn_context, event).await;
|
||||
let event = err.to_error_event(Some("Error running remote compact task".to_string()));
|
||||
sess.send_event(turn_context, EventMsg::Error(event)).await;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ 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;
|
||||
@@ -430,6 +431,37 @@ impl CodexErr {
|
||||
pub fn downcast_ref<T: std::any::Any>(&self) -> Option<&T> {
|
||||
(self as &dyn std::any::Any).downcast_ref::<T>()
|
||||
}
|
||||
|
||||
pub fn http_status_code(&self) -> Option<StatusCode> {
|
||||
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<String>) -> 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<StatusCode>) -> Option<u16> {
|
||||
http_status_code.as_ref().map(StatusCode::as_u16)
|
||||
}
|
||||
|
||||
pub fn get_error_message_ui(e: &CodexErr) -> String {
|
||||
@@ -775,4 +807,43 @@ 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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
- `EventMsg::Error` – A task stopped with an error (includes an optional `http_status_code` when available)
|
||||
- `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.
|
||||
|
||||
|
||||
@@ -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(_) => {
|
||||
|
||||
@@ -539,6 +539,7 @@ fn error_event_produces_error() {
|
||||
"e1",
|
||||
EventMsg::Error(codex_core::protocol::ErrorEvent {
|
||||
message: "boom".to_string(),
|
||||
http_status_code: Some(500),
|
||||
}),
|
||||
));
|
||||
assert_eq!(
|
||||
@@ -578,6 +579,7 @@ fn stream_error_event_produces_error() {
|
||||
"e1",
|
||||
EventMsg::StreamError(codex_core::protocol::StreamErrorEvent {
|
||||
message: "retrying".to_string(),
|
||||
http_status_code: Some(500),
|
||||
}),
|
||||
));
|
||||
assert_eq!(
|
||||
@@ -596,6 +598,7 @@ fn error_followed_by_task_complete_produces_turn_failed() {
|
||||
"e1",
|
||||
EventMsg::Error(ErrorEvent {
|
||||
message: "boom".to_string(),
|
||||
http_status_code: Some(500),
|
||||
}),
|
||||
);
|
||||
assert_eq!(
|
||||
|
||||
@@ -686,6 +686,8 @@ pub struct ExitedReviewModeEvent {
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct ErrorEvent {
|
||||
pub message: String,
|
||||
#[serde(default)]
|
||||
pub http_status_code: Option<u16>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
@@ -1363,6 +1365,8 @@ pub struct UndoCompletedEvent {
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct StreamErrorEvent {
|
||||
pub message: String,
|
||||
#[serde(default)]
|
||||
pub http_status_code: Option<u16>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
|
||||
@@ -1627,7 +1627,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 {
|
||||
@@ -1670,7 +1670,9 @@ 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);
|
||||
|
||||
@@ -2502,6 +2502,7 @@ fn stream_error_updates_status_indicator() {
|
||||
id: "sub-1".into(),
|
||||
msg: EventMsg::StreamError(StreamErrorEvent {
|
||||
message: msg.to_string(),
|
||||
http_status_code: None,
|
||||
}),
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user