mirror of
https://github.com/openai/codex.git
synced 2026-11-05 08:28:29 +00:00
chore: improve expired Bedrock credential errors (#28992)
## Why Amazon Bedrock returns a `401 Unauthorized` response containing `Signature expired:` when an AWS credential, including a short-lived `AWS_BEARER_TOKEN_BEDROCK`, has expired. Codex currently surfaces that response as a generic `unexpected status` error, which does not explain how to recover. Environment-provided bearer tokens cannot be refreshed automatically, so the error should direct users to refresh their AWS credentials or replace or remove the environment token and restart Codex. This classification belongs to the Amazon Bedrock provider so similar responses from other providers retain their existing behavior. ## What changed - Add a synchronous `ModelProvider::map_api_error` hook that defaults to the existing provider-neutral API error mapping, and route model request, stream, WebSocket, and terminal unauthorized errors through the active provider. - Override the hook for Amazon Bedrock. After preserving the structured status, body, URL, and request metadata, recognize `401` responses containing `Signature expired:` and attach actionable credential guidance. - Keep `codex-protocol` provider-neutral by representing the guidance as an optional `user_message`. Error rendering prefers this message while continuing to append the URL, request ID, Cloudflare ray, and authorization diagnostics. - Add model-provider coverage for expired signatures and negative cases, core coverage for provider dispatch after unauthorized recovery, and a TUI snapshot for the rendered error. ## Testing Tested with a real request with expired bedrock key: <img width="962" height="126" alt="Screenshot 2026-06-22 at 3 56 51 PM" src="https://github.com/user-attachments/assets/7e21cc7c-798e-4662-8467-7f304a2f2b59" />
This commit is contained in:
1 parent
b294638bb5
commit
e65e480e0d
12 files changed
+298
-82
No files matched your search
+17
-10
@@ -112,7 +112,6 @@ use crate::feedback_tags;
|
||||
use crate::responses_metadata::CodexResponsesMetadata;
|
||||
use crate::responses_metadata::subagent_header_value;
|
||||
use crate::util::emit_feedback_auth_recovery_tags;
|
||||
use codex_api::map_api_error;
|
||||
use codex_feedback::FeedbackRequestTags;
|
||||
use codex_feedback::emit_feedback_request_tags_with_auth_env;
|
||||
use codex_login::auth_env_telemetry::AuthEnvTelemetry;
|
||||
@@ -577,7 +576,7 @@ impl ModelClient {
|
||||
turn_state.as_deref(),
|
||||
)
|
||||
.await
|
||||
.map_err(map_api_error);
|
||||
.map_err(|error| self.state.provider.map_api_error(error));
|
||||
trace_attempt.record_result(result.as_deref());
|
||||
result
|
||||
}
|
||||
@@ -604,7 +603,7 @@ impl ModelClient {
|
||||
let response = ApiRealtimeCallClient::new(transport, api_provider, client_setup.api_auth)
|
||||
.create_with_session_and_headers(sdp, session_config, extra_headers)
|
||||
.await
|
||||
.map_err(map_api_error)?;
|
||||
.map_err(|error| self.state.provider.map_api_error(error))?;
|
||||
Ok(RealtimeWebrtcCallStart {
|
||||
sdp: response.sdp,
|
||||
call_id: response.call_id,
|
||||
@@ -658,7 +657,7 @@ impl ModelClient {
|
||||
client
|
||||
.summarize_input(&payload, self.build_subagent_headers())
|
||||
.await
|
||||
.map_err(map_api_error)
|
||||
.map_err(|error| self.state.provider.map_api_error(error))
|
||||
}
|
||||
|
||||
fn build_subagent_headers(&self) -> ApiHeaderMap {
|
||||
@@ -1337,6 +1336,7 @@ impl ModelClientSession {
|
||||
stream,
|
||||
session_telemetry.clone(),
|
||||
inference_trace_attempt,
|
||||
Arc::clone(&self.client.state.provider),
|
||||
);
|
||||
return Ok(stream);
|
||||
}
|
||||
@@ -1355,6 +1355,7 @@ impl ModelClientSession {
|
||||
unauthorized_transport,
|
||||
&mut auth_recovery,
|
||||
session_telemetry,
|
||||
&self.client.state.provider,
|
||||
)
|
||||
.await?,
|
||||
);
|
||||
@@ -1363,7 +1364,7 @@ impl ModelClientSession {
|
||||
Err(err) => {
|
||||
let response_debug_context =
|
||||
extract_response_debug_context_from_api_error(&err);
|
||||
let err = map_api_error(err);
|
||||
let err = self.client.state.provider.map_api_error(err);
|
||||
inference_trace_attempt.record_failed(
|
||||
&err,
|
||||
response_debug_context.request_id.as_deref(),
|
||||
@@ -1469,12 +1470,13 @@ impl ModelClientSession {
|
||||
unauthorized_transport,
|
||||
&mut auth_recovery,
|
||||
session_telemetry,
|
||||
&self.client.state.provider,
|
||||
)
|
||||
.await?,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
Err(err) => return Err(map_api_error(err)),
|
||||
Err(err) => return Err(self.client.state.provider.map_api_error(err)),
|
||||
}
|
||||
|
||||
let (mut ws_request, previous_response_id_from_untraced_warmup) =
|
||||
@@ -1503,7 +1505,7 @@ impl ModelClientSession {
|
||||
self.websocket_session.last_response_from_untraced_warmup = warmup;
|
||||
let websocket_connection =
|
||||
self.websocket_session.connection.as_ref().ok_or_else(|| {
|
||||
map_api_error(ApiError::Stream(
|
||||
self.client.state.provider.map_api_error(ApiError::Stream(
|
||||
"websocket connection is unavailable".to_string(),
|
||||
))
|
||||
})?;
|
||||
@@ -1517,7 +1519,7 @@ impl ModelClientSession {
|
||||
.map_err(|err| {
|
||||
let response_debug_context =
|
||||
extract_response_debug_context_from_api_error(&err);
|
||||
let err = map_api_error(err);
|
||||
let err = self.client.state.provider.map_api_error(err);
|
||||
inference_trace_attempt.record_failed(
|
||||
&err,
|
||||
response_debug_context.request_id.as_deref(),
|
||||
@@ -1529,6 +1531,7 @@ impl ModelClientSession {
|
||||
stream_result,
|
||||
session_telemetry.clone(),
|
||||
inference_trace_attempt,
|
||||
Arc::clone(&self.client.state.provider),
|
||||
);
|
||||
self.websocket_session.last_response_rx = Some(last_request_rx);
|
||||
return Ok(WebsocketStreamOutcome::Stream(stream));
|
||||
@@ -1761,6 +1764,7 @@ fn map_response_stream(
|
||||
api_stream: codex_api::ResponseStream,
|
||||
session_telemetry: SessionTelemetry,
|
||||
inference_trace_attempt: InferenceTraceAttempt,
|
||||
provider: SharedModelProvider,
|
||||
) -> (ResponseStream, oneshot::Receiver<LastResponse>) {
|
||||
let codex_api::ResponseStream {
|
||||
rx_event,
|
||||
@@ -1775,6 +1779,7 @@ fn map_response_stream(
|
||||
api_stream,
|
||||
session_telemetry,
|
||||
inference_trace_attempt,
|
||||
provider,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1783,6 +1788,7 @@ fn map_response_events<S>(
|
||||
api_stream: S,
|
||||
session_telemetry: SessionTelemetry,
|
||||
inference_trace_attempt: InferenceTraceAttempt,
|
||||
provider: SharedModelProvider,
|
||||
) -> (ResponseStream, oneshot::Receiver<LastResponse>)
|
||||
where
|
||||
S: futures::Stream<Item = std::result::Result<ResponseEvent, ApiError>>
|
||||
@@ -1893,7 +1899,7 @@ where
|
||||
if let Some(upstream_request_id) = upstream_request_id {
|
||||
feedback_tags!(last_model_request_id = upstream_request_id);
|
||||
}
|
||||
let mapped = map_api_error(err);
|
||||
let mapped = provider.map_api_error(err);
|
||||
inference_trace_attempt.record_failed(
|
||||
&mapped,
|
||||
upstream_request_id,
|
||||
@@ -1999,6 +2005,7 @@ async fn handle_unauthorized(
|
||||
transport: TransportError,
|
||||
auth_recovery: &mut Option<UnauthorizedRecovery>,
|
||||
session_telemetry: &SessionTelemetry,
|
||||
provider: &SharedModelProvider,
|
||||
) -> Result<UnauthorizedRecoveryExecution> {
|
||||
let debug = extract_response_debug_context(&transport);
|
||||
if let Some(recovery) = auth_recovery
|
||||
@@ -2108,7 +2115,7 @@ async fn handle_unauthorized(
|
||||
debug.auth_error_code.as_deref(),
|
||||
);
|
||||
|
||||
Err(map_api_error(ApiError::Transport(transport)))
|
||||
Err(provider.map_api_error(ApiError::Transport(transport)))
|
||||
}
|
||||
|
||||
fn api_error_http_status(error: &ApiError) -> Option<u16> {
|
||||
|
||||
Reference in new issue
Block a user