🥅 - Don't panic when failing to parse bsconfig

rolandpeelen · rolandpeelen · commit 9fabbb9b3160 · 2024-11-20T10:59:59.000+01:00
diff --git a/src/build.rs b/src/build.rs
@@ -13,6 +13,7 @@ use crate::helpers::emojis::*;
 use crate::helpers::{self, get_workspace_root};
 use crate::sourcedirs;
 use ahash::AHashSet;
+use anyhow::{anyhow, Result};
 use build_types::*;
 use console::style;
 use indicatif::{ProgressBar, ProgressStyle};
@@ -54,15 +55,19 @@ pub struct CompilerArgs {
     pub parser_args: Vec<String>,
 }
 
-pub fn get_compiler_args(path: &str, rescript_version: Option<String>, bsc_path: Option<String>) -> String {
+pub fn get_compiler_args(
+    path: &str,
+    rescript_version: Option<String>,
+    bsc_path: Option<String>,
+) -> Result<String> {
     let filename = &helpers::get_abs_path(path);
     let package_root = helpers::get_abs_path(
         &helpers::get_nearest_config(&std::path::PathBuf::from(path)).expect("Couldn't find package root"),
     );
     let workspace_root = get_workspace_root(&package_root).map(|p| helpers::get_abs_path(&p));
     let root_rescript_config =
-        packages::read_config(&workspace_root.to_owned().unwrap_or(package_root.to_owned()));
-    let rescript_config = packages::read_config(&package_root);
+        packages::read_config(&workspace_root.to_owned().unwrap_or(package_root.to_owned()))?;
+    let rescript_config = packages::read_config(&package_root)?;
     let rescript_version = if let Some(rescript_version) = rescript_version {
         rescript_version
     } else {
@@ -111,28 +116,13 @@ pub fn get_compiler_args(path: &str, rescript_version: Option<String>, bsc_path:
         &workspace_root,
         &None,
     );
-    serde_json::to_string_pretty(&CompilerArgs {
+
+    let result = serde_json::to_string_pretty(&CompilerArgs {
         compiler_args,
         parser_args,
-    })
-    .unwrap()
-}
+    })?;
 
-#[derive(Debug, Clone)]
-pub enum InitializeBuildError {
-    PackageDependencyValidation,
-}
-
-impl fmt::Display for InitializeBuildError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self {
-            Self::PackageDependencyValidation => write!(
-                f,
-                "{}  {}Could not Validate Package Dependencies",
-                LINE_CLEAR, CROSS,
-            ),
-        }
-    }
+    Ok(result)
 }
 
 pub fn initialize_build(
@@ -141,14 +131,14 @@ pub fn initialize_build(
     show_progress: bool,
     path: &str,
     bsc_path: Option<String>,
-) -> Result<BuildState, InitializeBuildError> {
+) -> Result<BuildState> {
     let project_root = helpers::get_abs_path(path);
     let workspace_root = helpers::get_workspace_root(&project_root);
     let bsc_path = match bsc_path {
         Some(bsc_path) => bsc_path,
         None => helpers::get_bsc(&project_root, workspace_root.to_owned()),
     };
-    let root_config_name = packages::get_package_name(&project_root);
+    let root_config_name = packages::get_package_name(&project_root)?;
     let rescript_version = helpers::get_rescript_version(&bsc_path);
 
     if show_progress {
@@ -157,7 +147,7 @@ pub fn initialize_build(
     }
 
     let timing_package_tree = Instant::now();
-    let packages = packages::make(filter, &project_root, &workspace_root);
+    let packages = packages::make(filter, &project_root, &workspace_root, show_progress)?;
     let timing_package_tree_elapsed = timing_package_tree.elapsed();
 
     if show_progress {
@@ -173,7 +163,7 @@ pub fn initialize_build(
     }
 
     if !packages::validate_packages_dependencies(&packages) {
-        return Err(InitializeBuildError::PackageDependencyValidation);
+        return Err(anyhow!("Failed to validate package dependencies"));
     }
 
     let timing_source_files = Instant::now();
@@ -435,27 +425,6 @@ pub fn incremental_build(
     }
 }
 
-#[derive(Debug, Clone)]
-pub enum BuildError {
-    InitializeBuild(InitializeBuildError),
-    IncrementalBuild(IncrementalBuildError),
-}
-
-impl fmt::Display for BuildError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self {
-            Self::InitializeBuild(e) => {
-                write!(f, "{}  {}Error Initializing Build: {}", LINE_CLEAR, CROSS, e)
-            }
-            Self::IncrementalBuild(e) => write!(
-                f,
-                "{}  {}Error Running Incremental Build: {}",
-                LINE_CLEAR, CROSS, e
-            ),
-        }
-    }
-}
-
 // write build.ninja files in the packages after a non-incremental build
 // this is necessary to bust the editor tooling cache. The editor tooling
 // is watching this file.
@@ -477,15 +446,15 @@ pub fn build(
     no_timing: bool,
     create_sourcedirs: bool,
     bsc_path: Option<String>,
-) -> Result<BuildState, BuildError> {
+) -> Result<BuildState> {
     let default_timing: Option<std::time::Duration> = if no_timing {
         Some(std::time::Duration::new(0.0 as u64, 0.0 as u32))
     } else {
         None
     };
     let timing_total = Instant::now();
     let mut build_state = initialize_build(default_timing, filter, show_progress, path, bsc_path)
-        .map_err(BuildError::InitializeBuild)?;
+        .map_err(|e| anyhow!("Could not initialize build. Error: {e}"))?;
 
     match incremental_build(
         &mut build_state,
@@ -512,7 +481,7 @@ pub fn build(
         Err(e) => {
             clean::cleanup_after_build(&build_state);
             write_build_ninja(&build_state);
-            Err(BuildError::IncrementalBuild(e))
+            Err(anyhow!("Incremental build failed. Error: {e}"))
         }
     }
 }
diff --git a/src/build/clean.rs b/src/build/clean.rs
@@ -3,6 +3,7 @@ use super::packages;
 use crate::helpers;
 use crate::helpers::emojis::*;
 use ahash::AHashSet;
+use anyhow::Result;
 use console::style;
 use rayon::prelude::*;
 use std::io::Write;
@@ -318,11 +319,11 @@ pub fn cleanup_after_build(build_state: &BuildState) {
     });
 }
 
