Fix error handling when rescript.json parsing fails (#8067)

* Fix error handling when rescript.json parsing fails

* Add key path to error message

* CHANGELOG

* CHANGELOG

Author: cknitt

CHANGELOG.md

@@ -24,6 +24,7 @@
 - Fix missing `ignore` function in some Stdlib modules. https://github.com/rescript-lang/rescript/pull/8060
 - Fix signature matching for externals when abstract alias hides function arity. https://github.com/rescript-lang/rescript/pull/8045
 - Fix arity detection for arrows returning nested generics. https://github.com/rescript-lang/rescript/pull/8064
+- Fix error handling when rescript.json parsing fails and improve error message. https://github.com/rescript-lang/rescript/pull/8067
 
 #### :memo: Documentation
 

rewatch/Cargo.lock

@@ -25,6 +25,7 @@ regex = "1.7.1"
 serde = { version = "1.0.152", features = ["derive"] }
 serde_json = { version = "1.0.93" }
 serde_ignored = "0.1.11"
+serde_path_to_error = "0.1.16"
 sysinfo = "0.29.10"
 tempfile = "3.10.1"
 

rewatch/Cargo.toml

@@ -16,7 +16,7 @@ use crate::helpers::emojis::*;
 use crate::helpers::{self};
 use crate::project_context::ProjectContext;
 use crate::{config, sourcedirs};
-use anyhow::{Result, anyhow};
+use anyhow::{Context, Result, anyhow};
 use build_types::*;
 use console::style;
 use indicatif::{ProgressBar, ProgressStyle};
@@ -501,7 +501,7 @@ pub fn build(
         plain_output,
         warn_error,
     )
-    .map_err(|e| anyhow!("Could not initialize build. Error: {e}"))?;
+    .with_context(|| "Could not initialize build")?;
 
     match incremental_build(
         &mut build_state,

rewatch/src/build.rs

@@ -2,7 +2,7 @@ use crate::build::packages;
 use crate::helpers;
 use crate::helpers::deserialize::*;
 use crate::project_context::ProjectContext;
-use anyhow::{Result, bail};
+use anyhow::{Result, anyhow, bail};
 use convert_case::{Case, Casing};
 use serde::de::{Error as DeError, Visitor};
 use serde::{Deserialize, Deserializer};
@@ -440,10 +440,19 @@ impl Config {
     /// Try to convert a bsconfig from a string to a bsconfig struct
     pub fn new_from_json_string(config_str: &str) -> Result<Self> {
         let mut deserializer = serde_json::Deserializer::from_str(config_str);
+        let mut tracker = serde_path_to_error::Track::new();
+        let path_deserializer = serde_path_to_error::Deserializer::new(&mut deserializer, &mut tracker);
         let mut unknown_fields = Vec::new();
-        let mut config: Config = serde_ignored::deserialize(&mut deserializer, |path| {
-            unknown_fields.push(path.to_string());
-        })?;
+        let mut config: Config =
+            serde_ignored::deserialize(path_deserializer, |path| unknown_fields.push(path.to_string()))
+                .map_err(|err: serde_json::Error| {
+                    let path = tracker.path().to_string();
+                    if path.is_empty() {
+                        anyhow!("Failed to parse rescript.json: {err}")
+                    } else {
+                        anyhow!("Failed to parse rescript.json at {path}: {err}")
+                    }
+                })?;
 
         config.handle_deprecations()?;
         config.unknown_fields = unknown_fields;

rewatch/src/config.rs

@@ -56,7 +56,7 @@ fn main() -> Result<()> {
                 (*build_args.warn_error).clone(),
             ) {
                 Err(e) => {
-                    println!("{e}");
+                    println!("{:#}", e);
                     std::process::exit(1)
                 }
                 Ok(_) => {
@@ -76,17 +76,21 @@ fn main() -> Result<()> {
                 );
             }
 
-            watcher::start(
+            match watcher::start(
                 &watch_args.filter,
                 show_progress,
                 &watch_args.folder,
                 (*watch_args.after_build).clone(),
                 *watch_args.create_sourcedirs,
                 plain_output,
                 (*watch_args.warn_error).clone(),
-            );
-
-            Ok(())
+            ) {
+                Err(e) => {
+                    println!("{:#}", e);
+                    std::process::exit(1)
+                }
+                Ok(_) => Ok(()),
+            }
         }
         cli::Command::Clean { folder, dev } => {
             let _lock = get_lock(&folder);

rewatch/src/main.rs

@@ -2,8 +2,8 @@ use crate::build::packages;
 use crate::config::Config;
 use crate::helpers;
 use ahash::{AHashMap, AHashSet};
-use anyhow::Result;
 use anyhow::anyhow;
+use anyhow::{Context, Result};
 use log::debug;
 use std::fmt;
 use std::path::{Path, PathBuf};
@@ -168,7 +168,7 @@ impl ProjectContext {
     pub fn new(path: &Path) -> Result<ProjectContext> {
         let path = helpers::get_abs_path(path);
         let current_config = packages::read_config(&path)
-            .map_err(|_| anyhow!("Could not read rescript.json at {}", path.to_string_lossy()))?;
+            .with_context(|| format!("Could not read rescript.json at {}", path.to_string_lossy()))?;
         let nearest_parent_config_path = match path.parent() {
             None => Err(anyhow!(
                 "The current path \"{}\" does not have a parent folder",

rewatch/src/project_context.rs

@@ -8,6 +8,7 @@ use crate::helpers::emojis::*;
 use crate::lock::LOCKFILE;
 use crate::queue::FifoQueue;
 use crate::queue::*;
+use anyhow::{Context, Result};
 use futures_timer::Delay;
 use notify::event::ModifyKind;
 use notify::{Config, Error, Event, EventKind, RecommendedWatcher, RecursiveMode, Watcher};
@@ -78,10 +79,10 @@ async fn async_watch(
         plain_output,
         warn_error,
     }: AsyncWatchArgs<'_>,
-) -> notify::Result<()> {
+) -> Result<()> {
     let mut build_state: build::build_types::BuildCommandState =
         build::initialize_build(None, filter, show_progress, path, plain_output, warn_error)
-            .expect("Can't initialize build");
+            .with_context(|| "Could not initialize build")?;
     let mut needs_compile_type = CompileType::Incremental;
     // create a mutex to capture if ctrl-c was pressed
     let ctrlc_pressed = Arc::new(Mutex::new(false));
@@ -278,7 +279,7 @@ async fn async_watch(
                     plain_output,
                     build_state.get_warn_error_override(),
                 )
-                .expect("Can't initialize build");
+                .expect("Could not initialize build");
                 let _ = build::incremental_build(
                     &mut build_state,
                     None,
@@ -324,7 +325,7 @@ pub fn start(
     create_sourcedirs: bool,
     plain_output: bool,
     warn_error: Option<String>,
-) {
+) -> Result<()> {
     futures::executor::block_on(async {
         let queue = Arc::new(FifoQueue::<Result<Event, Error>>::new());
         let producer = queue.clone();
@@ -341,7 +342,7 @@ pub fn start(
 
         let path = Path::new(folder);
 
-        if let Err(e) = async_watch(AsyncWatchArgs {
+        async_watch(AsyncWatchArgs {
             q: consumer,
             path,
             show_progress,
@@ -352,8 +353,5 @@ pub fn start(
             warn_error: warn_error.clone(),
         })
         .await
-        {
-            println!("{e:?}")
-        }
     })
 }

rewatch/src/watcher.rs

@@ -38,3 +38,88 @@ fi
 mv rescript.json.bak rescript.json
 
 success "experimental-features emits -enable-experimental twice as string list"
+
+bold "Test: build surfaces experimental-features list parse error"
+
+cp rescript.json rescript.json.bak
+
+node -e '
+const fs = require("fs");
+const j = JSON.parse(fs.readFileSync("rescript.json", "utf8"));
+j["experimental-features"] = ["LetUnwrap"];
+fs.writeFileSync("rescript.json", JSON.stringify(j, null, 2));
+'
+
+out=$(rewatch build 2>&1)
+status=$?
+
+mv rescript.json.bak rescript.json
+rm -f lib/rescript.lock
+
+if [ $status -eq 0 ]; then
+  error "Expected build to fail for experimental-features list input"
+  echo "$out"
+  exit 1
+fi
+
+echo "$out" | grep -q "Could not read rescript.json"
+if [ $? -ne 0 ]; then
+  error "Missing rescript.json path context in build error"
+  echo "$out"
+  exit 1
+fi
+
+echo "$out" | grep -qi "experimental-features.*invalid type"
+if [ $? -ne 0 ]; then
+  error "Missing detailed parse error in build output"
+  echo "$out"
+  exit 1
+fi
+
+success "Experimental-features list produces detailed build error"
+
+bold "Test: watch reports invalid experimental-features without panicking"
+
+cp rescript.json rescript.json.bak
+
+node -e '
+const fs = require("fs");
+const cfg = JSON.parse(fs.readFileSync("rescript.json", "utf8"));
+cfg["experimental-features"] = ["LetUnwrap"];
+fs.writeFileSync("rescript.json", JSON.stringify(cfg, null, 2));
+'
+
+out=$(rewatch watch 2>&1)
+status=$?
+
+mv rescript.json.bak rescript.json
+rm -f lib/rescript.lock
+
+if [ $status -eq 0 ]; then
+  error "Expected watch to fail for invalid experimental-features list"
+  echo "$out"
+  exit 1
+fi
+
+echo "$out" | grep -q "Could not read rescript.json"
+if [ $? -ne 0 ]; then
+  error "Missing rescript.json path context in watch error"
+  echo "$out"
+  exit 1
+fi
+
+echo "$out" | grep -qi "experimental-features.*invalid type"
+if [ $? -ne 0 ]; then
+  error "Missing detailed parse error for experimental-features list"
+  echo "$out"
+  exit 1
+fi
+
+echo "$out" | grep -q "panicked"
+if [ $? -eq 0 ]; then
+  error "Watcher should not panic when config is invalid"
+  echo "$out"
+  exit 1
+fi
+
+success "Invalid experimental-features list handled without panic"

rewatch/tests/experimental.sh

@@ -11,6 +11,6 @@ Use 'dev-dependencies' instead.
 The field 'bsc-flags' found in the package config of '@testrepo/deprecated-config' is deprecated and will be removed in a future version.
 Use 'compiler-flags' instead.
 
-The field 'ignored-dirs' found in the package config of '@testrepo/deprecated-config' is not supported by ReScript's new build system (rewatch).
+The field 'ignored-dirs' found in the package config of '@testrepo/deprecated-config' is not supported by ReScript 12's new build system.
 
 Unknown field 'some-new-field' found in the package config of '@testrepo/deprecated-config'. This option will be ignored.