Fix author line parsing and updating

oxidecomputer · Jan 7, 2025 · 0958b2c · 0958b2c
1 parent 152e39e
commit 0958b2c
Show file tree

Hide file tree

Showing 13 changed files with 692 additions and 194 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs
@@ -11,7 +11,7 @@ use octorust::{
 };
 use partial_struct::partial;
 use rfd_data::{
-    content::{RfdContent, RfdDocument, RfdTemplate, TemplateError},
+    content::{RfdContent, RfdContentError, RfdDocument, RfdTemplate, TemplateError},
     RfdNumber,
 };
 use rfd_github::{GitHubError, GitHubNewRfdNumber, GitHubRfdRepo};
@@ -84,6 +84,8 @@ pub enum UpdateRfdContentError {
     GitHub(#[from] GitHubError),
     #[error("Internal GitHub state does not currently allow for update. This commit appears as the head commit on multiple branches.")]
     InternalState,
+    #[error("Unable to parse RFD contents")]
+    InvalidContent(#[from] RfdContentError),
     #[error("Failed to construct new RFD template")]
     InvalidTemplate(#[from] TemplateError),
     #[error("Unable to perform action. Unable to find the default branch on GitHub.")]
@@ -623,8 +625,14 @@ impl RfdContext {
                 .map_err(|err| err.inner_into())?;
 
             let sha = latest_revision.commit.clone();
-            let mut updated_content: RfdContent = latest_revision.into();
-            updated_content.update_body(content);
+            let mut updated_content: RfdContent = latest_revision
+                .try_into()
+                .map_err(UpdateRfdContentError::InvalidContent)
+                .to_resource_result()?;
+            updated_content
+                .update_body(content)
+                .map_err(UpdateRfdContentError::InvalidContent)
+                .to_resource_result()?;
 
             self.commit_rfd_document(
                 caller,

diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs
@@ -409,7 +409,13 @@ async fn view_rfd_attr_op(
         let rfd = ctx.view_rfd(caller, rfd_number, None).await?;
         let content = match (rfd.content, rfd.format) {
             (Some(content), Some(ContentFormat::Asciidoc)) => {
-                RfdContent::Asciidoc(RfdAsciidoc::new(content))
+                RfdContent::Asciidoc(RfdAsciidoc::new(content).map_err(|err| {
+                    tracing::warn!(?err, "Failed to parse RFD content");
+                    HttpError::for_internal_error(format!(
+                        "Failed to parse RFD content for RFD {}",
+                        number
+                    ))
+                })?)
             }
             (Some(content), Some(ContentFormat::Markdown)) => {
                 RfdContent::Markdown(RfdMarkdown::new(content))
@@ -688,7 +694,15 @@ async fn set_rfd_attr_op(
 
         // TODO: Get rid of these clones
         let mut content = match revision.content_format {
-            ContentFormat::Asciidoc => RfdContent::Asciidoc(RfdAsciidoc::new(revision.content)),
+            ContentFormat::Asciidoc => {
+                RfdContent::Asciidoc(RfdAsciidoc::new(revision.content).map_err(|err| {
+                    tracing::warn!(?err, "Failed to parse RFD content");
+                    HttpError::for_internal_error(format!(
+                        "Failed to parse RFD content for RFD {}",
+                        number
+                    ))
+                })?)
+            }
             ContentFormat::Markdown => RfdContent::Markdown(RfdMarkdown::new(revision.content)),
         };
 
@@ -701,9 +715,16 @@ async fn set_rfd_attr_op(
                     tracing::info!(?err, "Invalid state was supplied");
                     HttpError::for_bad_request(None, "Invalid RFD state".to_string())
                 })?;
-                content.update_state(&state.to_string());
+                content.update_state(&state.to_string())
             }
-        };
+        }
+        .map_err(|err| {
+            tracing::info!(?err, "Update resulted in malfored RFD");
+            HttpError::for_bad_request(
+                None,
+                "Update would result in a malformed RFD. Update has not been applied".to_string(),
+            )
+        })?;
 
         tracing::info!("Updated attribute in RFD document");
 

diff --git a/rfd-data/Cargo.toml b/rfd-data/Cargo.toml
@@ -11,3 +11,4 @@ rfd-model = { path = "../rfd-model" }
 schemars = { workspace = true }
 serde = { workspace = true }
 thiserror = { workspace = true }
+tracing = { workspace = true }