From c1df5d29cb5ecf8f80c5ed8ef051249f11c47ad5 Mon Sep 17 00:00:00 2001
From: Mac L <mjladson@pm.me>
Date: Thu, 24 Feb 2022 00:31:35 +0000
Subject: [PATCH] Ensure logfile respects the validators-dir CLI flag (#3003)

## Issue Addressed

Closes #2990

## Proposed Changes

Add a check to see if the `--validators-dir` CLI flag is set and if so store validator logs into it.
Ensure that if the log directory cannot be created, emit a `WARN` and disable file logging rather than panicking.

## Additional Info

Panics associated with logfiles can still occur in these scenarios:
1. The `$datadir/validators/logs` directory already exists with the wrong permissions (or was changed after creation).
1. The logfile already exists with the wrong permissions (or was changed after creation).
> These panics are cosmetic only since only the logfile thread panics. Following the panics, LH will continue to function as normal.

I believe this is due to the use of [`slog::Fuse`](https://docs.rs/slog/latest/slog/struct.Fuse.html) when initializing the logger.
I'm not sure if there a better way of handling logfile errors?
I think ideally, rather than panicking, we would emit a `WARN` to the stdout logger with the panic reason, then exit the logfile thread gracefully.
---
 lighthouse/environment/src/lib.rs | 16 +++++++++++++++-
 lighthouse/src/main.rs            | 25 ++++++++++++++++---------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/lighthouse/environment/src/lib.rs b/lighthouse/environment/src/lib.rs
index 448c84b54d4..91feef5b058 100644
--- a/lighthouse/environment/src/lib.rs
+++ b/lighthouse/environment/src/lib.rs
@@ -182,7 +182,21 @@ impl<E: EthSpec> EnvironmentBuilder<E> {
 
             // Create the necessary directories for the correct service and network.
             if !dir.exists() {
-                create_dir_all(dir).map_err(|e| format!("Unable to create directory: {:?}", e))?;
+                let res = create_dir_all(dir);
+
+                // If the directories cannot be created, warn and disable the logger.
+                match res {
+                    Ok(_) => (),
+                    Err(e) => {
+                        let log = stdout_logger;
+                        warn!(
+                            log,
+                            "Background file logging is disabled";
+                            "error" => e);
+                        self.log = Some(log);
+                        return Ok(self);
+                    }
+                }
             }
         }
 
diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs
index 51c1075cdb1..2f04b95ca4a 100644
--- a/lighthouse/src/main.rs
+++ b/lighthouse/src/main.rs
@@ -372,21 +372,28 @@ fn run<E: EthSpec>(
     // Construct the path to the log file.
     let mut log_path: Option<PathBuf> = clap_utils::parse_optional(matches, "logfile")?;
     if log_path.is_none() {
-        log_path = match matches.subcommand_name() {
-            Some("beacon_node") => Some(
+        log_path = match matches.subcommand() {
+            ("beacon_node", _) => Some(
                 parse_path_or_default(matches, "datadir")?
                     .join(DEFAULT_BEACON_NODE_DIR)
                     .join("logs")
                     .join("beacon")
                     .with_extension("log"),
             ),
-            Some("validator_client") => Some(
-                parse_path_or_default(matches, "datadir")?
-                    .join(DEFAULT_VALIDATOR_DIR)
-                    .join("logs")
-                    .join("validator")
-                    .with_extension("log"),
-            ),
+            ("validator_client", Some(vc_matches)) => {
+                let base_path = if vc_matches.is_present("validators-dir") {
+                    parse_path_or_default(vc_matches, "validators-dir")?
+                } else {
+                    parse_path_or_default(matches, "datadir")?.join(DEFAULT_VALIDATOR_DIR)
+                };
+
+                Some(
+                    base_path
+                        .join("logs")
+                        .join("validator")
+                        .with_extension("log"),
+                )
+            }
             _ => None,
         };
     }