Clean up Rust error handling and OTLP test assertions (#61201)

## What changed

- Store `CodexErr` in a `Box` within `ImageBackendError` while preserving its accessor.
- Terminate `bail!` calls with semicolons in the cloud client and TUI.
- Share a body-prefix length constant across OTLP loopback assertions and remove redundant borrows of diagnostic strings.

GitOrigin-RevId: 40aee7d4d03b17fc8dbd9295e0853c811852c177
This commit is contained in:
ioanbudea-OAI
2026-11-12 23:08:59 +00:00
committed by copyberry
parent 16e27626e6
commit 1f175fa3c4
9 changed files with 47 additions and 40 deletions
@@ -147,9 +147,11 @@ impl Replies {
// A server can close Attach immediately after returning successful headers.
// A healthy idle stream needs no event to prove that it is ready.
let first_event = match tokio::task::unconstrained(stream.next()).now_or_never() {
Some(None | Some(Err(_))) => bail!(
"Cloud conversation activated but its stream did not connect. Reopen it before sending."
),
Some(None | Some(Err(_))) => {
bail!(
"Cloud conversation activated but its stream did not connect. Reopen it before sending."
);
}
Some(Some(Ok(event))) => Some(event),
None => None,
};
@@ -172,7 +172,7 @@ impl Requests {
Some(protocol::ThreadItemsListCursor::Opaque(cursor)) => Some(cursor),
None => None,
Some(protocol::ThreadItemsListCursor::Anchor(_)) => {
bail!("Cloud item anchors are not supported yet.")
bail!("Cloud item anchors are not supported yet.");
}
};
let page = self
@@ -305,7 +305,7 @@ fn thread_list_query(params: protocol::ThreadListParams) -> Result<cloud::Thread
Some(protocol::ThreadSortKey::UpdatedAt) => cloud::ThreadSortKey::UpdatedAt,
Some(protocol::ThreadSortKey::RecencyAt) => cloud::ThreadSortKey::RecencyAt,
Some(protocol::ThreadSortKey::SectionPosition) => {
bail!("Cloud section ordering is not supported yet.")
bail!("Cloud section ordering is not supported yet.");
}
},
sort_direction: match params.sort_direction {
@@ -440,18 +440,22 @@ fn admitted_message(outcome: cloud::AdmissionOutcome) -> Result<CloudUserMessage
cloud::AdmissionOutcome::Duplicate { turn_id } => {
CloudUserMessageReceipt::Duplicate { turn_id }
}
cloud::AdmissionOutcome::Queued { .. } => bail!(
"Cloud already accepted this message without returning a turn. Reopen the conversation to see its status before sending again."
),
cloud::AdmissionOutcome::Queued { .. } => {
bail!(
"Cloud already accepted this message without returning a turn. Reopen the conversation to see its status before sending again."
);
}
})
}
fn admitted_turn(outcome: cloud::AdmissionOutcome) -> Result<protocol::Turn> {
match admitted_message(outcome)? {
CloudUserMessageReceipt::Started(turn) | CloudUserMessageReceipt::Steered(turn) => Ok(turn),
CloudUserMessageReceipt::Duplicate { .. } => bail!(
"Cloud already accepted this message without returning a turn. Reopen the conversation to see its status before sending again."
),
CloudUserMessageReceipt::Duplicate { .. } => {
bail!(
"Cloud already accepted this message without returning a turn. Reopen the conversation to see its status before sending again."
);
}
}
}
+3 -3
View File
@@ -19,7 +19,7 @@ const X_CODEX_IMAGE_TURN_ID_HEADER: &str = "x-codex-image-turn-id";
pub(crate) struct ImageBackendError {
message: String,
codex_error: CodexErr,
codex_error: Box<CodexErr>,
imagegen_request_id: Option<String>,
}
@@ -29,14 +29,14 @@ impl ImageBackendError {
let message = error.to_string();
Self {
message,
codex_error: map_api_error(error),
codex_error: Box::new(map_api_error(error)),
imagegen_request_id,
}
}
fn from_message(message: String) -> Self {
Self {
codex_error: CodexErr::Stream(message.clone()),
codex_error: Box::new(CodexErr::Stream(message.clone())),
message,
imagegen_request_id: None,
}
+22 -21
View File
@@ -26,6 +26,7 @@ use std::time::Duration;
use std::time::Instant;
use tracing_subscriber::layer::SubscriberExt;
const MAX_BODY_PREFIX_CHARS: usize = 2000;
static TRACE_CONTEXT_CONFIG_LOCK: Mutex<()> = Mutex::new(());
#[path = "otel_headers_helper.rs"]
@@ -299,17 +300,17 @@ fn otlp_http_exporter_honors_cumulative_temporality_env() -> Result<()> {
assert!(
body.contains("codex.turns"),
"expected metric name not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("codex.active"),
"expected gauge not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("\"codex.api_request\""),
"expected API-request counter not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains(EXEC_SERVER_CLIENT_REQUEST_COUNT_METRIC),
@@ -318,47 +319,47 @@ fn otlp_http_exporter_honors_cumulative_temporality_env() -> Result<()> {
assert!(
body.contains("\"codex.api_request.duration_ms\""),
"expected API-request duration not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("\"codex.conversation.turn.count\""),
"expected conversation turn count not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("\"codex.responses_api_engine_iapi_ttft.duration_ms\""),
"expected engine IAPI TTFT duration not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("\"codex.responses_api_engine_service_tbt.duration_ms\""),
"expected engine service TBT duration not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("\"codex.responses_api_engine_service_ttft.duration_ms\""),
"expected engine service TTFT duration not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("\"codex.tool.call\""),
"expected tool-call counter not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("\"codex.turn.token_usage\""),
"expected turn-token histogram not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("\"codex.tool.call.duration_ms\""),
"expected tool-call duration not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("component") && body.contains("test"),
"expected gauge tag not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
Ok(())
@@ -453,7 +454,7 @@ fn otlp_http_exporter_sends_logs_to_collector()
assert!(
body.contains("codex.test.log_exported"),
"expected exported log event not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
Ok(())
}
@@ -615,22 +616,22 @@ fn otlp_http_exporter_sends_traces_to_collector()
assert!(
body.contains("trace-loopback"),
"expected span name not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("codex-cli"),
"expected service name not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("test.configured_attribute") && body.contains("configured-value"),
"expected configured span attribute not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("codex.test.trace_event"),
"expected trace event not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
Ok(())
@@ -740,12 +741,12 @@ async fn otlp_http_exporter_sends_traces_to_collector_with_bounded_shutdown_in_t
assert!(
body.contains("trace-loopback-tokio"),
"expected span name not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("codex-cli"),
"expected service name not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
Ok(())
@@ -944,12 +945,12 @@ fn otlp_http_exporter_sends_traces_to_collector_in_current_thread_tokio_runtime(
assert!(
body.contains("trace-loopback-current-thread"),
"expected span name not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
assert!(
body.contains("codex-cli"),
"expected service name not found; body prefix: {}",
&body.chars().take(2000).collect::<String>()
body.chars().take(MAX_BODY_PREFIX_CHARS).collect::<String>()
);
Ok(())
+1 -1
View File
@@ -138,7 +138,7 @@ pub(super) async fn reconnect(
}
}
tracing::warn!(stage, "automatic reconnect timed out");
color_eyre::eyre::bail!("Automatic reconnect timed out while {stage}.")
color_eyre::eyre::bail!("Automatic reconnect timed out while {stage}.");
}
impl App {
@@ -1222,7 +1222,7 @@ async fn command_center_new_checkout_and_worktree_preserve_source_and_default_br
return Ok::<_, color_eyre::Report>(());
}
}
color_eyre::eyre::bail!("permissions menu event stream closed")
color_eyre::eyre::bail!("permissions menu event stream closed");
})
.await??;
app.chat_widget.handle_key_event(KeyCode::Up.into());
@@ -194,7 +194,7 @@ async fn run_verification_rpc_scenario(scenario: RpcScenario) -> color_eyre::Res
}
}
}
color_eyre::eyre::bail!("connection closed before the elicitation response")
color_eyre::eyre::bail!("connection closed before the elicitation response");
});
let client = crate::connect_remote_app_server(endpoint).await?;
let mode = if scenario == RpcScenario::RemoteWorkspace {
+1 -1
View File
@@ -680,7 +680,7 @@ pub(crate) fn backtrack_revert_before_turn_id(
}
}
bail!("the selected prompt was not found in the persisted thread")
bail!("the selected prompt was not found in the persisted thread");
}
/// Returns whether a turn is the reconstructed inline-review child with duplicated prompt inputs.
+1 -1
View File
@@ -20,7 +20,7 @@ pub(crate) async fn connect(target: &AppServerTarget) -> color_eyre::Result<AppS
color_eyre::eyre::bail!("Reopen Codex to reconnect the cloud backend.");
}
AppServerTarget::Embedded => {
color_eyre::eyre::bail!("embedded sessions have no remote connection")
color_eyre::eyre::bail!("embedded sessions have no remote connection");
}
#[cfg(windows)]
AppServerTarget::LocalDaemon {