Support PORT env var using runtime.exs

[rhcarvalho]

While upgrading an app from v1.7.21 to v1.8.1, I noticed changes to config/dev.exs in new projects which depended on the system environment at build time.

This commit updates the support for the PORT environment variable in new projects generated with phx.new to be done in runtime.exs instead of dev.exs, which is meant for static build time config.

Dev support for PORT was added a few months ago in 7a88d0f, and shipped first in v1.8.0-rc.1.

[rhcarvalho]

Hmm the failed test passes for me locally:

$ mix test test/phoenix/integration/endpoint_test.exs:126
Compiling 74 files (.ex)
Generated phoenix app
Running ExUnit with seed: 996645, max_cases: 16
Excluding tags: [:test]
Including tags: [location: {"test/phoenix/integration/endpoint_test.exs", 126}]

.
Finished in 1.1 seconds (0.00s async, 1.1s sync)
4 tests, 0 failures, 3 excluded

[SteffenDE]

@rhcarvalho I did not experience any issues with having it in the build-time config for dev, did you?

[rhcarvalho]

@rhcarvalho I did not experience any issues with having it in the build-time config for dev, did you?

No particular issue. Maybe it's my limited knowledge :)

My first instinct when seeing the new code in config/dev.exs was "I learned somewhere I should not use System.get_env/1 in "compile-time config files", but rather in runtime.exs.

When Mix is available, PORT=5001 mix phx.server works fine. I think the problem arises in Releases, and why runtime config, including reading from env vars, is written in runtime.exs and not prod.exs.

I don't remember where exactly I got my "lesson" from, but found a few related threads in the forum:

• https://elixirforum.com/t/config-exs-and-system-get-env-2-are-values-actually-read-at-runtime/56481
• https://elixirforum.com/t/how-to-use-system-get-env-inside-my-config-exs-file/67883
• https://elixirforum.com/t/warnings-about-application-get-env-2-when-using-environment-variables-for-configuration/56567

And I was about to close this PR until I found two interesting historical commits:

1. In the past, 2014, dev.exs and config.exs: e8d9b45

$ git show e8d9b45c32989377726da21075f85f1f3866a837:priv/template/config/dev.exs
use Mix.Config

config :<%= application_name %>, <%= application_module %>.Endpoint,
  http: [port: System.get_env("PORT") || 4000],
  debug_errors: true,
  cache_static_lookup: false

# Enables code reloading for development
config :phoenix, :code_reloader, true
$ git show e8d9b45c32989377726da21075f85f1f3866a837
commit e8d9b45c32989377726da21075f85f1f3866a837
Author: José Valim <[email protected]>
Date:   Sat Dec 13 14:25:38 2014 +0100

    Remove repeated HTTP config

diff --git a/priv/template/config/config.exs b/priv/template/config/config.exs
index 3932e08cd..eec091a6d 100644
--- a/priv/template/config/config.exs
+++ b/priv/template/config/config.exs
@@ -8,7 +8,6 @@ use Mix.Config
 # Configures the endpoint
 config :<%= application_name %>, <%= application_module %>.Endpoint,
   url: [host: "localhost"],
-  http: [port: System.get_env("PORT")],
   secret_key_base: "<%= secret_key_base %>",
   debug_errors: false

Note that the Endpoint :port config used to be exactly what we have now, either PORT env or 4000. But...

2. Later in 2015, still quite long ago: feef607

Quoting José's commit message:

Do not use System.get_env/1 in config files
They do not work with releases, so let's not teach
something that is going to break during deployment.

And then dev.exs became the static config as we've had until recently, hard coded to port 4000.

My intuition and motivation for the change matched exactly what José wrote back then.

---

I'm guessing Chris reintroduced the configurable port in dev for something like phoenix.new or other initiative. Myself, I haven't needed the customization anyway, as most of the time I'm running in a container and can map any local port to port 4000 on the container.

@SteffenDE what do you think, shall we drop this? Or is it still "teaching the right patterns" and worth doing again nowadays?

[SteffenDE]

Yeah, I think @josevalim was / is right about not teaching about System.get_env in config files other than runtime.exs. I'd just write it as

if port = System.get_env("PORT") do
  config :my_app, MyAppWeb.Endpoint, http: [port: String.to_integer(port)]
end

and not check the config_env at all for the port.

[rhcarvalho]

I'd just write it as [...] and not check the config_env at all for the port.

Good call, makes it work the same regardless of env. We'd be potentially affecting the standard :test env, do you think that'd be a problem? AFAIK we don't start any HTTP servers by default in tests, right?

I can update the PR.

[rhcarvalho]

Yeah, I think @josevalim was / is right about not teaching about System.get_env in config files other than runtime.exs. I'd just write it as

if port = System.get_env("PORT") do
  config :my_app, MyAppWeb.Endpoint, http: [port: String.to_integer(port)]
end

and not check the config_env at all for the port.

While I edit the code, I see one downside to globally setting the port: endpoint config becomes spread out, making the setting of IP and port far apart in the config file, which can cause friction for humans trying to update the config or reason about what is set and why. This concern might not be real.

[rhcarvalho]

Rebased on top of latest main and applied your suggestion @SteffenDE, indeed applying the same config to all environments makes the overall config less repetitive.

[rhcarvalho]

https://github.com/phoenixframework/phoenix/actions/runs/17941936145/job/51020424195?pr=6449

Test error seems to be due to a Docker Hub timeout. Looks like other PRs are running into the same issue.

[SteffenDE]

yeah Docker Hub isn't too reliable...

[josevalim]

💚 💙 💜 💛 ❤️