mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
Distinguish retryable thread-store errors from unknown write outcomes (#54664)
## What changed Add `ThreadStoreError::Retryable` for transient failures known to be safe to retry and `ThreadStoreError::OutcomeUnknown` for writes that may already have committed. Recovery remains the operation owner's responsibility; neither category automatically triggers retries. Preserve existing app-server error mappings and fatal session startup behavior, and report the new categories in shutdown diagnostics without exposing raw error payloads. ## Testing Add coverage for conservative SQLx error classification, mutation error mappings, session initialization categories, and shutdown diagnostics that omit raw error payloads. GitOrigin-RevId: 259a9bfaa6ff3cbfa2ee61a01a1f1e744c90f118
This commit is contained in:
@@ -16,7 +16,11 @@ pub(crate) fn reject_paginated_history_mode(
|
||||
}
|
||||
|
||||
/// Error type shared by thread-store implementations.
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
///
|
||||
/// Implementations must explicitly distinguish known transient failures from writes whose
|
||||
/// outcome is unknown. These categories carry information only: the operation owner decides
|
||||
/// whether and how to recover, and no variant automatically triggers a retry.
|
||||
#[derive(Clone, Debug, thiserror::Error)]
|
||||
pub enum ThreadStoreError {
|
||||
/// The requested thread does not exist in this store.
|
||||
#[error("thread {thread_id} not found")]
|
||||
@@ -32,7 +36,7 @@ pub enum ThreadStoreError {
|
||||
message: String,
|
||||
},
|
||||
|
||||
/// The operation conflicted with current store state.
|
||||
/// The operation conflicted with current store state. This alone does not imply retryability.
|
||||
#[error("thread-store conflict: {message}")]
|
||||
Conflict {
|
||||
/// User-facing explanation of the conflict.
|
||||
@@ -46,10 +50,37 @@ pub enum ThreadStoreError {
|
||||
operation: &'static str,
|
||||
},
|
||||
|
||||
/// A transient failure for which retrying the operation is safe.
|
||||
///
|
||||
/// For a write, the implementation must know the failed attempt did not commit. Otherwise
|
||||
/// use [`Self::OutcomeUnknown`]. Retry scheduling and limits belong to the operation owner.
|
||||
#[error("thread-store retryable error: {message}")]
|
||||
Retryable {
|
||||
/// User-facing explanation of the transient failure.
|
||||
message: String,
|
||||
},
|
||||
|
||||
/// A write failed to report a definitive outcome and may already have committed.
|
||||
///
|
||||
/// The operation owner must reconcile the stored state or use an idempotent replay strategy
|
||||
/// before retrying. This must not be treated as an ordinary retryable failure.
|
||||
#[error("thread-store write outcome unknown: {message}")]
|
||||
OutcomeUnknown {
|
||||
/// User-facing explanation of why the write outcome is uncertain.
|
||||
message: String,
|
||||
},
|
||||
|
||||
/// Catch-all for implementation failures that do not fit a more specific category.
|
||||
///
|
||||
/// This includes unclassified failures: callers must not infer retryability or whether a
|
||||
/// write committed from this variant or its message.
|
||||
#[error("thread-store internal error: {message}")]
|
||||
Internal {
|
||||
/// User-facing explanation of the implementation failure.
|
||||
message: String,
|
||||
},
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "error_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
//! Verifies conservative classification at the shared storage error boundary.
|
||||
|
||||
use super::ThreadStoreError;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn unclassified_sqlx_failures_remain_internal() {
|
||||
for error in [
|
||||
sqlx::Error::PoolTimedOut,
|
||||
sqlx::Error::Io(std::io::Error::new(
|
||||
std::io::ErrorKind::ConnectionReset,
|
||||
"connection lost after write",
|
||||
)),
|
||||
sqlx::Error::Protocol("retryable conflict; outcome unknown".to_owned()),
|
||||
] {
|
||||
let expected = format!("failed to access thread history: {error}");
|
||||
let error = ThreadStoreError::from(error);
|
||||
let ThreadStoreError::Internal { message } = error else {
|
||||
panic!("unclassified SQLx failure must remain internal: {error:?}");
|
||||
};
|
||||
assert_eq!(message, expected);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user