-pub fn clean(path: &str, show_progress: bool, bsc_path: Option<String>) {
+pub fn clean(path: &str, show_progress: bool, bsc_path: Option<String>) -> Result<()> {
     let project_root = helpers::get_abs_path(path);
     let workspace_root = helpers::get_workspace_root(&project_root);
-    let packages = packages::make(&None, &project_root, &workspace_root);
-    let root_config_name = packages::get_package_name(&project_root);
+    let packages = packages::make(&None, &project_root, &workspace_root, show_progress)?;
+    let root_config_name = packages::get_package_name(&project_root)?;
     let bsc_path = match bsc_path {
         Some(bsc_path) => bsc_path,
         None => helpers::get_bsc(&project_root, workspace_root.to_owned()),
@@ -399,4 +400,6 @@ pub fn clean(path: &str, show_progress: bool, bsc_path: Option<String>) {
         );
         let _ = std::io::stdout().flush();
     }
+
+    Ok(())
 }
diff --git a/src/build/packages.rs b/src/build/packages.rs
@@ -5,6 +5,7 @@ use crate::config;
 use crate::helpers;
 use crate::helpers::emojis::*;
 use ahash::{AHashMap, AHashSet};
+use anyhow::Result;
 use console::style;
 use log::{debug, error};
 use rayon::prelude::*;
@@ -216,7 +217,7 @@ fn get_source_dirs(source: config::Source, sub_path: Option<PathBuf>) -> AHashSe
     source_folders
 }
 
