From 1e6ec5f1be3086bcc3df6746f21b8e07ab755ec9 Mon Sep 17 00:00:00 2001 From: Atharva0506 Date: Thu, 20 Aug 2026 12:42:13 +0530 Subject: [PATCH 1/3] fix: support the standard PORT environment variable Managed platforms (Render, Railway, Heroku, Cloud Run) inject a PORT variable and expect the process to bind to it. ThruBox only read RELAY_SERVER_PORT, so it kept binding to 3000 and deploys either failed their health checks or relied on the platform's port auto-detection. Read PORT as a fallback in applyEnvOverrides. RELAY_SERVER_PORT stays authoritative when both are set, so existing deployments are unaffected. An invalid RELAY_SERVER_PORT does not silently fall through to PORT -- it warns and keeps the default, matching the previous behaviour. Adds the first test file in the repository, covering the precedence matrix, and documents PORT in the README configuration table. Closes #26 --- README.md | 8 ++- internal/config/config.go | 13 +++-- internal/config/config_test.go | 92 ++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 internal/config/config_test.go diff --git a/README.md b/README.md index a263a55..b9e6770 100644 --- a/README.md +++ b/README.md @@ -188,7 +188,7 @@ docker compose up -d go test ./... ``` -> No test files exist in the repository yet — this is the standard command to run once tests are added. See `CONTRIBUTING.md` before submitting a PR that adds functionality without tests. +> Tests live beside the code they cover (for example `internal/config/config_test.go`). See `CONTRIBUTING.md` before submitting a PR that adds functionality without tests. ### Configuration @@ -197,6 +197,7 @@ Edit `config.yaml` or use environment variables: | Setting | YAML Key | Env Variable | Default | |---------|----------|-------------|---------| | Server port | `server.port` | `RELAY_SERVER_PORT` | `3000` | +| Server port (fallback) | — | `PORT` | _unset_ — used only when `RELAY_SERVER_PORT` is not set | | Server host | `server.host` | `RELAY_SERVER_HOST` | `0.0.0.0` | | Storage path | `storage.path` | `RELAY_STORAGE_PATH` | `./data/relay.db` | | Message TTL | `messages.ttl_days` | `RELAY_MESSAGES_TTL_DAYS` | `7` (0 = forever) | @@ -204,6 +205,11 @@ Edit `config.yaml` or use environment variables: | Rate limit | `security.rate_limit` | `RELAY_SECURITY_RATE_LIMIT` | `30` req/min/IP | | API key | `security.api_key` | `RELAY_SECURITY_API_KEY` | `` (disabled) | +> **Deploying to a managed platform?** Render, Railway, Heroku and Cloud Run +> inject a `PORT` variable and expect the process to bind to it. ThruBox reads +> it automatically, so no extra configuration is needed. `RELAY_SERVER_PORT` +> still takes precedence when both are set. + --- ## 🙌 Contributing diff --git a/internal/config/config.go b/internal/config/config.go index 46afad3..367ab16 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -97,11 +97,18 @@ func Load(path string) (*Config, error) { // applyEnvOverrides checks for environment variables and overrides // the corresponding config values if set. func applyEnvOverrides(cfg *Config) { - if v := os.Getenv("RELAY_SERVER_PORT"); v != "" { - if port, err := strconv.Atoi(v); err == nil { + // Port resolution: RELAY_SERVER_PORT is authoritative. PORT is only a + // fallback, for managed platforms (Render, Railway, Heroku, Cloud Run) + // that inject it and expect the process to bind to it. + portVar, portVal := "RELAY_SERVER_PORT", os.Getenv("RELAY_SERVER_PORT") + if portVal == "" { + portVar, portVal = "PORT", os.Getenv("PORT") + } + if portVal != "" { + if port, err := strconv.Atoi(portVal); err == nil { cfg.Server.Port = port } else { - log.Printf("warning: invalid RELAY_SERVER_PORT=%q, using default", v) + log.Printf("warning: invalid %s=%q, using default", portVar, portVal) } } diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..07b858f --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,92 @@ +package config + +import "testing" + +// TestApplyEnvOverrides_Port covers the precedence rules between the +// project-specific RELAY_SERVER_PORT and the platform-conventional PORT. +func TestApplyEnvOverrides_Port(t *testing.T) { + tests := []struct { + name string + relayServerPort string + port string + want int + }{ + { + name: "neither set falls back to the default", + want: 3000, + }, + { + name: "PORT alone is honoured", + port: "8080", + want: 8080, + }, + { + name: "RELAY_SERVER_PORT alone is honoured", + relayServerPort: "9090", + want: 9090, + }, + { + name: "RELAY_SERVER_PORT wins when both are set", + relayServerPort: "3000", + port: "8080", + want: 3000, + }, + { + name: "invalid PORT falls back to the default", + port: "not-a-port", + want: 3000, + }, + { + name: "invalid RELAY_SERVER_PORT does not fall through to PORT", + relayServerPort: "not-a-port", + port: "8080", + want: 3000, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Empty is equivalent to unset for the override logic, and + // t.Setenv restores whatever the developer's shell had. + t.Setenv("RELAY_SERVER_PORT", tt.relayServerPort) + t.Setenv("PORT", tt.port) + + cfg := Default() + applyEnvOverrides(cfg) + + if cfg.Server.Port != tt.want { + t.Errorf("Server.Port = %d, want %d", cfg.Server.Port, tt.want) + } + }) + } +} + +// TestApplyEnvOverrides_PortDoesNotDisturbOtherSettings guards against the +// port resolution accidentally clobbering neighbouring config values. +func TestApplyEnvOverrides_PortDoesNotDisturbOtherSettings(t *testing.T) { + t.Setenv("RELAY_SERVER_PORT", "") + t.Setenv("PORT", "8080") + + cfg := Default() + applyEnvOverrides(cfg) + + if got, want := cfg.Server.Host, "0.0.0.0"; got != want { + t.Errorf("Server.Host = %q, want %q", got, want) + } + if got, want := cfg.Storage.Path, "./data/relay.db"; got != want { + t.Errorf("Storage.Path = %q, want %q", got, want) + } +} + +// TestConfigAddr checks the listen address built from the resolved port. +func TestConfigAddr(t *testing.T) { + t.Setenv("RELAY_SERVER_PORT", "") + t.Setenv("PORT", "8080") + + cfg := Default() + applyEnvOverrides(cfg) + + if got, want := cfg.Addr(), "0.0.0.0:8080"; got != want { + t.Errorf("Addr() = %q, want %q", got, want) + } +} From 9b2fd9299b89e0ee121620fbd9fa6095d5fa5c7d Mon Sep 17 00:00:00 2001 From: Atharva0506 Date: Thu, 20 Aug 2026 13:23:12 +0530 Subject: [PATCH 2/3] docs: clarify PORT fallback applies to an empty RELAY_SERVER_PORT The config table said PORT is used when RELAY_SERVER_PORT is "not set", but applyEnvOverrides falls through on an empty value too -- which is what an empty docker-compose entry (RELAY_SERVER_PORT=) produces. Say "unset or empty" so the docs match the code. Also pin the port range behaviour with tests. A value that does not parse warns and keeps the default; a value that parses but falls outside 1..65535 fails startup through Validate. That asymmetry is deliberate and predates the PORT fallback, so the tests now assert it rather than leaving it to be rediscovered. Addresses CodeRabbit review feedback on #34. --- README.md | 5 +-- internal/config/config_test.go | 62 +++++++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index b9e6770..a22f399 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,7 @@ Edit `config.yaml` or use environment variables: | Setting | YAML Key | Env Variable | Default | |---------|----------|-------------|---------| | Server port | `server.port` | `RELAY_SERVER_PORT` | `3000` | -| Server port (fallback) | — | `PORT` | _unset_ — used only when `RELAY_SERVER_PORT` is not set | +| Server port (fallback) | — | `PORT` | _unset_ — used when `RELAY_SERVER_PORT` is unset **or empty** | | Server host | `server.host` | `RELAY_SERVER_HOST` | `0.0.0.0` | | Storage path | `storage.path` | `RELAY_STORAGE_PATH` | `./data/relay.db` | | Message TTL | `messages.ttl_days` | `RELAY_MESSAGES_TTL_DAYS` | `7` (0 = forever) | @@ -208,7 +208,8 @@ Edit `config.yaml` or use environment variables: > **Deploying to a managed platform?** Render, Railway, Heroku and Cloud Run > inject a `PORT` variable and expect the process to bind to it. ThruBox reads > it automatically, so no extra configuration is needed. `RELAY_SERVER_PORT` -> still takes precedence when both are set. +> still takes precedence whenever it is set to a non-empty value, so setting it +> to `""` (as an empty `docker-compose` entry does) falls through to `PORT`. --- diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 07b858f..98b2457 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,6 +1,9 @@ package config -import "testing" +import ( + "path/filepath" + "testing" +) // TestApplyEnvOverrides_Port covers the precedence rules between the // project-specific RELAY_SERVER_PORT and the platform-conventional PORT. @@ -90,3 +93,60 @@ func TestConfigAddr(t *testing.T) { t.Errorf("Addr() = %q, want %q", got, want) } } + +// TestLoad_PortOutOfRangeIsRejected pins a deliberate asymmetry: a port value +// that does not parse is a warning and falls back to the default, but a value +// that parses and is out of the valid TCP range fails startup via Validate. +// +// The distinction is intentional. "abc" reads as an unset or placeholder +// variable, so continuing is reasonable. 0, -1 or 65536 is a real number that +// someone meant, and quietly serving on 3000 instead would leave a deployment +// answering on a port nobody configured -- exactly the confusing failure this +// change set out to remove. Validate() has enforced this for RELAY_SERVER_PORT +// since before the PORT fallback existed; the fallback inherits it unchanged. +func TestLoad_PortOutOfRangeIsRejected(t *testing.T) { + tests := []struct { + name string + relayServerPort string + port string + wantErr bool + wantPort int + }{ + {name: "RELAY_SERVER_PORT=0", relayServerPort: "0", wantErr: true}, + {name: "RELAY_SERVER_PORT=-1", relayServerPort: "-1", wantErr: true}, + {name: "RELAY_SERVER_PORT=65536", relayServerPort: "65536", wantErr: true}, + {name: "PORT=0", port: "0", wantErr: true}, + {name: "PORT=-1", port: "-1", wantErr: true}, + {name: "PORT=65536", port: "65536", wantErr: true}, + + {name: "PORT=1 is the low boundary", port: "1", wantPort: 1}, + {name: "PORT=65535 is the high boundary", port: "65535", wantPort: 65535}, + + // Unparseable is a warning, not a failure -- unchanged behaviour. + {name: "PORT=abc warns and uses the default", port: "abc", wantPort: 3000}, + {name: "RELAY_SERVER_PORT=abc warns and uses the default", relayServerPort: "abc", wantPort: 3000}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("RELAY_SERVER_PORT", tt.relayServerPort) + t.Setenv("PORT", tt.port) + + // A path that does not exist exercises defaults + env + Validate. + cfg, err := Load(filepath.Join(t.TempDir(), "absent.yaml")) + + if tt.wantErr { + if err == nil { + t.Fatalf("Load() error = nil, want a validation error") + } + return + } + if err != nil { + t.Fatalf("Load() error = %v, want nil", err) + } + if cfg.Server.Port != tt.wantPort { + t.Errorf("Server.Port = %d, want %d", cfg.Server.Port, tt.wantPort) + } + }) + } +} From 5142a54fcd70b0b0560931eb8bee732ccd496716 Mon Sep 17 00:00:00 2001 From: Atharva0506 Date: Thu, 20 Aug 2026 19:49:24 +0530 Subject: [PATCH 3/3] docs: drop the file-specific example from the test note The note named internal/config/config_test.go, which only exists once this branch lands. #36 adds test files too and had to correct the same sentence, so the two edits collided. Saying only that tests live beside the code they cover is accurate on either branch and lets the two merge without a conflict. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index a22f399..46dc1c9 100644 --- a/README.md +++ b/README.md @@ -188,7 +188,7 @@ docker compose up -d go test ./... ``` -> Tests live beside the code they cover (for example `internal/config/config_test.go`). See `CONTRIBUTING.md` before submitting a PR that adds functionality without tests. +> Tests live beside the code they cover. See `CONTRIBUTING.md` before submitting a PR that adds functionality without tests. ### Configuration