mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-15 01:55:13 +02:00
Collect file-I/O and parse errors in QueryRegistry::load in one pass
load() early-returned on any unreadable .gq file, masking parse / identity / tool-name-collision errors in the OTHER (readable) files — so an operator fixed the missing file, restarted, and only then saw the next broken query. Now it collects I/O errors but still runs from_specs on the readable specs and returns the union, so every broken entry surfaces at once (matching the collected-errors contract the rest of the registry already follows). Safe: from_specs' tool-name collision check runs over loaded queries only, so dropping an I/O-failed entry can only under-report a collision, never invent one. I/O errors are ordered first (BTreeMap key order), then spec errors. Adds a load-level test (tempdir: a valid, a missing, and a parse-broken .gq) asserting all three surface in one Err — confirmed red before the fix.
This commit is contained in:
parent
a7f6fbc073
commit
c9e13f3707
1 changed files with 45 additions and 3 deletions
|
|
@ -195,10 +195,19 @@ impl QueryRegistry {
|
|||
}
|
||||
}
|
||||
|
||||
if !errors.is_empty() {
|
||||
return Err(errors);
|
||||
// Parse/identity/uniqueness-check the readable specs even when some
|
||||
// files failed to read, so every broken entry (I/O, parse, identity,
|
||||
// tool-name collision) surfaces in one pass rather than one per
|
||||
// restart. I/O errors come first (in `entries` key order), then the
|
||||
// spec errors. A non-empty `errors` always fails the load.
|
||||
match Self::from_specs(specs) {
|
||||
Ok(registry) if errors.is_empty() => Ok(registry),
|
||||
Ok(_) => Err(errors),
|
||||
Err(spec_errors) => {
|
||||
errors.extend(spec_errors);
|
||||
Err(errors)
|
||||
}
|
||||
}
|
||||
Self::from_specs(specs)
|
||||
}
|
||||
|
||||
pub fn lookup(&self, name: &str) -> Option<&StoredQuery> {
|
||||
|
|
@ -623,4 +632,37 @@ embedding: Vector(4)
|
|||
let entry2 = api::query_catalog_entry(reg2.lookup("no_params").unwrap());
|
||||
assert!(entry2.params.is_empty(), "no declared params → empty list");
|
||||
}
|
||||
|
||||
// --- load() error collection (file I/O + parse in one pass) ---
|
||||
|
||||
#[test]
|
||||
fn load_collects_io_and_parse_errors_in_one_pass() {
|
||||
use crate::config::load_config;
|
||||
let temp = tempfile::tempdir().unwrap();
|
||||
std::fs::write(
|
||||
temp.path().join("good.gq"),
|
||||
"query good() { match { $u: User } return { $u.name } }",
|
||||
)
|
||||
.unwrap();
|
||||
std::fs::write(temp.path().join("broken.gq"), "query broken( {{ not valid").unwrap();
|
||||
// `missing.gq` is deliberately not written (an I/O failure).
|
||||
std::fs::write(
|
||||
temp.path().join("omnigraph.yaml"),
|
||||
"queries:\n good:\n file: ./good.gq\n \
|
||||
missing:\n file: ./missing.gq\n broken:\n file: ./broken.gq\n",
|
||||
)
|
||||
.unwrap();
|
||||
let config = load_config(Some(&temp.path().join("omnigraph.yaml"))).unwrap();
|
||||
|
||||
let errors = QueryRegistry::load(&config, config.query_entries()).unwrap_err();
|
||||
let joined = errors.iter().map(|e| e.to_string()).collect::<Vec<_>>().join("\n");
|
||||
// Both the missing file AND the parse error surface in one pass —
|
||||
// the I/O failure must not mask the parse failure.
|
||||
assert!(joined.contains("missing"), "I/O error must surface: {joined}");
|
||||
assert!(
|
||||
joined.contains("broken") && joined.contains("parse error"),
|
||||
"the parse error in a readable file must surface in the same pass: {joined}"
|
||||
);
|
||||
assert!(!joined.contains("'good'"), "the valid entry is not an error: {joined}");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue