move the one direct environment check to config (#103)

README.md

@@ -47,7 +47,7 @@ An example `.envrc` is provided for optional but recommended use with [`direnv`]
 * `LISTEN_FD` -- if supplied on unix-like systems, if this is set to an open file descriptor number, the server will listen to that fd
 * `OTEL_EXPORTER_PROMETHEUS_HOST` -- default `"localhost"`
 * `OTEL_EXPORTER_PROMETHEUS_PORT` -- default `9464`
-* `SKIP_APP_COMPILATION` -- we currently build the react app in a build script. To avoid this behavior, set this environment variable.
+* `SKIP_APP_COMPILATION` -- we currently build the react app in a build script. To avoid this behavior, set this environment variable to `true`.
 * `ASSET_DIR` -- set this to skip building the react app and include react app assets from a different directory
 
 ## Initial setup
@@ -136,7 +136,7 @@ $ cargo run --features aggregator-api-mock
 By default, building the rust server will also build the react app. To skip this (for example, when running a development server), set `SKIP_APP_COMPIILATION=1`
 
 ```bash
-$ SKIP_APP_COMPILATION=1 cargo run --features aggregator-api-mock
+$ SKIP_APP_COMPILATION=true cargo run --features aggregator-api-mock
 ```
 
 ### Running the React development server

src/config.rs

@@ -26,6 +26,7 @@ pub struct ApiConfig {
     pub email_address: EmailAddress,
     pub postmark_url: Url,
     pub client: Client,
+    pub skip_app_compilation: bool,
 }
 
 #[derive(Debug, Error, Clone, Copy)]
@@ -54,22 +55,11 @@ fn var<T: FromStr>(name: &'static str) -> Result<T, ApiConfigError> {
         })
 }
 
-fn var_optional<T: FromStr + 'static>(
-    name: &'static str,
-    default: &'static str,
-) -> Result<T, ApiConfigError> {
-    let format = std::any::type_name::<T>();
-    let input_res = std::env::var(name);
-    let input = match &input_res {
-        Ok(value) => value,
-        Err(VarError::NotPresent) => default,
-        Err(VarError::NotUnicode(_)) => {
-            return Err(ApiConfigError::InvalidEnvVarFormat(name, format))
-        }
-    };
-    input
-        .parse()
-        .map_err(|_| ApiConfigError::InvalidEnvVarFormat(name, format))
+fn var_optional<T: FromStr>(name: &'static str, default: T) -> Result<T, ApiConfigError> {
+    match var(name) {
+        Err(ApiConfigError::MissingEnvVar(_)) => Ok(default),
+        other => other,
+    }
 }
 
 impl ApiConfig {
@@ -86,11 +76,12 @@ impl ApiConfig {
             aggregator_dap_url: var("AGGREGATOR_DAP_URL")?,
             aggregator_api_url: var("AGGREGATOR_API_URL")?,
             aggregator_secret: var("AGGREGATOR_SECRET")?,
-            prometheus_host: var_optional("OTEL_EXPORTER_PROMETHEUS_HOST", "localhost")?,
-            prometheus_port: var_optional("OTEL_EXPORTER_PROMETHEUS_PORT", "9464")?,
+            prometheus_host: var_optional("OTEL_EXPORTER_PROMETHEUS_HOST", "localhost".into())?,
+            prometheus_port: var_optional("OTEL_EXPORTER_PROMETHEUS_PORT", 9464)?,
             postmark_token: var("POSTMARK_TOKEN")?,
             email_address: var("EMAIL_ADDRESS")?,
             postmark_url: Url::parse("https://api.postmarkapp.com").unwrap(),
+            skip_app_compilation: var_optional("SKIP_APP_COMPILATION", false)?,
             client: Client::new(RustlsConfig::default().with_tcp_config(ClientConfig::default()))
                 .with_default_pool(),
         })

src/handler/assets.rs

@@ -12,7 +12,7 @@ use url::Url;
 const ONE_YEAR: Duration = Duration::from_secs(60 * 60 * 24 * 365);
 
 pub fn static_assets(config: &ApiConfig) -> impl Handler {
-    if std::env::var("SKIP_APP_COMPILATION").is_ok() {
+    if config.skip_app_compilation {
         None
     } else {
         Some(origin_router().with_handler(

tests/harness/mod.rs

@@ -71,6 +71,7 @@ pub fn config(api_mocks: impl Handler) -> ApiConfig {
         email_address: "test@example.test".parse().unwrap(),
         postmark_url: POSTMARK_URL.parse().unwrap(),
         client: Client::new(trillium_testing::connector(api_mocks)),
+        skip_app_compilation: false,
     }
 }
 
@@ -89,7 +90,6 @@ where
     F: FnOnce(DivviupApi) -> Fut,
     Fut: Future<Output = Result<(), Box<dyn Error>>> + Send + 'static,
 {
-    std::env::remove_var("SKIP_APP_COMPILATION");
     block_on(async move {
         let (app, _) = build_test_app().await;
         f(app).await.unwrap();