-pub fn read_config(package_dir: &str) -> config::Config {
+pub fn read_config(package_dir: &str) -> Result<config::Config> {
     let prefix = if package_dir.is_empty() {
         "".to_string()
     } else {
@@ -290,6 +291,7 @@ fn read_dependencies(
     parent_path: &str,
     project_root: &str,
     workspace_root: Option<String>,
+    show_progress: bool,
 ) -> Vec<Dependency> {
     return parent_config
         .bs_dependencies
@@ -311,16 +313,34 @@ fn read_dependencies(
             let (config, canonical_path) =
                 match read_dependency(package_name, parent_path, project_root, &workspace_root) {
                     Err(error) => {
-                        log::error!(
+                    if show_progress {
+                        println!(
                             "{} {} Error building package tree. {}",
                             style("[1/2]").bold().dim(),
                             CROSS,
                             error
                         );
+                    }
+
+                        log::error!(
+                            "We could not build package tree reading depencency '{package_name}', at path '{parent_path}'. Error: {error}",
+                        );
+
                         std::process::exit(2)
                     }
-                    Ok(canonical_path) => (read_config(&canonical_path), canonical_path),
+                    Ok(canonical_path) => {
+                        match read_config(&canonical_path) {
+                            Ok(config) => (config, canonical_path),
+                            Err(error) => {
+                                log::error!(
+                                    "We could not build package tree  '{package_name}', at path '{parent_path}', Error: {error}",
+                                );
+                                std::process::exit(2)
+                                    }
+                                }
+                        }
                 };
+
             let is_pinned = parent_config
                 .pinned_dependencies
                 .as_ref()
@@ -333,6 +353,7 @@ fn read_dependencies(
                 &canonical_path,
                 project_root,
                 workspace_root.to_owned(),
+                show_progress
             );
 
             Dependency {
@@ -397,8 +418,12 @@ fn make_package(config: config::Config, package_path: &str, is_pinned_dep: bool,
     }
 }
 
-fn read_packages(project_root: &str, workspace_root: Option<String>) -> AHashMap<String, Package> {
-    let root_config = read_config(project_root);
+fn read_packages(
+    project_root: &str,
+    workspace_root: Option<String>,
+    show_progress: bool,
+) -> Result<AHashMap<String, Package>> {
+    let root_config = read_config(project_root)?;
 
     // Store all packages and completely deduplicate them
     let mut map: AHashMap<String, Package> = AHashMap::new();
@@ -414,6 +439,7 @@ fn read_packages(project_root: &str, workspace_root: Option<String>) -> AHashMap
         project_root,
         project_root,
         workspace_root,
+        show_progress,
     ));
     dependencies.iter().for_each(|d| {
         if !map.contains_key(&d.name) {
@@ -424,7 +450,7 @@ fn read_packages(project_root: &str, workspace_root: Option<String>) -> AHashMap
         }
     });
 
-    map
+    Ok(map)
 }
 
 /// `get_source_files` is essentially a wrapper around `read_structure`, which read a
@@ -527,25 +553,29 @@ pub fn make(
     filter: &Option<regex::Regex>,
     root_folder: &str,
     workspace_root: &Option<String>,
-) -> AHashMap<String, Package> {
-    let map = read_packages(root_folder, workspace_root.to_owned());
+    show_progress: bool,
+) -> Result<AHashMap<String, Package>> {
+    let map = read_packages(root_folder, workspace_root.to_owned(), show_progress)?;
 
     /* Once we have the deduplicated packages, we can add the source files for each - to minimize
      * the IO */
     let result = extend_with_children(filter, map);
+
     result.values().for_each(|package| {
         if let Some(dirs) = &package.dirs {
             dirs.iter().for_each(|dir| {
                 let _ = std::fs::create_dir_all(std::path::Path::new(&package.get_bs_build_path()).join(dir));
             })
         }
     });
-    result
+
+    Ok(result)
 }
 
-pub fn get_package_name(path: &str) -> String {
-    let config = read_config(path);
-    config.name
+pub fn get_package_name(path: &str) -> Result<String> {
+    let config = read_config(path)?;
+
+    Ok(config.name)
 }
 
 pub fn parse_packages(build_state: &mut BuildState) {
diff --git a/src/config.rs b/src/config.rs
@@ -1,5 +1,6 @@
 use crate::build::packages;
 use crate::helpers::deserialize::*;
+use anyhow::Result;
 use convert_case::{Case, Casing};
 use serde::Deserialize;
 use std::fs;
@@ -267,17 +268,11 @@ pub fn flatten_ppx_flags(
 }
 
 /// Try to convert a bsconfig from a certain path to a bsconfig struct
-pub fn read(path: String) -> Config {
-    fs::read_to_string(path.clone())
-        .map_err(|e| format!("Could not read bsconfig. {path} - {e}"))
-        // .and_then(|x| {
-        //     dbg!(&x);
-        //     repair(x).map_err(|e| format!("Json was invalid and could not be repaired. {path} - {e}"))
-        // })
-        .and_then(|x| {
-            serde_json::from_str::<Config>(&x).map_err(|e| format!("Could not parse bsconfig. {path} - {e}"))
-        })
-        .expect("Errors reading bsconfig")
+pub fn read(path: String) -> Result<Config> {
+    let read = fs::read_to_string(path.clone())?;
+    let parse = serde_json::from_str::<Config>(&read)?;
+
+    Ok(parse)
 }
 
 fn check_if_rescript11_or_higher(version: &str) -> Result<bool, String> {
diff --git a/src/main.rs b/src/main.rs
@@ -1,3 +1,4 @@
+use anyhow::Result;
 use clap::{Parser, ValueEnum};
 use clap_verbosity_flag::InfoLevel;
 use log::LevelFilter;
@@ -72,7 +73,7 @@ struct Args {
     bsc_path: Option<String>,
 }
 
-fn main() {
+fn main() -> Result<()> {
     let args = Args::parse();
     let log_level_filter = args.verbose.log_level_filter();
 
@@ -93,7 +94,7 @@ fn main() {
         Some(path) => {
             println!(
                 "{}",
-                build::get_compiler_args(&path, args.rescript_version, args.bsc_path)
+                build::get_compiler_args(&path, args.rescript_version, args.bsc_path)?
             );
             std::process::exit(0);
         }
@@ -139,6 +140,8 @@ fn main() {
                     args.after_build,
                     args.create_sourcedirs.unwrap_or(false),
                 );
+
+                Ok(())
             }
         },
     }
