Refactor handling of non-fatal errors, including warnings.

crates/weaver_codegen_test/build.rs

@@ -45,6 +45,8 @@ fn main() {
     let registry_repo =
         RegistryRepo::try_new("main", &registry_path).unwrap_or_else(|e| process_error(&logger, e));
     let semconv_specs = SchemaResolver::load_semconv_specs(&registry_repo)
+        .ignore_severity_warnings()
+        .into_result_failing_non_fatal()
         .unwrap_or_else(|e| process_error(&logger, e));
     let mut registry = SemConvRegistry::from_semconv_specs(REGISTRY_ID, semconv_specs);
     let schema = SchemaResolver::resolve_semantic_convention_registry(&mut registry)

crates/weaver_common/src/diagnostic.rs

@@ -116,6 +116,12 @@ impl DiagnosticMessage {
             diagnostic,
         }
     }
+
+    /// Returns true if the diagnostic message is a warning
+    #[must_use]
+    pub fn is_warning(&self) -> bool {
+        self.diagnostic.severity == Some(Severity::Warning)
+    }
 }
 
 impl DiagnosticMessages {
@@ -137,6 +143,12 @@ impl DiagnosticMessages {
         self.0.extend(diag_msgs.0);
     }
 
+    /// Extends the current `DiagnosticMessages` with the provided
+    /// `Vec<DiagnosticMessage>`.
+    pub fn extend_from_vec(&mut self, diag_msgs: Vec<DiagnosticMessage>) {
+        self.0.extend(diag_msgs);
+    }
+
     /// Logs all the diagnostic messages
     pub fn log(&self, logger: impl Logger) {
         self.0

crates/weaver_common/src/lib.rs

@@ -6,6 +6,7 @@ pub mod diagnostic;
 pub mod error;
 pub mod in_memory;
 pub mod quiet;
+pub mod result;
 
 use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::sync::{Arc, Mutex};

crates/weaver_common/src/result.rs

@@ -0,0 +1,230 @@
+// SPDX-License-Identifier: Apache-2.0
+
+//! Weaver Result type supporting both non-fatal errors (NFEs) and fatal errors.
+//!
+//! NFEs do not prevent the next operations from completing successfully. For example,
+//! if a semconv file is invalid, we generate a non-fatal error and continue processing
+//! the other files.
+//!
+//! NFEs in Weaver are standard Rust errors.
+
+use crate::diagnostic::{DiagnosticMessage, DiagnosticMessages};
+use crate::error::WeaverError;
+use miette::Diagnostic;
+use serde::Serialize;
+use std::error::Error;
+
+/// Weaver Result type supporting both non-fatal errors (NFEs) and fatal errors.
+#[must_use]
+pub enum WResult<T, E> {
+    /// The operation was successful, the result T is returned.
+    Ok(T),
+    /// The operation was successful, the result T is returned along with
+    /// a list of non-fatal errors.
+    OkWithNFEs(T, Vec<E>),
+    /// The operation failed with a fatal error. By definition, we can only have
+    /// one fatal error.
+    FatalErr(E),
+}
+
+impl<T, E> WResult<T, E>
+where
+    E: WeaverError<E> + Error + Diagnostic + Serialize + Send + Sync + 'static,
+{
+    /// Creates a new [`WResult`] with a successful result.
+    pub fn with_non_fatal_errors(result: T, non_fatal_errors: Vec<E>) -> Self {
+        if non_fatal_errors.is_empty() {
+            WResult::Ok(result)
+        } else {
+            WResult::OkWithNFEs(result, non_fatal_errors)
+        }
+    }
+
+    /// Converts a [`WResult`] into a standard [`Result`], optionally capturing non-fatal errors.
+    pub fn capture_non_fatal_errors(self, diag_msgs: &mut DiagnosticMessages) -> Result<T, E> {
+        match self {
+            WResult::Ok(result) => Ok(result),
+            WResult::OkWithNFEs(result, nfes) => {
+                let msgs = nfes.into_iter().map(DiagnosticMessage::new).collect();
+                diag_msgs.extend_from_vec(msgs);
+                Ok(result)
+            }
+            WResult::FatalErr(fatal_err) => Err(fatal_err),
+        }
+    }
+
+    /// Capture the warnings into the provided vector and return a [`WResult`]
+    /// without the warnings.
+    pub fn capture_warnings(self, diag_msgs: &mut DiagnosticMessages) -> WResult<T, E> {
+        if let WResult::OkWithNFEs(result, nfes) = self {
+            let (warnings, errors): (Vec<_>, Vec<_>) = nfes
+                .into_iter()
+                .partition(|e| matches!(e.severity(), Some(miette::Severity::Warning)));
+            let warnings: Vec<_> = warnings.into_iter().map(DiagnosticMessage::new).collect();
+            diag_msgs.extend_from_vec(warnings);
+            if errors.is_empty() {
+                WResult::Ok(result)
+            } else {
+                WResult::OkWithNFEs(result, errors)
+            }
+        } else {
+            self
+        }
+    }
+
+    /// Return a [`WResult`] without the non-fatal errors with severity=warning.
+    pub fn ignore_severity_warnings(self) -> WResult<T, E> {
+        match self {
+            WResult::OkWithNFEs(result, non_fatal_errors) => {
+                // Remove warnings from the non-fatal errors.
+                let errors: Vec<_> = non_fatal_errors
+                    .into_iter()
+                    .filter(|e| !matches!(e.severity(), Some(miette::Severity::Warning)))
+                    .collect();
+                if errors.is_empty() {
+                    WResult::Ok(result)
+                } else {
+                    WResult::OkWithNFEs(result, errors)
+                }
+            }
+            _ => self,
+        }
+    }
+
+    /// Calls a function with a reference to the contained value if [`Ok`].
+    ///
+    /// Returns the original result.
+    pub fn inspect<F: FnOnce(&T, Option<&[E]>)>(self, f: F) -> Self {
+        match &self {
+            WResult::Ok(result) => f(result, None),
+            WResult::OkWithNFEs(result, nfes) => f(result, Some(nfes)),
+            WResult::FatalErr(_) => {}
+        }
+
+        self
+    }
+
+    /// Converts a [`WResult`] into a standard [`Result`], potentially
+    /// aggregating non-fatal errors into a single error.
+    pub fn into_result_failing_non_fatal(self) -> Result<T, E> {
+        match self {
+            WResult::Ok(result) => Ok(result),
+            WResult::OkWithNFEs(result, errors) => {
+                if errors.is_empty() {
+                    Ok(result)
+                } else if errors.len() == 1 {
+                    Err(errors
+                        .into_iter()
+                        .next()
+                        .expect("should never happen as we checked the length"))
+                } else {
+                    let compound_error = E::compound(errors);
+                    Err(compound_error)
+                }
+            }
+            WResult::FatalErr(e) => Err(e),
+        }
+    }
+
+    /// Converts a [`WResult`] into a standard [`Result`], returning the result
+    /// alongside any non-fatal errors.
+    pub fn into_result_with_non_fatal(self) -> Result<(T, Vec<E>), E> {
+        match self {
+            WResult::Ok(result) => Ok((result, Vec::new())),
+            WResult::OkWithNFEs(result, errors) => Ok((result, errors)),
+            WResult::FatalErr(e) => Err(e),
+        }
+    }
+
+    /// Maps a [`WResult<T, E>`] to [`WResult<U, E>`] by applying a function to a
+    /// contained [`Ok`] value, leaving an [`Err`] value untouched.
+    pub fn map<U, F: FnOnce(T) -> U>(self, op: F) -> WResult<U, E> {
+        match self {
+            WResult::Ok(t) => WResult::Ok(op(t)),
+            WResult::OkWithNFEs(t, nfes) => WResult::OkWithNFEs(op(t), nfes),
+            WResult::FatalErr(err) => WResult::FatalErr(err),
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::diagnostic::DiagnosticMessages;
+    use crate::error::WeaverError;
+    use crate::result::WResult;
+    use miette::Diagnostic;
+    use serde::Serialize;
+
+    #[derive(thiserror::Error, Debug, PartialEq, Serialize, Diagnostic)]
+    enum TestError {
+        #[error("Warning")]
+        #[diagnostic(severity(Warning))]
+        Warning,
+        #[error("Error")]
+        Error,
+        #[error("Compound error")]
+        CompoundError(Vec<TestError>),
+    }
+
+    impl WeaverError<TestError> for TestError {
+        fn compound(errors: Vec<TestError>) -> Self {
+            TestError::CompoundError(errors)
+        }
+    }
+
+    #[test]
+    fn test_werror() -> Result<(), TestError> {
+        let mut diag_msgs = DiagnosticMessages::empty();
+        let result: Result<i32, TestError> = WResult::Ok(42)
+            .inspect(|r, _| assert_eq!(*r, 42))
+            .capture_warnings(&mut diag_msgs)
+            .into_result_failing_non_fatal();
+
+        assert_eq!(result, Ok(42));
+        assert_eq!(diag_msgs.len(), 0);
+
+        let mut diag_msgs = DiagnosticMessages::empty();
+        let result: Result<i32, TestError> = WResult::Ok(42)
+            .inspect(|r, _| assert_eq!(*r, 42))
+            .capture_warnings(&mut diag_msgs)
+            .into_result_failing_non_fatal();
+
+        assert_eq!(result, Ok(42));
+        assert_eq!(diag_msgs.len(), 0);
+
+        let mut diag_msgs = DiagnosticMessages::empty();
+        let result: Result<i32, TestError> =
+            WResult::OkWithNFEs(42, vec![TestError::Warning, TestError::Error])
+                .inspect(|r, nfes| {
+                    assert_eq!(*r, 42);
+                    assert_eq!(nfes.unwrap().len(), 2);
+                })
+                .capture_warnings(&mut diag_msgs)
+                .into_result_failing_non_fatal();
+
+        assert_eq!(result, Err(TestError::Error));
+        assert_eq!(diag_msgs.len(), 1);
+
+        let mut diag_msgs = DiagnosticMessages::empty();
+        let result = WResult::OkWithNFEs(42, vec![TestError::Warning, TestError::Error])
+            .inspect(|r, nfes| {
+                assert_eq!(*r, 42);
+                assert_eq!(nfes.unwrap().len(), 2);
+            })
+            .capture_non_fatal_errors(&mut diag_msgs)?;
+
+        assert_eq!(result, 42);
+        assert_eq!(diag_msgs.len(), 2);
+
+        let (result, nfes) = WResult::OkWithNFEs(42, vec![TestError::Warning, TestError::Error])
+            .inspect(|r, nfes| {
+                assert_eq!(*r, 42);
+                assert_eq!(nfes.unwrap().len(), 2);
+            })
+            .into_result_with_non_fatal()?;
+        assert_eq!(result, 42);
+        assert_eq!(nfes.len(), 2);
+
+        Ok(())
+    }
+}

crates/weaver_forge/README.md

@@ -856,7 +856,7 @@ In addition, OTel Weaver provides the following custom function:
   prefixed with "Notes: ". If the attribute note is not defined then the comment will
   only contain the brief description without the prefix "Notes: ".
 
-  `{{ [attr.brief, concat_if("\n\nNotes: ", attr.note) | comment }}`
+  `{{ [attr.brief, concat_if("\n\nNotes: ", attr.note)] | comment }}`
 
 ### Jinja Tests Reference
 

crates/weaver_forge/src/lib.rs

@@ -753,6 +753,7 @@ mod tests {
     ) {
         let registry_id = "default";
         let registry = SemConvRegistry::try_from_path_pattern(registry_id, "data/*.yaml")
+            .into_result_failing_non_fatal()
             .expect("Failed to load registry");
         prepare_test_with_registry(target, cli_params, registry_id, registry)
     }
@@ -949,6 +950,7 @@ mod tests {
 
         let registry_id = "default";
         let mut registry = SemConvRegistry::try_from_path_pattern(registry_id, "data/*.yaml")
+            .into_result_failing_non_fatal()
             .expect("Failed to load registry");
         let schema = SchemaResolver::resolve_semantic_convention_registry(&mut registry)
             .expect("Failed to resolve registry");
@@ -1084,6 +1086,7 @@ mod tests {
             registry_id,
             "data/mini_registry_for_comments/*.yaml",
         )
+        .into_result_failing_non_fatal()
         .expect("Failed to load registry");
         let (logger, engine, template_registry, observed_output, expected_output) =
             prepare_test_with_registry("comment_format", Params::default(), registry_id, registry);