mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Preserve image generation request IDs on failure (#47327)
## Why Failed image requests discarded the `x-codex-imagegen-request-id` header, leaving failure analytics without the request ID. ## What changed Carry the request ID through `ImageRequestError` and backend errors for image generation and editing. Preserve it on failed image generation items for HTTP errors, response decoding errors, and responses with no image data. ## Testing Extend the app-server failure test to verify that a request ID returned with an HTTP 500 response reaches `codex_image_generation_event` analytics. GitOrigin-RevId: 57cddacc0221e5b58877ac375d5eca0afb99d853
This commit is contained in:
@@ -396,7 +396,11 @@ async fn standalone_image_generation_failure_emits_terminal_item() -> Result<()>
|
||||
let server = responses::start_mock_server().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/api/codex/images/generations"))
|
||||
.respond_with(ResponseTemplate::new(500).set_body_string("image backend failed"))
|
||||
.respond_with(
|
||||
ResponseTemplate::new(500)
|
||||
.insert_header("x-codex-imagegen-request-id", "req-imagegen-failed-123")
|
||||
.set_body_string("image backend failed"),
|
||||
)
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
@@ -472,6 +476,17 @@ async fn standalone_image_generation_failure_emits_terminal_item() -> Result<()>
|
||||
)
|
||||
);
|
||||
|
||||
let event = wait_for_analytics_event(
|
||||
&server,
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
"codex_image_generation_event",
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
event["event_params"]["imagegen_request_id"],
|
||||
json!("req-imagegen-failed-123")
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ use crate::images::ImageResponse;
|
||||
use crate::provider::Provider;
|
||||
use codex_client::HttpTransport;
|
||||
use codex_client::RequestTelemetry;
|
||||
use codex_client::TransportError;
|
||||
use http::HeaderMap;
|
||||
use http::Method;
|
||||
use serde::Serialize;
|
||||
@@ -19,6 +20,32 @@ pub struct ImagesClient<T: HttpTransport> {
|
||||
session: EndpointSession<T>,
|
||||
}
|
||||
|
||||
/// Image request failure with the nested ImageGen request ID, when available.
|
||||
#[derive(Debug)]
|
||||
pub struct ImageRequestError {
|
||||
error: ApiError,
|
||||
imagegen_request_id: Option<String>,
|
||||
}
|
||||
|
||||
impl ImageRequestError {
|
||||
fn from_api_error(error: ApiError) -> Self {
|
||||
let imagegen_request_id = match &error {
|
||||
ApiError::Transport(TransportError::Http { headers, .. }) => {
|
||||
headers.as_ref().and_then(imagegen_request_id_from_headers)
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
Self {
|
||||
error,
|
||||
imagegen_request_id,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn into_parts(self) -> (ApiError, Option<String>) {
|
||||
(self.error, self.imagegen_request_id)
|
||||
}
|
||||
}
|
||||
|
||||
impl<T: HttpTransport> ImagesClient<T> {
|
||||
pub fn new(transport: T, provider: Provider, auth: SharedAuthProvider) -> Self {
|
||||
Self {
|
||||
@@ -36,7 +63,7 @@ impl<T: HttpTransport> ImagesClient<T> {
|
||||
&self,
|
||||
request: &ImageGenerationRequest,
|
||||
extra_headers: HeaderMap,
|
||||
) -> Result<(ImageResponse, Option<String>), ApiError> {
|
||||
) -> Result<(ImageResponse, Option<String>), ImageRequestError> {
|
||||
self.post_image_request(
|
||||
"images/generations",
|
||||
request,
|
||||
@@ -50,7 +77,7 @@ impl<T: HttpTransport> ImagesClient<T> {
|
||||
&self,
|
||||
request: &ImageEditRequest,
|
||||
extra_headers: HeaderMap,
|
||||
) -> Result<(ImageResponse, Option<String>), ApiError> {
|
||||
) -> Result<(ImageResponse, Option<String>), ImageRequestError> {
|
||||
self.post_image_request("images/edits", request, extra_headers, "image edit")
|
||||
.await
|
||||
}
|
||||
@@ -61,25 +88,33 @@ impl<T: HttpTransport> ImagesClient<T> {
|
||||
request: &R,
|
||||
extra_headers: HeaderMap,
|
||||
operation: &str,
|
||||
) -> Result<(ImageResponse, Option<String>), ApiError> {
|
||||
let body = to_value(request)
|
||||
.map_err(|e| ApiError::Stream(format!("failed to encode {operation} request: {e}")))?;
|
||||
) -> Result<(ImageResponse, Option<String>), ImageRequestError> {
|
||||
let body = to_value(request).map_err(|e| ImageRequestError {
|
||||
error: ApiError::Stream(format!("failed to encode {operation} request: {e}")),
|
||||
imagegen_request_id: None,
|
||||
})?;
|
||||
let resp = self
|
||||
.session
|
||||
.execute(Method::POST, path, extra_headers, Some(body))
|
||||
.await?;
|
||||
let imagegen_request_id = resp
|
||||
.headers
|
||||
.get(X_CODEX_IMAGEGEN_REQUEST_ID_HEADER)
|
||||
.and_then(|value| value.to_str().ok())
|
||||
.filter(|request_id| !request_id.is_empty())
|
||||
.map(str::to_string);
|
||||
let response = serde_json::from_slice(&resp.body)
|
||||
.map_err(|e| ApiError::Stream(format!("failed to decode {operation} response: {e}")))?;
|
||||
.await
|
||||
.map_err(ImageRequestError::from_api_error)?;
|
||||
let imagegen_request_id = imagegen_request_id_from_headers(&resp.headers);
|
||||
let response = serde_json::from_slice(&resp.body).map_err(|e| ImageRequestError {
|
||||
error: ApiError::Stream(format!("failed to decode {operation} response: {e}")),
|
||||
imagegen_request_id: imagegen_request_id.clone(),
|
||||
})?;
|
||||
Ok((response, imagegen_request_id))
|
||||
}
|
||||
}
|
||||
|
||||
fn imagegen_request_id_from_headers(headers: &HeaderMap) -> Option<String> {
|
||||
headers
|
||||
.get(X_CODEX_IMAGEGEN_REQUEST_ID_HEADER)
|
||||
.and_then(|value| value.to_str().ok())
|
||||
.filter(|request_id| !request_id.is_empty())
|
||||
.map(str::to_string)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -355,6 +390,7 @@ mod tests {
|
||||
)
|
||||
.await
|
||||
.expect_err("image response without data should fail");
|
||||
let (error, _) = error.into_parts();
|
||||
|
||||
let ApiError::Stream(message) = error else {
|
||||
panic!("expected image response decode error");
|
||||
|
||||
@@ -8,6 +8,7 @@ pub(crate) mod responses_websocket;
|
||||
pub(crate) mod search;
|
||||
mod session;
|
||||
|
||||
pub use images::ImageRequestError;
|
||||
pub use images::ImagesClient;
|
||||
pub use memories::MemoriesClient;
|
||||
pub use models::ModelsClient;
|
||||
|
||||
@@ -49,6 +49,7 @@ pub use crate::common::WS_REQUEST_HEADER_TRACEPARENT_CLIENT_METADATA_KEY;
|
||||
pub use crate::common::WS_REQUEST_HEADER_TRACESTATE_CLIENT_METADATA_KEY;
|
||||
pub use crate::common::create_text_param_for_request;
|
||||
pub use crate::common::response_create_client_metadata;
|
||||
pub use crate::endpoint::ImageRequestError;
|
||||
pub use crate::endpoint::ImagesClient;
|
||||
pub use crate::endpoint::MemoriesClient;
|
||||
pub use crate::endpoint::ModelsClient;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use codex_api::ImageEditRequest;
|
||||
use codex_api::ImageGenerationRequest;
|
||||
use codex_api::ImageRequestError;
|
||||
use codex_api::ImageResponse;
|
||||
use codex_api::ImagesClient;
|
||||
use codex_api::ReqwestTransport;
|
||||
@@ -16,14 +17,17 @@ const X_CODEX_IMAGE_TURN_ID_HEADER: &str = "x-codex-image-turn-id";
|
||||
pub(crate) struct ImageBackendError {
|
||||
message: String,
|
||||
codex_error: CodexErr,
|
||||
imagegen_request_id: Option<String>,
|
||||
}
|
||||
|
||||
impl ImageBackendError {
|
||||
fn from_api(error: codex_api::ApiError) -> Self {
|
||||
fn from_image_request(error: ImageRequestError) -> Self {
|
||||
let (error, imagegen_request_id) = error.into_parts();
|
||||
let message = error.to_string();
|
||||
Self {
|
||||
message,
|
||||
codex_error: map_api_error(error),
|
||||
imagegen_request_id,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,6 +35,7 @@ impl ImageBackendError {
|
||||
Self {
|
||||
codex_error: CodexErr::Stream(message.clone()),
|
||||
message,
|
||||
imagegen_request_id: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -41,6 +46,10 @@ impl ImageBackendError {
|
||||
pub(crate) fn codex_error(&self) -> &CodexErr {
|
||||
&self.codex_error
|
||||
}
|
||||
|
||||
pub(crate) fn imagegen_request_id(&self) -> Option<&str> {
|
||||
self.imagegen_request_id.as_deref()
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
@@ -90,7 +99,7 @@ impl CodexImagesBackend {
|
||||
image_request_headers(self.originator.as_deref(), turn_id),
|
||||
)
|
||||
.await
|
||||
.map_err(ImageBackendError::from_api)
|
||||
.map_err(ImageBackendError::from_image_request)
|
||||
}
|
||||
|
||||
/// Sends a standalone image edit request through the configured Images client.
|
||||
@@ -106,7 +115,7 @@ impl CodexImagesBackend {
|
||||
image_request_headers(self.originator.as_deref(), turn_id),
|
||||
)
|
||||
.await
|
||||
.map_err(ImageBackendError::from_api)
|
||||
.map_err(ImageBackendError::from_image_request)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -174,6 +174,7 @@ impl ImageGenerationTool {
|
||||
(
|
||||
format!("image generation failed: {}", error.message()),
|
||||
usage_limit_failure(error.codex_error()),
|
||||
error.imagegen_request_id().map(str::to_string),
|
||||
)
|
||||
})
|
||||
.and_then(|(response, imagegen_request_id)| {
|
||||
@@ -182,23 +183,23 @@ impl ImageGenerationTool {
|
||||
Some(ImageBackground::Opaque) => Some(false),
|
||||
Some(ImageBackground::Auto) | None => None,
|
||||
};
|
||||
response
|
||||
.data
|
||||
.into_iter()
|
||||
.next()
|
||||
.map(|data| {
|
||||
(
|
||||
data.b64_json,
|
||||
transparent_background,
|
||||
imagegen_request_id,
|
||||
data.generation_id,
|
||||
)
|
||||
})
|
||||
.ok_or_else(|| ("image generation returned no image data".to_string(), None))
|
||||
match response.data.into_iter().next() {
|
||||
Some(data) => Ok((
|
||||
data.b64_json,
|
||||
transparent_background,
|
||||
imagegen_request_id,
|
||||
data.generation_id,
|
||||
)),
|
||||
None => Err((
|
||||
"image generation returned no image data".to_string(),
|
||||
None,
|
||||
imagegen_request_id,
|
||||
)),
|
||||
}
|
||||
});
|
||||
let (result, transparent_background, imagegen_request_id, generation_id) = match result {
|
||||
Ok(result) => result,
|
||||
Err((message, failure)) => {
|
||||
Err((message, failure, imagegen_request_id)) => {
|
||||
let item = ImageGenerationItem {
|
||||
id: call.call_id.clone(),
|
||||
status: "failed".to_string(),
|
||||
@@ -207,7 +208,7 @@ impl ImageGenerationTool {
|
||||
transparent_background: None,
|
||||
failure,
|
||||
saved_path: None,
|
||||
imagegen_request_id: None,
|
||||
imagegen_request_id,
|
||||
generation_id: None,
|
||||
};
|
||||
let legacy_event = legacy_end_event(&item);
|
||||
|
||||
Reference in New Issue
Block a user