From 05e0b2f4c042c6c23f0232b87e45c0648e5096ae Mon Sep 17 00:00:00 2001
From: Daniel Szoke <szokeasaurusrex@users.noreply.github.com>
Date: Mon, 25 Mar 2024 13:44:49 +0100
Subject: [PATCH] fix: Handle .env errors (#1987)

If there is an error loading a .env file, we now print a warning message. There is no warning when the .env was not found because users might not have set up a .env; we only warn when the .env was found but there was some error reading it.

Fixes GH-1624
---
 src/commands/mod.rs                           | 10 +++++-
 src/utils/system.rs                           | 34 +++++++++++++------
 .../integration/_cases/invalid_env/.gitignore |  2 ++
 .../_cases/invalid_env/invalid-env.in/.env    |  2 ++
 .../_cases/invalid_env/invalid-env.trycmd     |  7 ++++
 tests/integration/invalid_env.rs              |  6 ++++
 tests/integration/mod.rs                      |  1 +
 7 files changed, 51 insertions(+), 11 deletions(-)
 create mode 100644 tests/integration/_cases/invalid_env/.gitignore
 create mode 100644 tests/integration/_cases/invalid_env/invalid-env.in/.env
 create mode 100644 tests/integration/_cases/invalid_env/invalid-env.trycmd
 create mode 100644 tests/integration/invalid_env.rs

diff --git a/src/commands/mod.rs b/src/commands/mod.rs
index 715868b86d..26339c6206 100644
--- a/src/commands/mod.rs
+++ b/src/commands/mod.rs
@@ -308,12 +308,20 @@ pub fn execute() -> Result<()> {
 
 fn setup() {
     init_backtrace();
-    load_dotenv();
+
+    // Store the result of loading the dotenv file. We must load the dotenv file
+    // before setting the log level, as the log level can be set in the dotenv
+    // file, but we should only log a warning after setting the log level.
+    let load_dotenv_result = load_dotenv();
 
     // we use debug internally but our log handler then rejects to a lower limit.
     // This is okay for our uses but not as efficient.
     set_max_level(LevelFilter::Debug);
     set_logger(&Logger).unwrap();
+
+    if let Err(e) = load_dotenv_result {
+        log::warn!("Failed to load .env file: {}", e);
+    }
 }
 
 /// Executes the command line application and exits the process.
diff --git a/src/utils/system.rs b/src/utils/system.rs
index 13245708ab..96b0e32649 100644
--- a/src/utils/system.rs
+++ b/src/utils/system.rs
@@ -4,6 +4,7 @@ use std::process;
 
 use anyhow::{Error, Result};
 use console::style;
+use dotenv::Result as DotenvResult;
 use lazy_static::lazy_static;
 use regex::{Captures, Regex};
 
@@ -127,15 +128,28 @@ pub fn init_backtrace() {
 pub struct QuietExit(pub i32);
 
 /// Loads a .env file
-pub fn load_dotenv() {
-    if env::var("SENTRY_LOAD_DOTENV")
-        .map(|x| x.as_str() == "1")
-        .unwrap_or(true)
-    {
-        if let Ok(path) = env::var("SENTRY_DOTENV_PATH") {
-            dotenv::from_path(path).ok();
-        } else {
-            dotenv::dotenv().ok();
-        }
+pub fn load_dotenv() -> DotenvResult<()> {
+    let load_dotenv_unset = env::var("SENTRY_LOAD_DOTENV")
+        .map(|x| x.as_str() != "1")
+        .unwrap_or(false);
+
+    if load_dotenv_unset {
+        return Ok(());
     }
+
+    match env::var("SENTRY_DOTENV_PATH") {
+        Ok(path) => dotenv::from_path(path),
+        Err(_) => dotenv::dotenv().map(|_| ()),
+    }
+    .map_or_else(
+        |error| {
+            // We only propogate errors if the .env file was found and failed to load.
+            if error.not_found() {
+                Ok(())
+            } else {
+                Err(error)
+            }
+        },
+        |_| Ok(()),
+    )
 }
diff --git a/tests/integration/_cases/invalid_env/.gitignore b/tests/integration/_cases/invalid_env/.gitignore
new file mode 100644
index 0000000000..c2511ca6da
--- /dev/null
+++ b/tests/integration/_cases/invalid_env/.gitignore
@@ -0,0 +1,2 @@
+# We explicitly want to include invalid-env.in, since the contents are static.
+!invalid-env.in
diff --git a/tests/integration/_cases/invalid_env/invalid-env.in/.env b/tests/integration/_cases/invalid_env/invalid-env.in/.env
new file mode 100644
index 0000000000..6ff9a3efbc
--- /dev/null
+++ b/tests/integration/_cases/invalid_env/invalid-env.in/.env
@@ -0,0 +1,2 @@
+# Spaces in the variable assignment will produce an error reading this .env file
+TEST=1 4 2
diff --git a/tests/integration/_cases/invalid_env/invalid-env.trycmd b/tests/integration/_cases/invalid_env/invalid-env.trycmd
new file mode 100644
index 0000000000..925b255a75
--- /dev/null
+++ b/tests/integration/_cases/invalid_env/invalid-env.trycmd
@@ -0,0 +1,7 @@
+```
+$ sentry-cli
+? failed
+[..]WARN[..]Failed to load .env file:[..]
+...
+
+```
diff --git a/tests/integration/invalid_env.rs b/tests/integration/invalid_env.rs
new file mode 100644
index 0000000000..614ae6040c
--- /dev/null
+++ b/tests/integration/invalid_env.rs
@@ -0,0 +1,6 @@
+use super::register_test;
+
+#[test]
+fn testdinvalid_env() {
+    register_test("invalid_env/invalid-env.trycmd");
+}
diff --git a/tests/integration/mod.rs b/tests/integration/mod.rs
index 0787b66381..b061f226d1 100644
--- a/tests/integration/mod.rs
+++ b/tests/integration/mod.rs
@@ -4,6 +4,7 @@ mod deploys;
 mod events;
 mod help;
 mod info;
+mod invalid_env;
 mod issues;
 mod login;
 mod monitors;