removed the line number in the error message (#6736)
This commit is contained in:
@@ -8,7 +8,7 @@ use axum::routing::get;
|
||||
use axum::{extract::State, http::StatusCode, routing::post, Json, Router};
|
||||
use goose::recipe::local_recipes;
|
||||
use goose::recipe::validate_recipe::validate_recipe_template_from_content;
|
||||
use goose::recipe::Recipe;
|
||||
use goose::recipe::{strip_error_location, Recipe};
|
||||
use goose::{recipe_deeplink, slash_commands};
|
||||
|
||||
use serde::{Deserialize, Serialize};
|
||||
@@ -443,7 +443,7 @@ async fn save_recipe(
|
||||
payload: Result<Json<Value>, JsonRejection>,
|
||||
) -> Result<Json<SaveRecipeResponse>, ErrorResponse> {
|
||||
let Json(raw_json) = payload.map_err(json_rejection_to_error_response)?;
|
||||
let request = deserialize_save_recipe_request(raw_json)?;
|
||||
let request: SaveRecipeRequest = deserialize_save_recipe_request(raw_json)?;
|
||||
let has_security_warnings = request.recipe.check_for_security_warnings();
|
||||
if has_security_warnings {
|
||||
return Err(ErrorResponse {
|
||||
@@ -492,7 +492,7 @@ fn deserialize_save_recipe_request(value: Value) -> Result<SaveRecipeRequest, Er
|
||||
let result: Result<SaveRecipeRequest, _> = deserialize_with_path(&mut deserializer);
|
||||
result.map_err(|err| {
|
||||
let path = err.path().to_string();
|
||||
let inner = err.into_inner();
|
||||
let inner = strip_error_location(&err.into_inner().to_string());
|
||||
let message = if path.is_empty() {
|
||||
format!("Save recipe validation failed: {}", inner)
|
||||
} else {
|
||||
|
||||
@@ -93,3 +93,68 @@ pub struct SessionConfig {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub retry_config: Option<RetryConfig>,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn test_retry_config_validate_success() {
|
||||
let config = RetryConfig {
|
||||
max_retries: 3,
|
||||
checks: vec![],
|
||||
on_failure: None,
|
||||
timeout_seconds: Some(60),
|
||||
on_failure_timeout_seconds: Some(120),
|
||||
};
|
||||
assert!(config.validate().is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_retry_config_validate_max_retries_zero() {
|
||||
let config = RetryConfig {
|
||||
max_retries: 0,
|
||||
checks: vec![],
|
||||
on_failure: None,
|
||||
timeout_seconds: None,
|
||||
on_failure_timeout_seconds: None,
|
||||
};
|
||||
let result = config.validate();
|
||||
assert!(result.is_err());
|
||||
assert_eq!(result.unwrap_err(), "max_retries must be greater than 0");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_retry_config_validate_timeout_zero() {
|
||||
let config = RetryConfig {
|
||||
max_retries: 3,
|
||||
checks: vec![],
|
||||
on_failure: None,
|
||||
timeout_seconds: Some(0),
|
||||
on_failure_timeout_seconds: None,
|
||||
};
|
||||
let result = config.validate();
|
||||
assert!(result.is_err());
|
||||
assert_eq!(
|
||||
result.unwrap_err(),
|
||||
"timeout_seconds must be greater than 0 if specified"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_retry_config_validate_on_failure_timeout_zero() {
|
||||
let config = RetryConfig {
|
||||
max_retries: 3,
|
||||
checks: vec![],
|
||||
on_failure: None,
|
||||
timeout_seconds: None,
|
||||
on_failure_timeout_seconds: Some(0),
|
||||
};
|
||||
let result = config.validate();
|
||||
assert!(result.is_err());
|
||||
assert_eq!(
|
||||
result.unwrap_err(),
|
||||
"on_failure_timeout_seconds must be greater than 0 if specified"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -638,3 +638,29 @@ parameters:
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_recipe_from_template_invalid_retry_config() {
|
||||
let instructions_and_parameters = r#"
|
||||
"instructions": "Test instructions",
|
||||
"retry": {
|
||||
"max_retries": 0,
|
||||
"checks": []
|
||||
}"#;
|
||||
let (_temp_dir, recipe_content, recipe_dir) = setup_recipe_file(instructions_and_parameters);
|
||||
|
||||
let build_recipe_result =
|
||||
build_recipe_from_template(recipe_content, &recipe_dir, Vec::new(), NO_USER_PROMPT);
|
||||
assert!(build_recipe_result.is_err());
|
||||
let err = build_recipe_result.unwrap_err();
|
||||
|
||||
match err {
|
||||
RecipeError::TemplateRendering { source } => {
|
||||
assert_eq!(
|
||||
source.to_string(),
|
||||
"Invalid retry configuration: max_retries must be greater than 0"
|
||||
);
|
||||
}
|
||||
_ => panic!("Expected TemplateRendering error, got: {:?}", err),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -28,6 +28,16 @@ fn default_version() -> String {
|
||||
"1.0.0".to_string()
|
||||
}
|
||||
|
||||
/// Strips location information (e.g., "at line X column Y") from error messages
|
||||
/// to make them more user-friendly for UI display.
|
||||
pub fn strip_error_location(error_msg: &str) -> String {
|
||||
error_msg
|
||||
.split(" at line")
|
||||
.next()
|
||||
.unwrap_or_default()
|
||||
.to_string()
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, ToSchema)]
|
||||
pub struct Recipe {
|
||||
// Required fields
|
||||
@@ -268,25 +278,16 @@ impl Recipe {
|
||||
Ok(yaml_value) => {
|
||||
if let Some(nested_recipe) = yaml_value.get("recipe") {
|
||||
serde_yaml::from_value(nested_recipe.clone())
|
||||
.map_err(|e| anyhow::anyhow!("Failed to parse nested recipe: {}", e))?
|
||||
.map_err(|e| anyhow::anyhow!("{}", strip_error_location(&e.to_string())))?
|
||||
} else {
|
||||
serde_yaml::from_str(content)
|
||||
.map_err(|e| anyhow::anyhow!("Failed to parse recipe: {}", e))?
|
||||
.map_err(|e| anyhow::anyhow!("{}", strip_error_location(&e.to_string())))?
|
||||
}
|
||||
}
|
||||
Err(_) => serde_yaml::from_str(content)
|
||||
.map_err(|e| anyhow::anyhow!("Failed to parse recipe: {}", e))?,
|
||||
.map_err(|e| anyhow::anyhow!("{}", strip_error_location(&e.to_string())))?,
|
||||
};
|
||||
|
||||
if let Some(ref retry_config) = recipe.retry {
|
||||
if let Err(validation_error) = retry_config.validate() {
|
||||
return Err(anyhow::anyhow!(
|
||||
"Invalid retry configuration: {}",
|
||||
validation_error
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Ok(recipe)
|
||||
}
|
||||
}
|
||||
@@ -772,4 +773,52 @@ isGlobal: true"#;
|
||||
panic!("Expected Stdio extension");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_serde_error_removes_location() {
|
||||
let content = r#"{"version": "1.0.0"}"#;
|
||||
|
||||
let result = Recipe::from_content(content);
|
||||
assert!(result.is_err());
|
||||
|
||||
let error_msg = result.unwrap_err().to_string();
|
||||
assert_eq!(error_msg, "missing field `title`");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_serde_error_missing_title() {
|
||||
let content = r#"{
|
||||
"version": "1.0.0",
|
||||
"description": "A test recipe",
|
||||
"instructions": "Test instructions"
|
||||
}"#;
|
||||
|
||||
let result = Recipe::from_content(content);
|
||||
assert!(result.is_err());
|
||||
|
||||
let error_msg = result.unwrap_err().to_string();
|
||||
assert_eq!(error_msg, "missing field `title`");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_format_serde_error_invalid_type() {
|
||||
let content = r#"{
|
||||
"version": "1.0.0",
|
||||
"title": "Test",
|
||||
"description": "Test",
|
||||
"instructions": "Test",
|
||||
"settings": {
|
||||
"temperature": "not_a_number"
|
||||
}
|
||||
}"#;
|
||||
|
||||
let result = Recipe::from_content(content);
|
||||
assert!(result.is_err());
|
||||
|
||||
let error_msg = result.unwrap_err().to_string();
|
||||
assert_eq!(
|
||||
error_msg,
|
||||
"settings.temperature: invalid type: string \"not_a_number\", expected f32"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,7 +8,7 @@ use anyhow::Result;
|
||||
use minijinja::{Environment, UndefinedBehavior};
|
||||
use regex::Regex;
|
||||
|
||||
const CURRENT_TEMPLATE_NAME: &str = "current_template";
|
||||
const CURRENT_TEMPLATE_NAME: &str = "recipe_template";
|
||||
const OPEN_BRACE: &str = "{{";
|
||||
const CLOSE_BRACE: &str = "}}";
|
||||
|
||||
|
||||
@@ -44,6 +44,7 @@ pub fn validate_recipe_template_from_content(
|
||||
let (recipe, _) = parse_recipe_content(recipe_content, recipe_dir)?;
|
||||
|
||||
validate_prompt_or_instructions(&recipe)?;
|
||||
validate_retry_config(&recipe)?;
|
||||
if let Some(response) = &recipe.response {
|
||||
if let Some(json_schema) = &response.json_schema {
|
||||
validate_json_schema(json_schema)?;
|
||||
@@ -53,6 +54,18 @@ pub fn validate_recipe_template_from_content(
|
||||
Ok(recipe)
|
||||
}
|
||||
|
||||
fn validate_retry_config(recipe: &Recipe) -> Result<()> {
|
||||
if let Some(ref retry_config) = recipe.retry {
|
||||
if let Err(validation_error) = retry_config.validate() {
|
||||
return Err(anyhow::anyhow!(
|
||||
"Invalid retry configuration: {}",
|
||||
validation_error
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_prompt_or_instructions(recipe: &Recipe) -> Result<()> {
|
||||
let has_instructions = recipe
|
||||
.instructions
|
||||
|
||||
Reference in New Issue
Block a user