From cd6b25f41d214ef107e89e67e0f8296234515590 Mon Sep 17 00:00:00 2001
From: Lukas Kalbertodt <lukas.kalbertodt@gmail.com>
Date: Fri, 11 Oct 2024 11:21:11 +0200
Subject: [PATCH] Treat deserialization errors from empty env vars as unset env
 var

Fixes #35

I'm a bit worried that this might seem too much like magic, but I think
this is just the most useful behavior in all situations I can think of.
This covers not only Booleans, but also numbers and anything else for
which an empty string is not a valid value. It also leaves types with
empty strings as valid values untouched.

I've been thinking about how this behavior could confuse users. And I
could only think of two cases:

- A config value has a default value, and the user thinks that empty
  string is a valid value, when in fact it isn't. Then, setting `FOO=`
  will result in the default value, when the user expected an empty
  string. Here, a clear error "empty string invalid" would be more
  helpful.

- A config value with a type that can be deserialized from empty string,
  but the user thinks it cannot. Maybe the config value is a list of
  numbers, where  "" is just an empty list, but the user thought it was
  a single number. Then setting `FOO=` means setting the value to an
  empty list.

Not sure if any of these cases will happen often enough or is important
enough to not merge this?
---
 src/internal.rs | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/internal.rs b/src/internal.rs
index 3ad175f..dd7efc3 100644
--- a/src/internal.rs
+++ b/src/internal.rs
@@ -66,15 +66,18 @@ pub fn from_env_with_parser<T, E: std::error::Error + Send + Sync + 'static>(
     parse: fn(&str) -> Result<T, E>,
 ) -> Result<Option<T>, Error> {
     let v = get_env_var!(key, field);
-    parse(&v)
-        .map(Some)
-        .map_err(|err| {
+    let is_empty = v.is_empty();
+    match parse(&v) {
+        Ok(v) => Ok(Some(v)),
+        Err(_) if is_empty => Ok(None),
+        Err(err) => Err(
             ErrorInner::EnvParseError {
                 field: field.to_owned(),
                 key: key.to_owned(),
                 err: Box::new(err),
             }.into()
-        })
+        ),
+    }
 }
 
 pub fn from_env_with_deserializer<T>(
@@ -83,9 +86,11 @@ pub fn from_env_with_deserializer<T>(
     deserialize: fn(crate::env::Deserializer) -> Result<T, crate::env::DeError>,
 ) -> Result<Option<T>, Error> {
     let s = get_env_var!(key, field);
+    let is_empty = s.is_empty();
 
     match deserialize(crate::env::Deserializer::new(s)) {
         Ok(v) => Ok(Some(v)),
+        Err(_) if is_empty => Ok(None),
         Err(e) => Err(ErrorInner::EnvDeserialization {
             key: key.into(),
             field: field.into(),