feat: provide clear errors for missing required envs

CHANGELOG.md

@@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Fixed
+- **Runtime ENV handling** – Empty or invalid environment variables (e.g. `SMTP_PORT=`, `PORT=`, `POOL_SIZE=`, `DATABASE_PORT=`) no longer cause `ArgumentError` at boot. Instead raises clear errors for required vars set but empty (e.g. DATABASE_HOST, PHX_HOST/DOMAIN, SECRET_KEY_BASE).
+
 ## [1.1.1] - 2026-03-16
 
 ### Added

CODE_GUIDELINES.md

@@ -2177,6 +2177,14 @@ mix phx.gen.secret
 mix phx.gen.secret
 ```
 
+**Runtime configuration (config/runtime.exs):**
+
+- Production config is loaded from `config/runtime.exs` at boot (releases and `mix phx.server`). Environment variables are read via helpers so that **empty or invalid values do not cause cryptic crashes** (e.g. `ArgumentError` from `String.to_integer("")`).
+- **Helpers used:** `get_env_or_file` / `get_env_or_file!` (with `_FILE` support); `get_env_required` (required vars: raises if missing or empty after trim); `get_env_non_empty` (optional string: empty treated as unset, returns default); `parse_positive_integer` (PORT, POOL_SIZE, SMTP_PORT: empty or invalid → default).
+- **Required vars** (e.g. DATABASE_HOST, PHX_HOST/DOMAIN, SECRET_KEY_BASE): if set but empty, the app raises at boot with a clear message including “(Variable X is set but empty.)”.
+- **Optional numeric vars** (PORT, POOL_SIZE, SMTP_PORT, DATABASE_PORT): empty or invalid value is treated as “unset” and the documented default is used (e.g. PORT=4000, SMTP_PORT=587).
+- When adding new ENV in `runtime.exs`, use these helpers instead of raw `System.get_env(...)` and `String.to_integer(...)` so that misconfigured or empty variables fail fast with clear errors.
+
 ### 5.6 Security Headers
 
 **Configure Security Headers:**

config/runtime.exs

@@ -32,11 +32,91 @@ get_env_or_file = fn var_name, default ->
   end
 end
 
-# Same as get_env_or_file but raises if the value is not set
+# Same as get_env_or_file but raises if the value is not set or empty (after trim).
+# Empty values lead to unclear runtime errors; failing at boot with a clear message is preferred.
 get_env_or_file! = fn var_name, error_message ->
   case get_env_or_file.(var_name, nil) do
-    nil -> raise error_message
+    nil ->
-    value -> value
+      raise error_message
+
+    value when is_binary(value) ->
+      trimmed = String.trim(value)
+
+      if trimmed == "" do
+        raise """
+        #{error_message}
+        (Variable #{var_name} or #{var_name}_FILE is set but the value is empty.)
+        """
+      else
+        trimmed
+      end
+
+    value ->
+      value
+  end
+end
+
+# Returns default when env_value is nil, empty after trim, or not a valid positive integer.
+# Used for PORT, POOL_SIZE, SMTP_PORT to avoid ArgumentError on empty or invalid values.
+parse_positive_integer = fn env_value, default ->
+  case env_value do
+    nil ->
+      default
+
+    v when is_binary(v) ->
+      case String.trim(v) do
+        "" ->
+          default
+
+        trimmed ->
+          case Integer.parse(trimmed) do
+            {n, _} when n > 0 -> n
+            _ -> default
+          end
+      end
+
+    _ ->
+      default
+  end
+end
+
+# Returns default when the key is missing or the value is empty (after trim).
+# Use for optional string ENV vars (e.g. DATABASE_PORT) so empty string is treated as "unset".
+get_env_non_empty = fn key, default ->
+  case System.get_env(key) do
+    nil ->
+      default
+
+    v when is_binary(v) ->
+      trimmed = String.trim(v)
+      if trimmed == "", do: default, else: trimmed
+
+    v ->
+      v
+  end
+end
+
+# Returns the trimmed value when set and non-empty; otherwise raises with error_message.
+# Use for required vars (DATABASE_HOST, etc.) so "set but empty" fails at boot with a clear message.
+get_env_required = fn key, error_message ->
+  case System.get_env(key) do
+    nil ->
+      raise error_message
+
+    v when is_binary(v) ->
+      trimmed = String.trim(v)
+
+      if trimmed == "" do
+        raise """
+        #{error_message}
+        (Variable #{key} is set but empty.)
+        """
+      else
+        trimmed
+      end
+
+    v ->
+      v
   end
 end
 
@@ -49,12 +129,14 @@ build_database_url = fn ->
     nil ->
       # Build URL from separate components
       host =
-        System.get_env("DATABASE_HOST") ||
+        get_env_required.("DATABASE_HOST", """
-          raise "DATABASE_HOST is required when DATABASE_URL is not set"
+        DATABASE_HOST is required when DATABASE_URL is not set.
+        """)
 
       user =
-        System.get_env("DATABASE_USER") ||
+        get_env_required.("DATABASE_USER", """
-          raise "DATABASE_USER is required when DATABASE_URL is not set"
+        DATABASE_USER is required when DATABASE_URL is not set.
+        """)
 
       password =
         get_env_or_file!.("DATABASE_PASSWORD", """
@@ -62,10 +144,11 @@ build_database_url = fn ->
         """)
 
       database =
-        System.get_env("DATABASE_NAME") ||
+        get_env_required.("DATABASE_NAME", """
-          raise "DATABASE_NAME is required when DATABASE_URL is not set"
+        DATABASE_NAME is required when DATABASE_URL is not set.
+        """)
 
-      port = System.get_env("DATABASE_PORT", "5432")
+      port = get_env_non_empty.("DATABASE_PORT", "5432")
 
       # URL-encode the password to handle special characters
       encoded_password = URI.encode_www_form(password)
@@ -102,7 +185,7 @@ if config_env() == :prod do
   config :mv, Mv.Repo,
     # ssl: true,
     url: database_url,
-    pool_size: String.to_integer(System.get_env("POOL_SIZE") || "10"),
+    pool_size: parse_positive_integer.(System.get_env("POOL_SIZE"), 10),
     socket_options: maybe_ipv6
 
   # The secret key base is used to sign/encrypt cookies and other secrets.
@@ -120,11 +203,14 @@ if config_env() == :prod do
   # PHX_HOST or DOMAIN can be used to set the host for the application.
   # DOMAIN is commonly used in deployment environments (e.g., Portainer templates).
   host =
-    System.get_env("PHX_HOST") ||
+    get_env_non_empty.("PHX_HOST", nil) ||
-      System.get_env("DOMAIN") ||
+      get_env_non_empty.("DOMAIN", nil) ||
-      raise "Please define the PHX_HOST or DOMAIN environment variable."
+      raise """
+      Please define the PHX_HOST or DOMAIN environment variable.
+      (Variable may be set but empty.)
+      """
 
-  port = String.to_integer(System.get_env("PORT") || "4000")
+  port = parse_positive_integer.(System.get_env("PORT"), 4000)
 
   config :mv, :dns_cluster_query, System.get_env("DNS_CLUSTER_QUERY")
 
@@ -230,11 +316,7 @@ if config_env() == :prod do
   smtp_host_env = System.get_env("SMTP_HOST")
 
   if smtp_host_env && String.trim(smtp_host_env) != "" do
-    smtp_port_env =
+    smtp_port_env = parse_positive_integer.(System.get_env("SMTP_PORT"), 587)
-      case System.get_env("SMTP_PORT") do
-        nil -> 587
-        v -> String.to_integer(String.trim(v))
-      end
 
     smtp_password_env =
       case System.get_env("SMTP_PASSWORD") do

docs/smtp-configuration-concept.md

@@ -42,6 +42,8 @@ When an ENV variable is set, the corresponding Settings field is read-only in th
 | Sender name    | `MAIL_FROM_NAME`       | `smtp_from_name`    | Display name in "From" header (default: Mila)|
 | Sender email   | `MAIL_FROM_EMAIL`      | `smtp_from_email`   | Address in "From" header; must match SMTP user on most servers |
 
+**Boot-time ENV handling:** In `config/runtime.exs`, if `SMTP_PORT` is set but empty or invalid, it is treated as unset and default 587 is used. This avoids startup crashes (e.g. `ArgumentError` from `String.to_integer("")`) when variables are misconfigured in deployment.
+
 **Important:** On most SMTP servers (e.g. Postfix with strict relay policies) the sender email (`smtp_from_email`) must be the same address as `smtp_username` or an alias that is owned by that account.
 
 **Settings UI:** The form uses three rows on wide viewports: host, port, TLS/SSL | username, password | sender email, sender name. Content width is limited by the global settings wrapper (see `DESIGN_GUIDELINES.md` §6.4).

lib/mv/mailer.ex

@@ -100,7 +100,11 @@ defmodule Mv.Mailer do
   """
   @spec smtp_config() :: keyword()
   def smtp_config do
-    if Mv.Config.smtp_configured?() and not boot_smtp_configured?() do
+    # In test we use Swoosh.Adapters.Test; do not override with SMTP opts or emails would not land in the test mailbox.
+    adapter = Application.get_env(:mv, __MODULE__, []) |> Keyword.get(:adapter)
+
+    if Mv.Config.smtp_configured?() and not boot_smtp_configured?() and
+         adapter != Swoosh.Adapters.Test do
       verify_mode =
         if Application.get_env(:mv, :smtp_verify_peer, false),
           do: :verify_peer,

test/mv_web/controllers/auth_controller_test.exs

@@ -316,9 +316,11 @@ defmodule MvWeb.AuthControllerTest do
 
         # When OIDC-only is enabled, password sign-in must not succeed (no redirect to sign_in_with_token).
         case result do
-          {:error, {:redirect, %{to: to}}} ->
+          {:error, {:redirect, opts}} when is_map(opts) ->
-            refute to =~ "sign_in_with_token",
+            to_path = Map.get(opts, :to) || Map.get(opts, "to") || ""
-                   "Expected password sign-in to be rejected when OIDC-only, got redirect to: #{to}"
+
+            refute to_path =~ "sign_in_with_token",
+                   "Expected password sign-in to be rejected when OIDC-only, got redirect to: #{to_path}"
 
           _ ->
             # LiveView re-rendered (e.g. with flash error) instead of redirecting to success
@@ -398,8 +400,24 @@ defmodule MvWeb.AuthControllerTest do
 
     test "returns 200 when OIDC-only but OIDC not configured", %{conn: authenticated_conn} do
       {:ok, settings} = Membership.get_settings()
-      original_oidc_only = Map.get(settings, :oidc_only, false)
+
-      {:ok, _} = Membership.update_settings(settings, %{oidc_only: true})
+      prev = %{
+        oidc_only: settings.oidc_only,
+        oidc_client_id: settings.oidc_client_id,
+        oidc_base_url: settings.oidc_base_url,
+        oidc_redirect_uri: settings.oidc_redirect_uri,
+        oidc_client_secret: settings.oidc_client_secret
+      }
+
+      # Set OIDC-only but leave OIDC unconfigured so the plug does not redirect.
+      {:ok, _} =
+        Membership.update_settings(settings, %{
+          oidc_only: true,
+          oidc_client_id: nil,
+          oidc_base_url: nil,
+          oidc_redirect_uri: nil,
+          oidc_client_secret: nil
+        })
 
       try do
         conn = build_unauthenticated_conn(authenticated_conn)
@@ -407,7 +425,7 @@ defmodule MvWeb.AuthControllerTest do
         assert conn.status == 200
       after
         {:ok, s} = Membership.get_settings()
-        Membership.update_settings(s, %{oidc_only: original_oidc_only})
+        Membership.update_settings(s, prev)
       end
     end