mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: make ConstraintError an enum (#8330)
This will make it easier to test for expected errors in unit tests since we can compare based on the field values rather than the message (which might change over time). See https://github.com/openai/codex/pull/8298 for an example. It also ensures more consistency in the way a `ConstraintError` is constructed.
This commit is contained in:
@@ -78,7 +78,6 @@ use crate::client_common::ResponseEvent;
|
|||||||
use crate::compact::collect_user_messages;
|
use crate::compact::collect_user_messages;
|
||||||
use crate::config::Config;
|
use crate::config::Config;
|
||||||
use crate::config::Constrained;
|
use crate::config::Constrained;
|
||||||
use crate::config::ConstraintError;
|
|
||||||
use crate::config::ConstraintResult;
|
use crate::config::ConstraintResult;
|
||||||
use crate::config::GhostSnapshotConfig;
|
use crate::config::GhostSnapshotConfig;
|
||||||
use crate::config::types::ShellEnvironmentPolicy;
|
use crate::config::types::ShellEnvironmentPolicy;
|
||||||
@@ -836,11 +835,8 @@ impl Session {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
Err(err) => {
|
Err(err) => {
|
||||||
let wrapped = ConstraintError {
|
warn!("rejected session settings update: {err}");
|
||||||
message: format!("Could not update config: {err}"),
|
Err(err)
|
||||||
};
|
|
||||||
warn!(%wrapped, "rejected session settings update");
|
|
||||||
Err(wrapped)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -861,18 +857,15 @@ impl Session {
|
|||||||
}
|
}
|
||||||
Err(err) => {
|
Err(err) => {
|
||||||
drop(state);
|
drop(state);
|
||||||
let wrapped = ConstraintError {
|
|
||||||
message: format!("Could not update config: {err}"),
|
|
||||||
};
|
|
||||||
self.send_event_raw(Event {
|
self.send_event_raw(Event {
|
||||||
id: sub_id.clone(),
|
id: sub_id.clone(),
|
||||||
msg: EventMsg::Error(ErrorEvent {
|
msg: EventMsg::Error(ErrorEvent {
|
||||||
message: wrapped.to_string(),
|
message: err.to_string(),
|
||||||
codex_error_info: Some(CodexErrorInfo::BadRequest),
|
codex_error_info: Some(CodexErrorInfo::BadRequest),
|
||||||
}),
|
}),
|
||||||
})
|
})
|
||||||
.await;
|
.await;
|
||||||
return Err(wrapped);
|
return Err(err);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -4,25 +4,25 @@ use std::sync::Arc;
|
|||||||
use thiserror::Error;
|
use thiserror::Error;
|
||||||
|
|
||||||
#[derive(Debug, Error, PartialEq, Eq)]
|
#[derive(Debug, Error, PartialEq, Eq)]
|
||||||
#[error("{message}")]
|
pub enum ConstraintError {
|
||||||
pub struct ConstraintError {
|
#[error("value `{candidate}` is not in the allowed set {allowed}")]
|
||||||
pub message: String,
|
InvalidValue { candidate: String, allowed: String },
|
||||||
|
|
||||||
|
#[error("field `{field_name}` cannot be empty")]
|
||||||
|
EmptyField { field_name: String },
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ConstraintError {
|
impl ConstraintError {
|
||||||
pub fn invalid_value(candidate: impl Into<String>, allowed: impl Into<String>) -> Self {
|
pub fn invalid_value(candidate: impl Into<String>, allowed: impl Into<String>) -> Self {
|
||||||
Self {
|
Self::InvalidValue {
|
||||||
message: format!(
|
candidate: candidate.into(),
|
||||||
"value `{}` is not in the allowed set {}",
|
allowed: allowed.into(),
|
||||||
candidate.into(),
|
|
||||||
allowed.into()
|
|
||||||
),
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn empty_field(field_name: impl Into<String>) -> Self {
|
pub fn empty_field(field_name: impl Into<String>) -> Self {
|
||||||
Self {
|
Self::EmptyField {
|
||||||
message: format!("field `{}` cannot be empty", field_name.into()),
|
field_name: field_name.into(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2275,12 +2275,12 @@ async fn approvals_popup_shows_disabled_presets() {
|
|||||||
chat.config.approval_policy =
|
chat.config.approval_policy =
|
||||||
Constrained::new(AskForApproval::OnRequest, |candidate| match candidate {
|
Constrained::new(AskForApproval::OnRequest, |candidate| match candidate {
|
||||||
AskForApproval::OnRequest => Ok(()),
|
AskForApproval::OnRequest => Ok(()),
|
||||||
_ => Err(ConstraintError {
|
_ => Err(ConstraintError::invalid_value(
|
||||||
message: "this message should be printed in the description".to_string(),
|
candidate.to_string(),
|
||||||
}),
|
"this message should be printed in the description",
|
||||||
|
)),
|
||||||
})
|
})
|
||||||
.expect("construct constrained approval policy");
|
.expect("construct constrained approval policy");
|
||||||
|
|
||||||
chat.open_approvals_popup();
|
chat.open_approvals_popup();
|
||||||
|
|
||||||
let width = 80;
|
let width = 80;
|
||||||
@@ -2311,12 +2311,12 @@ async fn approvals_popup_navigation_skips_disabled() {
|
|||||||
chat.config.approval_policy =
|
chat.config.approval_policy =
|
||||||
Constrained::new(AskForApproval::OnRequest, |candidate| match candidate {
|
Constrained::new(AskForApproval::OnRequest, |candidate| match candidate {
|
||||||
AskForApproval::OnRequest => Ok(()),
|
AskForApproval::OnRequest => Ok(()),
|
||||||
_ => Err(ConstraintError {
|
_ => Err(ConstraintError::invalid_value(
|
||||||
message: "disabled preset".to_string(),
|
candidate.to_string(),
|
||||||
}),
|
"[on-request]",
|
||||||
|
)),
|
||||||
})
|
})
|
||||||
.expect("construct constrained approval policy");
|
.expect("construct constrained approval policy");
|
||||||
|
|
||||||
chat.open_approvals_popup();
|
chat.open_approvals_popup();
|
||||||
|
|
||||||
// The approvals popup is the active bottom-pane view; drive navigation via chat handle_key_event.
|
// The approvals popup is the active bottom-pane view; drive navigation via chat handle_key_event.
|
||||||
|
|||||||
Reference in New Issue
Block a user