Improve file blob upload diagnostics (#32305)

## Why

Blob upload failures previously surfaced the full signed upload URL and offered limited information for diagnosing transport and service errors.

## What changed

- Add a unique `x-ms-client-request-id` to each blob upload.
- Report transport failures with the upload host, elapsed time, error category, and client request ID, while stripping the signed URL from the underlying error.
- Report unsuccessful responses with Azure request and error IDs, and emit structured warning events with upload metadata and available Azure and Cloudflare identifiers.

## Testing

- Verify successful uploads include the client request ID header.
- Verify response and transport errors expose diagnostic fields without leaking signed URL parameters or response bodies.

GitOrigin-RevId: 9ae2cb2a9aacec4df237b3ee79a6467a6b9107e7
This commit is contained in:
Colin Young
2026-07-11 01:42:01 +00:00
committed by copyberry
parent dfefd8aa8b
commit 08ba14b03d
3 changed files with 174 additions and 7 deletions

1
codex-rs/Cargo.lock generated
View File

@@ -1961,6 +1961,7 @@ dependencies = [
"tracing",
"tungstenite",
"url",
"uuid",
"wiremock",
]

View File

@@ -29,6 +29,7 @@ eventsource-stream = { workspace = true }
regex-lite = { workspace = true }
tokio-util = { workspace = true, features = ["codec", "io"] }
url = { workspace = true }
uuid = { workspace = true }
[dev-dependencies]
anyhow = { workspace = true }

View File

@@ -11,6 +11,7 @@ use reqwest::StatusCode;
use reqwest::header::CONTENT_LENGTH;
use serde::Deserialize;
use tokio::time::Instant;
use uuid::Uuid;
pub const OPENAI_FILE_URI_PREFIX: &str = "sediment://";
pub const OPENAI_FILE_UPLOAD_LIMIT_BYTES: u64 = 512 * 1024 * 1024;
@@ -46,6 +47,27 @@ pub enum OpenAiFileError {
#[source]
source: reqwest::Error,
},
#[error(
"OpenAI file blob upload to {host} failed after {elapsed_ms} ms ({error_kind}, azure_client_request_id={azure_client_request_id}): {source}"
)]
BlobUploadRequest {
host: String,
elapsed_ms: u128,
error_kind: &'static str,
azure_client_request_id: String,
#[source]
source: reqwest::Error,
},
#[error(
"OpenAI file blob upload to {host} failed with status {status} (azure_client_request_id={azure_client_request_id}, azure_request_id={azure_request_id}, azure_error_code={azure_error_code})"
)]
BlobUploadStatus {
host: String,
status: StatusCode,
azure_client_request_id: String,
azure_request_id: String,
azure_error_code: String,
},
#[error("OpenAI file request to {url} failed with status {status}: {body}")]
UnexpectedStatus {
url: String,
@@ -139,25 +161,80 @@ pub async fn upload_openai_file(
source,
})?;
let upload_host = url::Url::parse(&create_payload.upload_url)
.ok()
.and_then(|url| url.host_str().map(str::to_owned))
.unwrap_or_else(|| "unknown-host".to_string());
let azure_client_request_id = Uuid::new_v4().to_string();
let upload_started_at = Instant::now();
let upload_response = build_reqwest_client(http_client_factory, &create_payload.upload_url)?
.put(&create_payload.upload_url)
.timeout(OPENAI_FILE_REQUEST_TIMEOUT)
.header("x-ms-blob-type", "BlockBlob")
.header("x-ms-client-request-id", &azure_client_request_id)
.header(CONTENT_LENGTH, file_size_bytes)
.body(reqwest::Body::wrap_stream(contents))
.send()
.await
.map_err(|source| OpenAiFileError::Request {
url: create_payload.upload_url.clone(),
source,
.map_err(|source| {
let elapsed_ms = upload_started_at.elapsed().as_millis();
let error_kind = if source.is_timeout() {
"timeout"
} else if source.is_connect() {
"connect"
} else if source.is_body() {
"body"
} else if source.is_request() {
"request"
} else {
"other"
};
tracing::event!(
target: "codex_otel.log_only",
tracing::Level::WARN,
event.name = "codex.openai_file_blob_upload_failed",
file_id = %create_payload.file_id,
host = %upload_host,
file_size_bytes,
elapsed_ms,
error_kind,
azure_client_request_id,
"OpenAI file blob upload transport failed"
);
OpenAiFileError::BlobUploadRequest {
host: upload_host.clone(),
elapsed_ms,
error_kind,
azure_client_request_id: azure_client_request_id.clone(),
source: source.without_url(),
}
})?;
let upload_status = upload_response.status();
let upload_body = upload_response.text().await.unwrap_or_default();
let cloudflare_ray_id = upload_response_header(&upload_response, "cf-ray");
let azure_request_id = upload_response_header(&upload_response, "x-ms-request-id");
let azure_error_code = upload_response_header(&upload_response, "x-ms-error-code");
if !upload_status.is_success() {
return Err(OpenAiFileError::UnexpectedStatus {
url: create_payload.upload_url.clone(),
tracing::event!(
target: "codex_otel.log_only",
tracing::Level::WARN,
event.name = "codex.openai_file_blob_upload_failed",
file_id = %create_payload.file_id,
host = %upload_host,
file_size_bytes,
elapsed_ms = upload_started_at.elapsed().as_millis(),
status = %upload_status,
cloudflare_ray_id,
azure_client_request_id,
azure_request_id,
azure_error_code,
"OpenAI file blob upload failed"
);
return Err(OpenAiFileError::BlobUploadStatus {
host: upload_host,
status: upload_status,
body: upload_body,
azure_client_request_id,
azure_request_id,
azure_error_code,
});
}
@@ -274,6 +351,15 @@ fn build_reqwest_client(
}
}
fn upload_response_header(response: &reqwest::Response, header: &str) -> String {
response
.headers()
.get(header)
.and_then(|value| value.to_str().ok())
.unwrap_or("missing")
.to_string()
}
#[cfg(test)]
mod tests {
use super::*;
@@ -288,6 +374,7 @@ mod tests {
use wiremock::ResponseTemplate;
use wiremock::matchers::body_json;
use wiremock::matchers::header;
use wiremock::matchers::header_regex;
use wiremock::matchers::method;
use wiremock::matchers::path;
@@ -336,6 +423,7 @@ mod tests {
Mock::given(method("PUT"))
.and(path("/upload/file_123"))
.and(header("content-length", "5"))
.and(header_regex("x-ms-client-request-id", "^[0-9a-f-]{36}$"))
.respond_with(ResponseTemplate::new(200))
.mount(&server)
.await;
@@ -386,4 +474,81 @@ mod tests {
assert_eq!(uploaded.mime_type, Some("text/plain".to_string()));
assert_eq!(finalize_attempts.load(Ordering::SeqCst), 2);
}
#[tokio::test]
async fn upload_openai_file_reports_blob_response_diagnostics_without_sas() {
let server = MockServer::start().await;
Mock::given(method("POST"))
.and(path("/backend-api/files"))
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
"file_id": "file_123",
"upload_url": format!("{}/upload/file_123?sig=secret", server.uri()),
})))
.mount(&server)
.await;
Mock::given(method("PUT"))
.and(path("/upload/file_123"))
.respond_with(
ResponseTemplate::new(500)
.insert_header("x-ms-request-id", "azure-request")
.insert_header("x-ms-error-code", "ServerBusy")
.set_body_string("try again"),
)
.mount(&server)
.await;
let error = upload_openai_file(
&base_url_for(&server),
&chatgpt_auth(),
&default_http_client_factory(),
"hello.txt".to_string(),
/*file_size_bytes*/ 5,
futures::stream::iter([Ok::<_, std::io::Error>(Bytes::from_static(b"hello"))]),
)
.await
.expect_err("blob response failure should be returned");
let message = error.to_string();
assert!(message.contains("failed with status 500"));
assert!(message.contains("azure_client_request_id="));
assert!(message.contains("azure_request_id=azure-request"));
assert!(message.contains("azure_error_code=ServerBusy"));
assert!(!message.contains("try again"));
assert!(!message.contains("sig=secret"));
}
#[tokio::test]
async fn upload_openai_file_reports_blob_transport_diagnostics_without_sas() {
let upload_listener = tokio::net::TcpListener::bind("127.0.0.1:0")
.await
.expect("bind upload address");
let upload_address = upload_listener.local_addr().expect("upload address");
drop(upload_listener);
let server = MockServer::start().await;
Mock::given(method("POST"))
.and(path("/backend-api/files"))
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
"file_id": "file_123",
"upload_url": format!("http://{upload_address}/upload?sig=secret"),
})))
.mount(&server)
.await;
let error = upload_openai_file(
&base_url_for(&server),
&chatgpt_auth(),
&default_http_client_factory(),
"hello.txt".to_string(),
/*file_size_bytes*/ 5,
futures::stream::iter([Ok::<_, std::io::Error>(Bytes::from_static(b"hello"))]),
)
.await
.expect_err("blob transport failure should be returned");
let message = error.to_string();
assert!(message.contains("failed after"));
assert!(message.contains("(connect,"), "{message}");
assert!(message.contains("azure_client_request_id="));
assert!(!message.contains("sig=secret"));
}
}