[realm_builder] Format ErrorList messages
This CL makes the format for cm_fild_analyzer::error::ErrorList
structured, helping it be more readable for users. It turns:
```
[98805.927319][18733956][18733958][realm_builder_server] WARN: unable to replace realm decl: a component manifest failed validation: ErrorList { errs: [InvalidField(DeclField { decl: "ExposeProtocol", field: "source_name" }), InvalidField(DeclField { decl: "ExposeProtocol", field: "target_name" })] }
```
To:
```
[00060.285693][50201][50203][realm_builder_server] WARN: The constructed component declaration is invalid. Please fix all the listed errors:
1. "secho_server" is referenced in ExposeProtocol.source but it does not appear in children. Declare one in the `children` section of the component declaration with the name "secho_server" in order to reference it.
2. Field `target` for ExposeProtocol is invalid.
3. Field `source_name` is missing for ExposeProtocol decl. Add this field to the component declaration.
For a reference as to how component declarations are authored, see https://fuchsia.dev/go/components/declaration. method=Realm.ReplaceRealmDecl
```
Fixed: 100731
Change-Id: I4b8659ca66586fa32bcd34f5a1eeadcdeac16dd6
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/689505
Commit-Queue: Yaneury Fermin <yaneury@google.com>
Reviewed-by: Gary Bressler <geb@google.com>
diff --git a/src/lib/fuchsia-component-test/realm_builder_server/src/main.rs b/src/lib/fuchsia-component-test/realm_builder_server/src/main.rs
index ed161a3..d8e1261 100644
--- a/src/lib/fuchsia-component-test/realm_builder_server/src/main.rs
+++ b/src/lib/fuchsia-component-test/realm_builder_server/src/main.rs
@@ -517,7 +517,10 @@
options: ftest::ChildOptions,
) -> Result<(), RealmBuilderError> {
if let Err(e) = cm_fidl_validator::validate(&component_decl) {
- return Err(RealmBuilderError::InvalidComponentDeclWithName(name, e));
+ return Err(RealmBuilderError::InvalidComponentDeclWithName(
+ name,
+ to_tabulated_string(e),
+ ));
}
let child_realm_node = RealmNode2::new_from_decl(component_decl.fidl_into_native(), false);
self.realm_node.add_child(name, options, child_realm_node).await
@@ -798,7 +801,7 @@
});
decl.children.get_or_insert(vec![]).extend(child_decls);
if let Err(e) = cm_fidl_validator::validate(&decl) {
- return Err(RealmBuilderError::InvalidComponentDecl(e));
+ return Err(RealmBuilderError::InvalidComponentDecl(to_tabulated_string(e)));
}
Ok(())
}
@@ -915,8 +918,12 @@
let fidl_decl = fuchsia_fs::read_file_fidl::<fcdecl::Component>(&file_proxy)
.await
.map_err(|e| RealmBuilderError::DeclReadError(relative_url.clone(), e))?;
- cm_fidl_validator::validate(&fidl_decl)
- .map_err(|e| RealmBuilderError::InvalidComponentDeclWithName(relative_url, e))?;
+ cm_fidl_validator::validate(&fidl_decl).map_err(|e| {
+ RealmBuilderError::InvalidComponentDeclWithName(
+ relative_url,
+ to_tabulated_string(e),
+ )
+ })?;
let mut self_ = RealmNode2::new_from_decl(fidl_decl.fidl_into_native(), false);
self_.component_loaded_from_pkg = true;
@@ -1061,9 +1068,10 @@
.await
{
Ok(url) => Ok(url),
- Err(e) => {
- Err(RealmBuilderError::InvalidComponentDeclWithName(walked_path.join("/"), e))
- }
+ Err(e) => Err(RealmBuilderError::InvalidComponentDeclWithName(
+ walked_path.join("/"),
+ to_tabulated_string(e),
+ )),
}
}
.boxed()
@@ -1630,11 +1638,11 @@
/// A component declaration failed validation.
#[error("The constructed component declaration is invalid. Please fix all the listed errors:\n{0}\nFor a reference as to how component declarations are authored, see https://fuchsia.dev/go/components/declaration.")]
- InvalidComponentDecl(cm_fidl_validator::error::ErrorList),
+ InvalidComponentDecl(String),
/// A component declaration failed validation.
#[error("The component declaration for child \"{0}\" is invalid. Please fix all the listed errors:\n{1}\nFor a reference as to how component declarations are authored, see https://fuchsia.dev/go/components/declaration.")]
- InvalidComponentDeclWithName(String, cm_fidl_validator::error::ErrorList),
+ InvalidComponentDeclWithName(String, String),
/// The referenced child does not exist.
#[error("No child exists with the name \"{0}\". Before fetching or changing its component declaration, a child must be added to the realm with the `AddChild` group of methods.")]
@@ -1752,6 +1760,21 @@
url.trim().ends_with(".cmx")
}
+// Formats an ErrorList into a tabulated string. This format is used to create
+// more readable user error messages.
+fn to_tabulated_string(errors: cm_fidl_validator::error::ErrorList) -> String {
+ let mut output = String::new();
+ for (i, err) in errors.errs.iter().enumerate() {
+ let is_last_element = errors.errs.len() - i == 1;
+ output.push_str(&format!(" {}. {}", i + 1, err));
+ if !is_last_element {
+ output.push('\n');
+ }
+ }
+
+ output
+}
+
#[cfg(test)]
mod tests {
use {
diff --git a/src/sys/lib/cm_fidl_validator/src/error.rs b/src/sys/lib/cm_fidl_validator/src/error.rs
index e93c495..5cc3138 100644
--- a/src/sys/lib/cm_fidl_validator/src/error.rs
+++ b/src/sys/lib/cm_fidl_validator/src/error.rs
@@ -7,47 +7,67 @@
/// Enum type that can represent any error encountered during validation.
#[derive(Debug, Error, PartialEq, Clone)]
pub enum Error {
- #[error("{} missing {}", .0.decl, .0.field)]
+ #[error("Field `{}` is missing for {}.", .0.field, .0.decl)]
MissingField(DeclField),
- #[error("{} has empty {}", .0.decl, .0.field)]
+
+ #[error("Field `{}` is empty for {}.", .0.field, .0.decl)]
EmptyField(DeclField),
- #[error("{} has extraneous {}", .0.decl, .0.field)]
+
+ #[error("{} has unnecessary field `{}`.", .0.decl, .0.field)]
ExtraneousField(DeclField),
- #[error("\"{1}\" is a duplicate {} {}", .0.decl, .0.field)]
+
+ #[error("\"{1}\" is duplicated for field `{}` in {}.", .0.field, .0.decl)]
DuplicateField(DeclField, String),
- #[error("{} has invalid {}", .0.decl, .0.field)]
+
+ #[error("Field `{}` for {} is invalid.", .0.field, .0.decl)]
InvalidField(DeclField),
- #[error("{}'s {} is too long", .0.decl, .0.field)]
- FieldTooLong(DeclField),
- #[error("\"{0}\" cannot declare a capability of type {1}")]
+
+ #[error("Field `{}` for {} is too long. The field must be up to {1} characters.", .0.field, .0.decl)]
+ FieldTooLong(DeclField, usize),
+
+ #[error("\"{0}\" cannot declare a capability of type `{1}`.")]
InvalidCapabilityType(DeclField, String),
- #[error("\"{0}\" target \"{1}\" is same as source")]
+
+ #[error("\"{0}\" target \"{1}\" is same as source.")]
OfferTargetEqualsSource(String, String),
- #[error("\"{1}\" is referenced in {0} but it does not appear in children")]
+
+ #[error("\"{1}\" is referenced in {0} but it does not appear in children.")]
InvalidChild(DeclField, String),
- #[error("\"{1}\" is referenced in {0} but it does not appear in collections")]
+
+ #[error("\"{1}\" is referenced in {0} but it does not appear in collections.")]
InvalidCollection(DeclField, String),
- #[error("\"{1}\" is referenced in {0} but it does not appear in storage")]
+
+ #[error("\"{1}\" is referenced in {0} but it does not appear in storage.")]
InvalidStorage(DeclField, String),
- #[error("\"{1}\" is referenced in {0} but it does not appear in environments")]
+
+ #[error("\"{1}\" is referenced in {0} but it does not appear in environments.")]
InvalidEnvironment(DeclField, String),
- #[error("\"{1}\" is referenced in {0} but it does not appear in capabilities")]
+
+ #[error("\"{1}\" is referenced in {0} but it does not appear in capabilities.")]
InvalidCapability(DeclField, String),
- #[error("\"{1}\" is referenced in {0} but it does not appear in runners")]
+
+ #[error("\"{1}\" is referenced in {0} but it does not appear in runners.")]
InvalidRunner(DeclField, String),
- #[error("\"{1}\" is referenced in {0} but it does not appear in events")]
+
+ #[error("\"{1}\" is referenced in {0} but there is no corresponding event provided to the component. Specifically, the event has to be `use`d.")]
EventStreamEventNotFound(DeclField, String),
- #[error("dependency cycle(s) exist: {0}")]
+
+ #[error("There are dependency cycle(s): {0}.")]
DependencyCycle(String),
- #[error("{} \"{}\" path overlaps with {} \"{}\"", decl, path, other_decl, other_path)]
+
+ #[error("Path \"{path}\" from {decl} overlaps with \"{other_path}\" path from {other_decl}. Paths across decls must be unique in order to avoid namespace collisions.")]
InvalidPathOverlap { decl: DeclField, path: String, other_decl: DeclField, other_path: String },
- #[error("built-in capability decl {0} should not specify a source path, found \"{1}\"")]
+
+ #[error("Source path \"{1}\" provided to {0} decl is unnecessary. Built-in capabilities don't need this field as they originate from the framework.")]
ExtraneousSourcePath(DeclField, String),
- #[error("configuration schema defines a vector nested in another vector")]
+
+ #[error("Configuration schema defines a vector nested inside another vector. Vector can only contain numbers, booleans, and strings.")]
NestedVector,
- #[error("the availability in {0} for {1} must be set to \"required\" because the source is not \"parent\" or \"void\"")]
+
+ #[error("The `availability` field in {0} for {1} must be set to \"required\" because the source is not parent.")]
AvailabilityMustBeRequired(DeclField, String),
- #[error("the availability in {0} for {1} must be set to \"optional\" because the source is \"void\"")]
+
+ #[error("The `availability` field in {0} for {1} must be set to \"optional\" because the source is \"void\".")]
AvailabilityMustBeOptional(DeclField, String),
}
@@ -80,7 +100,18 @@
}
pub fn field_too_long(decl_type: impl Into<String>, keyword: impl Into<String>) -> Self {
- Error::FieldTooLong(DeclField { decl: decl_type.into(), field: keyword.into() })
+ Error::FieldTooLong(
+ DeclField { decl: decl_type.into(), field: keyword.into() },
+ cm_types::MAX_NAME_LENGTH,
+ )
+ }
+
+ pub fn field_too_long_with_max(
+ decl_type: impl Into<String>,
+ keyword: impl Into<String>,
+ max: usize,
+ ) -> Self {
+ Error::FieldTooLong(DeclField { decl: decl_type.into(), field: keyword.into() }, max)
}
pub fn invalid_capability_type(
@@ -265,31 +296,37 @@
#[test]
fn test_errors() {
- assert_eq!(format!("{}", Error::missing_field("Decl", "keyword")), "Decl missing keyword");
- assert_eq!(format!("{}", Error::empty_field("Decl", "keyword")), "Decl has empty keyword");
+ assert_eq!(
+ format!("{}", Error::missing_field("Decl", "keyword")),
+ "Field `keyword` is missing for Decl."
+ );
+ assert_eq!(
+ format!("{}", Error::empty_field("Decl", "keyword")),
+ "Field `keyword` is empty for Decl."
+ );
assert_eq!(
format!("{}", Error::duplicate_field("Decl", "keyword", "foo")),
- "\"foo\" is a duplicate Decl keyword"
+ "\"foo\" is duplicated for field `keyword` in Decl."
);
assert_eq!(
format!("{}", Error::invalid_field("Decl", "keyword")),
- "Decl has invalid keyword"
+ "Field `keyword` for Decl is invalid."
);
assert_eq!(
- format!("{}", Error::field_too_long("Decl", "keyword")),
- "Decl's keyword is too long"
+ format!("{}", Error::field_too_long_with_max("Decl", "keyword", 100)),
+ "Field `keyword` for Decl is too long. The field must be up to 100 characters."
);
assert_eq!(
format!("{}", Error::invalid_child("Decl", "source", "child")),
- "\"child\" is referenced in Decl.source but it does not appear in children"
+ "\"child\" is referenced in Decl.source but it does not appear in children."
);
assert_eq!(
format!("{}", Error::invalid_collection("Decl", "source", "child")),
- "\"child\" is referenced in Decl.source but it does not appear in collections"
+ "\"child\" is referenced in Decl.source but it does not appear in collections."
);
assert_eq!(
format!("{}", Error::invalid_storage("Decl", "source", "name")),
- "\"name\" is referenced in Decl.source but it does not appear in storage"
+ "\"name\" is referenced in Decl.source but it does not appear in storage."
);
}
}
diff --git a/src/sys/lib/cm_fidl_validator/src/lib.rs b/src/sys/lib/cm_fidl_validator/src/lib.rs
index 0cc8930..91efc09 100644
--- a/src/sys/lib/cm_fidl_validator/src/lib.rs
+++ b/src/sys/lib/cm_fidl_validator/src/lib.rs
@@ -4518,15 +4518,15 @@
decl
},
result = Err(ErrorList::new(vec![
- Error::field_too_long("UseEvent", "source_name"),
- Error::field_too_long("UseEvent", "target_name"),
- Error::field_too_long("UseService", "source_name"),
- Error::field_too_long("UseService", "target_path"),
- Error::field_too_long("UseProtocol", "source_name"),
- Error::field_too_long("UseProtocol", "target_path"),
- Error::field_too_long("UseDirectory", "source_name"),
- Error::field_too_long("UseDirectory", "target_path"),
- Error::field_too_long("UseStorage", "target_path"),
+ Error::field_too_long_with_max("UseEvent", "source_name", 100),
+ Error::field_too_long_with_max("UseEvent", "target_name", 100),
+ Error::field_too_long_with_max("UseService", "source_name", 100),
+ Error::field_too_long_with_max("UseService", "target_path", 1024),
+ Error::field_too_long_with_max("UseProtocol", "source_name", 100),
+ Error::field_too_long_with_max("UseProtocol", "target_path", 1024),
+ Error::field_too_long_with_max("UseDirectory", "source_name", 100),
+ Error::field_too_long_with_max("UseDirectory", "target_path", 1024),
+ Error::field_too_long_with_max("UseStorage", "target_path", 1024),
])),
},
test_validate_conflicting_paths => {
@@ -7828,9 +7828,9 @@
decl
},
result = Err(ErrorList::new(vec![
- Error::field_too_long("Child", "name"),
- Error::field_too_long("Child", "url"),
- Error::field_too_long("Child", "environment"),
+ Error::field_too_long_with_max("Child", "name", 100),
+ Error::field_too_long_with_max("Child", "url", 4096),
+ Error::field_too_long_with_max("Child", "environment", 100),
Error::invalid_environment("Child", "environment", "a".repeat(1025)),
])),
},
@@ -8108,19 +8108,19 @@
decl
},
result = Err(ErrorList::new(vec![
- Error::field_too_long("Service", "name"),
- Error::field_too_long("Service", "source_path"),
- Error::field_too_long("Protocol", "name"),
- Error::field_too_long("Protocol", "source_path"),
- Error::field_too_long("Directory", "name"),
- Error::field_too_long("Directory", "source_path"),
- Error::field_too_long("Storage", "source.child.name"),
- Error::field_too_long("Storage", "name"),
- Error::field_too_long("Storage", "backing_dir"),
- Error::field_too_long("Runner", "name"),
- Error::field_too_long("Runner", "source_path"),
- Error::field_too_long("Resolver", "name"),
- Error::field_too_long("Resolver", "source_path"),
+ Error::field_too_long_with_max("Service", "name", 100),
+ Error::field_too_long_with_max("Service", "source_path", 1024),
+ Error::field_too_long_with_max("Protocol", "name", 100),
+ Error::field_too_long_with_max("Protocol", "source_path", 1024),
+ Error::field_too_long_with_max("Directory", "name", 100),
+ Error::field_too_long_with_max("Directory", "source_path", 1024),
+ Error::field_too_long_with_max("Storage", "source.child.name", 100),
+ Error::field_too_long_with_max("Storage", "name", 100),
+ Error::field_too_long_with_max("Storage", "backing_dir", 100),
+ Error::field_too_long_with_max("Runner", "name", 100),
+ Error::field_too_long_with_max("Runner", "source_path", 1024),
+ Error::field_too_long_with_max("Resolver", "name", 100),
+ Error::field_too_long_with_max("Resolver", "source_path", 1024),
])),
},
test_validate_capabilities_duplicate_name => {
diff --git a/src/sys/lib/cm_fidl_validator/src/util.rs b/src/sys/lib/cm_fidl_validator/src/util.rs
index b81ed4f..b8316da 100644
--- a/src/sys/lib/cm_fidl_validator/src/util.rs
+++ b/src/sys/lib/cm_fidl_validator/src/util.rs
@@ -45,7 +45,7 @@
match prop {
Some(prop) if prop.len() == 0 => errors.push(Error::empty_field(decl_type, keyword)),
Some(prop) if prop.len() > max_len => {
- errors.push(Error::field_too_long(decl_type, keyword))
+ errors.push(Error::field_too_long_with_max(decl_type, keyword, max_len))
}
Some(_) => (),
None => errors.push(Error::missing_field(decl_type, keyword)),
@@ -438,7 +438,7 @@
test_identifier_path_too_long => {
check_fn = check_path,
input = &format!("/{}", "a".repeat(1024)),
- result = Err(ErrorList::new(vec![Error::field_too_long("FooDecl", "foo")])),
+ result = Err(ErrorList::new(vec![Error::field_too_long_with_max("FooDecl", "foo", /*max=*/1024usize)])),
},
// name
@@ -460,12 +460,12 @@
test_identifier_name_too_long => {
check_fn = check_name,
input = &format!("{}", "a".repeat(MAX_NAME_LENGTH + 1)),
- result = Err(ErrorList::new(vec![Error::field_too_long("FooDecl", "foo")])),
+ result = Err(ErrorList::new(vec![Error::field_too_long_with_max("FooDecl", "foo", MAX_NAME_LENGTH)])),
},
test_identifier_dynamic_name_too_long => {
check_fn = check_dynamic_name,
input = &format!("{}", "a".repeat(MAX_DYNAMIC_NAME_LENGTH + 1)),
- result = Err(ErrorList::new(vec![Error::field_too_long("FooDecl", "foo")])),
+ result = Err(ErrorList::new(vec![Error::field_too_long_with_max("FooDecl", "foo", MAX_DYNAMIC_NAME_LENGTH)])),
},
// url
@@ -482,7 +482,7 @@
test_identifier_url_too_long => {
check_fn = check_url,
input = &format!("fuchsia-pkg://{}", "a".repeat(4083)),
- result = Err(ErrorList::new(vec![Error::field_too_long("FooDecl", "foo")])),
+ result = Err(ErrorList::new(vec![Error::field_too_long_with_max("FooDecl", "foo", 4096)])),
},
}
}
