Skip to content

Commit

Permalink
Update error messages to not suck, update tests for it.
Browse files Browse the repository at this point in the history
  • Loading branch information
jsuereth committed Dec 16, 2024
1 parent 6b03c69 commit 4a331ed
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 13 deletions.
121 changes: 109 additions & 12 deletions crates/weaver_forge/src/jq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use jaq_core::{
};
use jaq_json::Val;

type JqFileType = ();

fn semconv_prelude() -> impl Iterator<Item = Def<&'static str>> {
jaq_core::load::parse(crate::SEMCONV_JQ, |p| p.defs())
.expect("BAD WEAVER BUILD - default JQ library failed to compile")
Expand All @@ -25,6 +27,7 @@ fn prepare_jq_context(params: &BTreeMap<String, serde_json::Value>) -> (Vec<Stri
(jq_vars, jq_ctx)
}

/// This is our single entry point for calling into the jaq library to run jq filters.
pub fn execute_jq(
// The JSON input to JQ.
input: &serde_json::Value,
Expand All @@ -40,17 +43,18 @@ pub fn execute_jq(
.chain(semconv_prelude()), // [],
);
let arena = Arena::default();
let program = File {
let program: File<&str, JqFileType> = File {
code: filter_expr,
path: (), // ToDo - give this the weaver-config location.
};

// parse the filter
let modules = loader
.load(&arena, program)
.map_err(|errors| Error::FilterError {
.map_err(load_errors)
.map_err(|e| Error::FilterError {
filter: filter_expr.to_owned(),
error: format!("{errors:#?}"),
error: e,
})?;

let (names, values) = prepare_jq_context(params);
Expand All @@ -62,15 +66,10 @@ pub fn execute_jq(
// This is *NOT* a simple identity function, but a lifetime inference workaround.
.with_funs(funs.map(|x| x))
.compile(modules)
.map_err(|e| {
Error::CompoundError(
e.into_iter()
.map(|(_, errors)| Error::FilterError {
filter: filter_expr.to_owned(),
error: format!("{:?}", errors),
})
.collect(),
)
.map_err(compile_errors)
.map_err(|e| Error::FilterError {
filter: filter_expr.to_owned(),
error: e,
})?;
let inputs = RcIter::new(core::iter::empty());
let ctx = Ctx::new(values, &inputs);
Expand All @@ -93,6 +92,62 @@ pub fn execute_jq(
Ok(serde_json::Value::Array(values))
}

// JAQ errors must be parsed and synthesized. All of this code is adapted from `jaq/src/main.rs`.

/// Converts all errors from jaq into a single string.
fn errors_to_string<Reports: Iterator<Item = String>>(reports: Reports) -> String {
reports.into_iter().collect()
}

/// Turns loading errors from jaq into raw strings.
fn load_errors(errs: jaq_core::load::Errors<&str, JqFileType>) -> String {
use jaq_core::load::Error;
let errs = errs.into_iter().flat_map(|(_, err)| {
let result: Vec<String> = match err {
Error::Io(errs) => errs.into_iter().map(|e| report_io(e)).collect(),
Error::Lex(errs) => errs.into_iter().map(|e| report_lex(e)).collect(),
Error::Parse(errs) => errs.into_iter().map(|e| report_parse(e)).collect(),
};
result
});
errors_to_string(errs)
}

/// Turns compile errors from jaq into raw strings.
fn compile_errors(errs: jaq_core::compile::Errors<&str, JqFileType>) -> String {
let errs = errs
.into_iter()
.flat_map(|(_, errs)| errs.into_iter().map(|e| report_compile(e)));
errors_to_string(errs)
}

/// Turns IO errors from JQ into raw strings.
fn report_io((path, error): (&str, String)) -> String {
format!("could not load file {}: {}", path, error)
}

/// Turns lexing errors from JQ into raw strings.
fn report_lex((expected, _): jaq_core::load::lex::Error<&str>) -> String {
format!("expected {}", expected.as_str())
}

/// Turns parsing errors from JQ into raw strings.
fn report_parse((expected, _): jaq_core::load::parse::Error<&str>) -> String {
format!("expected {}", expected.as_str())
}

/// Turns erros coming from JAQ compile phase into raw strings.

Check warning on line 139 in crates/weaver_forge/src/jq.rs

View workflow job for this annotation

GitHub Actions / Spell Check with Typos

"erros" should be "errors".
fn report_compile((found, undefined): jaq_core::compile::Error<&str>) -> String {
use jaq_core::compile::Undefined::Filter;
let wnoa = |exp, got| format!("wrong number of arguments (expected {exp}, found {got})");
let message = match (found, undefined) {
("reduce", Filter(arity)) => wnoa("2", arity),
("foreach", Filter(arity)) => wnoa("2 or 3", arity),
(_, undefined) => format!("undefined {}", undefined.as_str()),
};
message
}

#[cfg(test)]
mod tests {
use serde_json::json;
Expand Down Expand Up @@ -126,4 +181,46 @@ mod tests {
let result = execute_jq(&input, "$ctx1", &values).unwrap();
assert_eq!(result, values["ctx1"]);
}

#[test]
fn test_lex_error() {
let input = json!({});
let values = BTreeMap::new();
let error = execute_jq(&input, "(", &values)
.err()
.expect("Should have failed to lex");
let msg = format!("{error}");
assert!(
msg.contains("expected closing parenthesis"),
"Expected lex error {msg}"
);
}

#[test]
fn test_parse_error() {
let input = json!({});
let values = BTreeMap::new();
let error = execute_jq(&input, "if false then .", &values)
.err()
.expect("Should have failed to parse");
let msg = format!("{error}");
assert!(
msg.contains("expected else or end"),
"Expected parse error {msg}"
);
}

#[test]
fn test_compile_error() {
let input = json!({});
let values = BTreeMap::new();
let error = execute_jq(&input, ".x | de", &values)
.err()
.expect("Should have failed to parse");
let msg = format!("{error}");
assert!(
msg.contains("undefined filter"),
"Expected compile error {msg}"
);
}
}
2 changes: 1 addition & 1 deletion crates/weaver_forge/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -739,7 +739,7 @@ mod tests {
let loader = FileSystemFileLoader::try_new("templates".into(), target)
.expect("Failed to create file system loader");
let config = WeaverConfig::try_from_path(format!("templates/{}", target)).unwrap();
let mut engine = TemplateEngine::new(config, loader, cli_params);
let engine = TemplateEngine::new(config, loader, cli_params);
let schema = SchemaResolver::resolve_semantic_convention_registry(&mut registry)
.expect("Failed to resolve registry");

Expand Down

0 comments on commit 4a331ed

Please sign in to comment.