Warn the user when Zebra doesn't recognize the format of `zebrad.toml` (#4689)

* Warn the user when Zebra cannot parse `zebrad.toml`

* Test that Zebra warns the user when it cannot parse `zebrad.toml`

* Fix up a mistaken merge change

* Suggest how to fix `zebrad.toml` when Zebra cannot parse it

Co-authored-by: teor <teor@riseup.net>

zebrad/src/application.rs

@@ -5,6 +5,7 @@ use std::{fmt::Write as _, io::Write as _, process};
 use abscissa_core::{
     application::{self, fatal_error, AppCell},
     config::{self, Configurable},
+    status_err,
     terminal::{component::Terminal, stderr, stdout, ColorChoice},
     Application, Component, EntryPoint, FrameworkError, Shutdown, StandardPaths, Version,
 };
@@ -202,11 +203,16 @@ impl Application for ZebradApp {
 
         // Load config *after* framework components so that we can
         // report an error to the terminal if it occurs.
-        let config = command
-            .config_path()
-            .map(|path| self.load_config(&path))
-            .transpose()?
-            .unwrap_or_default();
+        let config = match command.config_path() {
+            Some(path) => match self.load_config(&path) {
+                Ok(config) => config,
+                Err(e) => {
+                    status_err!("Zebra could not parse the provided config file. This might mean you are using a deprecated format of the file. You can generate a valid config by running \"zebrad generate\", and diff it against yours to examine any format inconsistencies.");
+                    return Err(e);
+                }
+            },
+            None => ZebradConfig::default(),
+        };
 
         let config = command.process_config(config)?;
 

zebrad/tests/acceptance.rs

@@ -103,10 +103,10 @@
 //!
 //! Please refer to the documentation of each test for more information.
 
-use std::{collections::HashSet, env, path::PathBuf};
+use std::{collections::HashSet, env, fs, path::PathBuf, time::Duration};
 
 use color_eyre::{
-    eyre::{Result, WrapErr},
+    eyre::{eyre, Result, WrapErr},
     Help,
 };
 
@@ -355,7 +355,6 @@ fn misconfigured_ephemeral_missing_directory() -> Result<()> {
 }
 
 fn ephemeral(cache_dir_config: EphemeralConfig, cache_dir_check: EphemeralCheck) -> Result<()> {
-    use std::fs;
     use std::io::ErrorKind;
 
     zebra_test::init();
@@ -502,19 +501,25 @@ fn version_args() -> Result<()> {
     Ok(())
 }
 
+/// Run config tests that use the default ports and paths.
+///
+/// Unlike the other tests, these tests can not be run in parallel, because
+/// they use the generated config. So parallel execution can cause port and
+/// cache conflicts.
 #[test]
-fn valid_generated_config_test() -> Result<()> {
-    // Unlike the other tests, these tests can not be run in parallel, because
-    // they use the generated config. So parallel execution can cause port and
-    // cache conflicts.
+fn config_test() -> Result<()> {
     valid_generated_config("start", "Starting zebrad")?;
 
-    // Check that the stored configuration we have for Zebra works
+    // Check what happens when Zebra parses an invalid config
+    invalid_generated_config()?;
+
+    // Check that an older stored configuration we have for Zebra works
     stored_config_works()?;
 
     Ok(())
 }
 
+/// Test that `zebrad start` can parse the output from `zebrad generate`.
 fn valid_generated_config(command: &str, expect_stdout_line_contains: &str) -> Result<()> {
     zebra_test::init();
 
@@ -564,6 +569,77 @@ fn valid_generated_config(command: &str, expect_stdout_line_contains: &str) -> R
     Ok(())
 }
 
+/// Checks that Zebra prints an informative message when it cannot parse the
+/// config file.
+fn invalid_generated_config() -> Result<()> {
+    zebra_test::init();
+
+    let testdir = &testdir()?;
+
+    // Add a config file name to tempdir path.
+    let config_path = testdir.path().join("zebrad.toml");
+
+    // Generate a valid config file in the temp dir.
+    let child = testdir.spawn_child(args!["generate", "-o": config_path.to_str().unwrap()])?;
+
+    let output = child.wait_with_output()?;
+    let output = output.assert_success()?;
+
+    assert_with_context!(
+        config_path.exists(),
+        &output,
+        "generated config file not found"
+    );
+
+    // Load the valid config file that Zebra generated.
+    let mut config_file = fs::read_to_string(config_path.to_str().unwrap()).unwrap();
+
+    // Let's now alter the config file so that it contains a deprecated format
+    // of `mempool.eviction_memory_time`.
+
+    config_file = config_file
+        .lines()
+        // Remove the valid `eviction_memory_time` key/value pair from the
+        // config.
+        .filter(|line| !line.contains("eviction_memory_time"))
+        .map(|line| line.to_owned() + "\n")
+        .collect();
+
+    // Append the `eviction_memory_time` key/value pair in a deprecated format.
+    config_file += r"
+
+            [mempool.eviction_memory_time]
+            nanos = 0
+            secs = 3600
+    ";
+
+    // Write the altered config file so that Zebra can pick it up.
+    fs::write(config_path.to_str().unwrap(), config_file.as_bytes())
+        .expect("Could not write the altered config file.");
+
+    // Run Zebra in a temp dir so that it loads the config.
+    let mut child = testdir.spawn_child(args!["start"])?;
+
+    // Return an error if Zebra is running for more than two seconds.
+    //
+    // Since the config is invalid, Zebra should terminate instantly after its
+    // start. Two seconds should be sufficient for Zebra to read the config file
+    // and terminate.
+    std::thread::sleep(Duration::from_secs(2));
+    if child.is_running() {
+        child.kill()?;
+        return Err(eyre!("Zebra should not be running anymore."));
+    }
+
+    let output = child.wait_with_output()?;
+
+    // Check that Zebra produced an informative message.
+    output.stderr_contains("Zebra could not parse the provided config file. This might mean you are using a deprecated format of the file.")?;
+
+    Ok(())
+}
+
+/// Test that an older `zebrad.toml` can still be parsed by the latest `zebrad`.
 fn stored_config_works() -> Result<()> {
     let stored_config_path = stored_config_path();
     let run_dir = testdir()?;
@@ -1596,8 +1672,6 @@ where
     // See #1781.
     #[cfg(target_os = "linux")]
     if node2.is_running() {
-        use color_eyre::eyre::eyre;
-
         return node2
             .kill_on_error::<(), _>(Err(eyre!(
                 "conflicted node2 was still running, but the test expected a panic"