From 153fc211cf46a088ecf1fbac22732e84079fc839 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Sat, 3 Oct 2026 12:38:39 +0700 Subject: [PATCH] refactor!: move every config default into compose.yml compose.yml now holds each default as ${VAR:-default}, and the code keeps no fallback values. The bot stops at startup on a missing or invalid PORT or LOG_LEVEL, /addsticker refuses without STICKER_PACK_NAME, and the renderer refuses to start until every RENDERER_* setting is a valid value, listing each problem. Settings whose empty value means none or all (MODULES, OWNER_ID, ADMIN_IDS, the API tokens) use ${VAR:-}. The renderer's npm start and dev load .env when present, so a local run works from a copy of .env.example. BREAKING CHANGE: running outside compose now requires LOG_LEVEL and PORT for the bot, STICKER_PACK_NAME for /addsticker, and every RENDERER_* variable for the renderer. --- .env.example | 13 ++++- README.md | 7 +++ cmd/server/main.go | 16 +++--- compose.yml | 32 +++++------ docs/deploy-coolify-selfhosted.md | 11 +++- docs/sticker-packs.md | 6 +- internal/log/log.go | 37 ++++++++----- internal/log/log_test.go | 17 ++++-- .../sticker/addsticker_command_test.go | 13 +++-- internal/modules/sticker/sticker_pack.go | 15 ++--- internal/modules/sticker/sticker_pack_test.go | 18 ++++-- renderer/.env.example | 2 + renderer/README.md | 6 +- renderer/docs/deployment.md | 10 ++-- renderer/package.json | 4 +- renderer/src/config.js | 55 ++++++++++++++----- renderer/test/config.test.js | 40 +++++++++++++- 17 files changed, 208 insertions(+), 94 deletions(-) diff --git a/.env.example b/.env.example index e879f13..2c8e45c 100644 --- a/.env.example +++ b/.env.example @@ -25,7 +25,8 @@ ADMIN_IDS= # MUST end in _by_: Telegram requires that suffix on sets a # bot creates and refuses to edit sets it did not create, so the suffix is what # proves the pack is manageable. Created automatically, owned by OWNER_ID, on -# the first /addsticker if it does not exist yet. Unset = the default below. +# the first /addsticker if it does not exist yet. Required by /addsticker: +# the code has no fallback (compose.yml supplies this value as its default). STICKER_PACK_NAME=miti99_by_miti99bot # SOURCE_COMMIT (commit SHA) is read at startup for the deploynotify owner DM. @@ -44,10 +45,16 @@ LOL_PANDASCORE_TOKEN= # only when running the bot outside compose. Leave blank to fall back to text. RENDERER_URL= +# ============================ Runtime ============================= +# Required: the code has no fallback. compose.yml supplies these values as its +# defaults; set them yourself when running outside compose. +# Log level: debug, info, warn, or error. +LOG_LEVEL=info +# Internal health server port. +PORT=8080 + # ====================== Leave UNSET on self-host ================== -# Defaults are correct for self-host: # KV_PROVIDER — auto-selects mongodb because MONGO_URL is set -# PORT — defaults to 8080 (internal health server) # TELEGRAM_WEBHOOK_SECRET — long polling has no webhook # GOLD_VNAPP_API_KEY — gold module auto-fetches + caches the key to Mongo # Stock/coin/gold URL env overrides are not supported; modules use coded diff --git a/README.md b/README.md index 1b1fdf9..5bf6d8a 100644 --- a/README.md +++ b/README.md @@ -262,6 +262,8 @@ shell, then run the server with Go: ```powershell # PowerShell $env:TELEGRAM_BOT_TOKEN = "…" +$env:LOG_LEVEL = "info" +$env:PORT = "8080" $env:MODULES = "" go run ./cmd/server ``` @@ -269,10 +271,15 @@ go run ./cmd/server ```sh # POSIX shells (Linux/macOS) export TELEGRAM_BOT_TOKEN="…" +export LOG_LEVEL=info +export PORT=8080 export MODULES="" go run ./cmd/server ``` +The code has no default values: `compose.yml` supplies them, so a local run +sets `LOG_LEVEL` and `PORT` itself (and `STICKER_PACK_NAME` for `/addsticker`). + The bot uses long polling, so a local run talks to Telegram directly — no `ngrok` or public URL. The server clears any existing webhook on startup. The dev bot is created manually; its token is injected through the environment. diff --git a/cmd/server/main.go b/cmd/server/main.go index bbb6775..02e9d30 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -368,9 +368,9 @@ type config struct { MongoDatabase string // required when KVProvider=mongodb } -// loadConfig reads config from the environment. PORT defaults to 8080 and an -// invalid PORT is fatal; malformed OWNER_ID / ADMIN_IDS entries are logged and -// ignored. +// loadConfig reads config from the environment. compose.yml supplies every +// default, so the code keeps none: a missing or invalid PORT or LOG_LEVEL is +// fatal. Malformed OWNER_ID / ADMIN_IDS entries are logged and ignored. func loadConfig() config { envMap := make(map[string]string, len(os.Environ())) for _, kv := range os.Environ() { @@ -378,15 +378,17 @@ func loadConfig() config { envMap[kv[:eq]] = kv[eq+1:] } } - port := envMap["PORT"] - if port == "" { - port = "8080" + level, err := log.ParseLevel(envMap["LOG_LEVEL"]) + if err != nil { + log.Fatal("invalid LOG_LEVEL", "err", err) } + log.SetLevel(level) + port := strings.TrimSpace(envMap["PORT"]) // PORT must be a number in 0..65535. http.Server uses ":" verbatim, // so a junk value would otherwise surface only at ListenAndServe time; // fail fast here instead. if n, err := strconv.Atoi(port); err != nil || n < 0 || n > 65535 { - log.Fatal("invalid PORT", "value", port) + log.Fatal("missing or invalid PORT", "value", port) } return config{ Port: port, diff --git a/compose.yml b/compose.yml index b3d02b8..914e124 100644 --- a/compose.yml +++ b/compose.yml @@ -11,18 +11,18 @@ services: MONGO_URL: ${MONGO_URL} # Atlas SRV string incl. credentials — SECRET MONGO_DATABASE: ${MONGO_DATABASE} # e.g. miti99bot - # --- Access control (optional; empty uses the default) --- - MODULES: ${MODULES} # CSV of modules; default: all modules - OWNER_ID: ${OWNER_ID} # Telegram user id for owner-only commands; default: none - ADMIN_IDS: ${ADMIN_IDS} # CSV of admin Telegram user ids; default: none + # --- Access control (optional; empty means none / all) --- + MODULES: ${MODULES:-} # CSV of modules; empty = all modules + OWNER_ID: ${OWNER_ID:-} # Telegram user id for owner-only commands; empty = none + ADMIN_IDS: ${ADMIN_IDS:-} # CSV of admin Telegram user ids; empty = none - # --- Module settings (optional; empty uses the default) --- - LOL_PANDASCORE_TOKEN: ${LOL_PANDASCORE_TOKEN} # PandaScore token — SECRET; default: none (/lol* fetches fail) - GOLD_VNAPP_API_KEY: ${GOLD_VNAPP_API_KEY} # VNAppMob key — SECRET; default: fetched and cached in Mongo - STICKER_PACK_NAME: ${STICKER_PACK_NAME} # /addsticker set; default: miti99_by_miti99bot + # --- Module settings (optional) --- + LOL_PANDASCORE_TOKEN: ${LOL_PANDASCORE_TOKEN:-} # PandaScore token — SECRET; empty = /lol* fetches fail + GOLD_VNAPP_API_KEY: ${GOLD_VNAPP_API_KEY:-} # VNAppMob key — SECRET; empty = fetched and cached in Mongo + STICKER_PACK_NAME: ${STICKER_PACK_NAME:-miti99_by_miti99bot} # /addsticker set; must end in _by_ - # --- Runtime (optional; empty uses the default) --- - LOG_LEVEL: ${LOG_LEVEL} # debug|info|warn|error; default: info + # --- Runtime (defaults below; the code has no fallback) --- + LOG_LEVEL: ${LOG_LEVEL:-info} # debug|info|warn|error # --- Fixed by this stack (not Coolify settings) --- # The bundled renderer service below draws /wheelofnames, /gacha and @@ -30,12 +30,12 @@ services: # platform-level value cannot point the bot elsewhere; the bot appends # each /api/ route itself. RENDERER_URL: http://renderer:3000 + PORT: "8080" # health server; keep in sync with the healthcheck below # --- Deliberately not declared --- # SOURCE_COMMIT: Coolify provides it at runtime via its generated env # file; declaring it here with Compose interpolation can override the # runtime value with an empty string. - # PORT: the health server defaults to 8080, which the healthcheck uses. # KV_PROVIDER: storage auto-selects mongodb because MONGO_URL is set. # Long polling = no TELEGRAM_WEBHOOK_SECRET, no /webhook, no public domain. # Cron is in-process only — no CRON_MODE, no /cron route, no secret. @@ -70,11 +70,11 @@ services: # Every renderer setting carries the RENDERER_ prefix so it reads as a # renderer setting and cannot clash with the bot's variables. - # --- Tuning (optional; empty uses the default) --- - RENDERER_MAX_CONCURRENT_RENDERS: ${RENDERER_MAX_CONCURRENT_RENDERS} # default: 1 - RENDERER_RENDER_TIMEOUT_MS: ${RENDERER_RENDER_TIMEOUT_MS} # default: 15000 (floor 7000) - RENDERER_MAX_OPTIONS: ${RENDERER_MAX_OPTIONS} # default: 32 wheel options - RENDERER_MAX_OPTION_CHARS: ${RENDERER_MAX_OPTION_CHARS} # default: 40 chars per option/label + # --- Tuning (defaults below; the renderer has no fallback) --- + RENDERER_MAX_CONCURRENT_RENDERS: ${RENDERER_MAX_CONCURRENT_RENDERS:-1} + RENDERER_RENDER_TIMEOUT_MS: ${RENDERER_RENDER_TIMEOUT_MS:-15000} # floor 7000 + RENDERER_MAX_OPTIONS: ${RENDERER_MAX_OPTIONS:-32} # wheel options + RENDERER_MAX_OPTION_CHARS: ${RENDERER_MAX_OPTION_CHARS:-40} # chars per option/label # --- Fixed by this stack (not Coolify settings) --- NODE_ENV: production diff --git a/docs/deploy-coolify-selfhosted.md b/docs/deploy-coolify-selfhosted.md index d4d488d..f7e4382 100644 --- a/docs/deploy-coolify-selfhosted.md +++ b/docs/deploy-coolify-selfhosted.md @@ -34,15 +34,20 @@ Copy [`.env.example`](../.env.example) → `.env` (gitignored) and fill in. | `MODULES` | optional | CSV; empty = all modules, including any added later | | `OWNER_ID` | optional | Telegram user id for owner-only commands, the deploy DM, and the `/addsticker` pack owner. Unset = owner-only commands are denied and `/addsticker` refuses | | `ADMIN_IDS` | optional | CSV of Telegram user ids for admin-only commands | -| `STICKER_PACK_NAME` | optional | set `/addsticker` writes to; default `miti99_by_miti99bot`. See [sticker packs](sticker-packs.md) | +| `STICKER_PACK_NAME` | optional | set `/addsticker` writes to; `compose.yml` default `miti99_by_miti99bot`. See [sticker packs](sticker-packs.md) | | `LOL_PANDASCORE_TOKEN` | optional | PandaScore API token for the lol module (free tier) — secret, never logged; without it every `/lol*` fetch fails (stale cache may still serve briefly) | | `RENDERER_URL` | leave unset | base URL of the animation renderer; fixed by `compose.yml` to the bundled renderer (`http://renderer:3000`), so a Coolify value is ignored | -| `LOG_LEVEL` | optional | `debug`, `info` (default), `warn`, or `error`; logs are JSON on stdout | +| `LOG_LEVEL` | optional | `debug`, `info`, `warn`, or `error`; `compose.yml` default `info`; logs are JSON on stdout | | `GOLD_VNAPP_API_KEY` | optional | VNAppMob key — secret; empty = the gold module fetches one and caches it in MongoDB | | `KV_PROVIDER` | leave unset | `memory` or `mongodb`; unset = `mongodb` when `MONGO_URL` is set, otherwise `memory` | -| `PORT` | leave unset | health server port; default `8080` | +| `PORT` | leave unset | health server port; fixed to `8080` by `compose.yml`, which the health check uses | | `SOURCE_COMMIT` | never set | provided by Coolify at runtime for the deploy DM (see step 5 below) | +Defaults live in `compose.yml` only (`${VAR:-default}`); the code has no +fallback values. `PORT`, `LOG_LEVEL`, and (for `/addsticker`) +`STICKER_PACK_NAME` must therefore be set when running outside compose — a +missing or invalid `PORT` or `LOG_LEVEL` stops the bot at startup. + Stock, coin, and gold provider URL overrides are not supported in runtime env; modules use coded defaults. There is no `TELEGRAM_WEBHOOK_SECRET`: long polling has no webhook. diff --git a/docs/sticker-packs.md b/docs/sticker-packs.md index 82ffc16..c1208d9 100644 --- a/docs/sticker-packs.md +++ b/docs/sticker-packs.md @@ -22,14 +22,14 @@ Single-shot: one message replying to the media to add. No conversation state. ## Configuration -| Env | Default | Meaning | +| Env | `compose.yml` default | Meaning | |---|---|---| -| `STICKER_PACK_NAME` | `miti99_by_miti99bot` | The Telegram set to write to | +| `STICKER_PACK_NAME` | `miti99_by_miti99bot` | The Telegram set to write to; required — unset, `/addsticker` refuses | | `OWNER_ID` | — | Must be the account that **owns** that set | `OWNER_ID` is reused rather than given a sticker-specific twin because `addStickerToSet` takes the **set owner's** user ID, not the caller's, and the -default pack belongs to the bot owner. Point `OWNER_ID` at the owning account if +standard pack belongs to the bot owner. Point `OWNER_ID` at the owning account if the configured pack belongs to someone else. The caller's identity is used nowhere. That is what makes the command stateless: diff --git a/internal/log/log.go b/internal/log/log.go index 197c336..4c7d995 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -19,36 +19,43 @@ package log import ( "context" + "fmt" "log/slog" "os" "strings" ) -// defaultLogger is constructed at init from LOG_LEVEL. Tests can swap it via -// SetDefault — but the public Info/Warn/Error/Fatal helpers always read the -// current default so test substitutions take effect immediately. -var defaultLogger *slog.Logger +// level is the default logger's minimum level. It starts at Info so logging +// works before configuration is read (and in tests); SetLevel applies the +// configured LOG_LEVEL at startup. +var level = new(slog.LevelVar) -func init() { - defaultLogger = slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{ - Level: parseLevel(os.Getenv("LOG_LEVEL")), - })) -} +// defaultLogger writes JSON to stdout. Tests can swap it via SetDefault — but +// the public Info/Warn/Error/Fatal helpers always read the current default so +// test substitutions take effect immediately. +var defaultLogger = slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: level})) -// parseLevel maps LOG_LEVEL env to a slog.Level. Unknown / empty → Info. -func parseLevel(s string) slog.Level { +// ParseLevel maps a LOG_LEVEL value to a slog.Level. There is no fallback: an +// empty or unknown value is an error, so a typo cannot silently change what +// gets logged. +func ParseLevel(s string) (slog.Level, error) { switch strings.ToLower(strings.TrimSpace(s)) { case "debug": - return slog.LevelDebug + return slog.LevelDebug, nil + case "info": + return slog.LevelInfo, nil case "warn", "warning": - return slog.LevelWarn + return slog.LevelWarn, nil case "error": - return slog.LevelError + return slog.LevelError, nil default: - return slog.LevelInfo + return 0, fmt.Errorf("invalid log level %q: want debug, info, warn, or error", s) } } +// SetLevel sets the default logger's minimum level. +func SetLevel(l slog.Level) { level.Set(l) } + // SetDefault swaps the package-level logger. Used by tests to capture output; // production code never calls this. func SetDefault(l *slog.Logger) { defaultLogger = l } diff --git a/internal/log/log_test.go b/internal/log/log_test.go index 29c8339..c5bba0c 100644 --- a/internal/log/log_test.go +++ b/internal/log/log_test.go @@ -32,8 +32,7 @@ func decodeOne(t *testing.T, buf *bytes.Buffer) map[string]any { } func TestParseLevel(t *testing.T) { - tests := map[string]slog.Level{ - "": slog.LevelInfo, + valid := map[string]slog.Level{ "info": slog.LevelInfo, "INFO": slog.LevelInfo, "debug": slog.LevelDebug, @@ -41,11 +40,17 @@ func TestParseLevel(t *testing.T) { "warning": slog.LevelWarn, "error": slog.LevelError, " Error ": slog.LevelError, - "bogus": slog.LevelInfo, } - for in, want := range tests { - if got := parseLevel(in); got != want { - t.Errorf("parseLevel(%q) = %v, want %v", in, got, want) + for in, want := range valid { + got, err := ParseLevel(in) + if err != nil || got != want { + t.Errorf("ParseLevel(%q) = %v, %v; want %v", in, got, err, want) + } + } + // No fallback: empty and unknown values are configuration errors. + for _, in := range []string{"", " ", "bogus"} { + if _, err := ParseLevel(in); err == nil { + t.Errorf("ParseLevel(%q) = nil error, want an error", in) } } } diff --git a/internal/modules/sticker/addsticker_command_test.go b/internal/modules/sticker/addsticker_command_test.go index f37e483..89f3839 100644 --- a/internal/modules/sticker/addsticker_command_test.go +++ b/internal/modules/sticker/addsticker_command_test.go @@ -94,17 +94,18 @@ func TestAddSticker_NonOwnerWritesToConfiguredPack(t *testing.T) { } } -func TestAddSticker_DefaultsToMiti99Pack(t *testing.T) { +// There is no fallback pack: an unset STICKER_PACK_NAME must not write +// anywhere. +func TestAddSticker_RequiresPackName(t *testing.T) { rb := installAddSticker(t, "", "miti99bot") rb.Bot.ProcessUpdate(context.Background(), stickerReply(999, "", "src", "")) - call, ok := callTo(rb, "addStickerToSet") - if !ok { - t.Fatalf("no addStickerToSet call; got %+v", rb.Sent()) + if _, ok := callTo(rb, "addStickerToSet"); ok { + t.Fatal("addStickerToSet called without a configured pack name") } - if got := call.Form["name"]; got != "miti99_by_miti99bot" { - t.Errorf("name = %q, want the default pack", got) + if _, ok := callTo(rb, "createNewStickerSet"); ok { + t.Fatal("createNewStickerSet called without a configured pack name") } } diff --git a/internal/modules/sticker/sticker_pack.go b/internal/modules/sticker/sticker_pack.go index f366231..c88556c 100644 --- a/internal/modules/sticker/sticker_pack.go +++ b/internal/modules/sticker/sticker_pack.go @@ -17,18 +17,15 @@ import ( ) const ( - // stickerPackNameEnv overrides which set /addsticker writes to. The name - // must end in "_by_", the only thing that makes a set + // stickerPackNameEnv names the set /addsticker writes to; compose.yml + // supplies its default. The name must end in "_by_", the only thing that makes a set // bot-manageable; packTitle checks it before any upload. A set that does // not exist yet is created by the first successful /addsticker. stickerPackNameEnv = "STICKER_PACK_NAME" - // defaultStickerPackName is the shared pack used when the env is unset. - defaultStickerPackName = "miti99_by_miti99bot" - // stickerPackOwnerEnv reuses the bot-wide owner setting rather than // introducing a second variable: AddStickerToSet needs the *set owner's* - // user ID, and the default pack above belongs to the bot owner. A pack + // user ID, and the standard pack belongs to the bot owner. A pack // owned by any other account needs this env pointed at that account. stickerPackOwnerEnv = "OWNER_ID" @@ -78,6 +75,10 @@ type stickerPack struct { OwnerID int64 // the account the set belongs to; AddStickerToSet demands it } +// errNoPackName means STICKER_PACK_NAME is unset, so no sticker can be added. +// Internal, not user-facing: nothing the caller does fixes a misconfiguration. +var errNoPackName = errors.New("util: sticker pack name unset") + // errNoPackOwner means the owner ID is unset, so no sticker can be added. // Internal, not user-facing: nothing the caller does fixes a misconfiguration. var errNoPackOwner = errors.New("util: sticker pack owner ID unset") @@ -90,7 +91,7 @@ var errNoPackOwner = errors.New("util: sticker pack owner ID unset") func loadStickerPack() (stickerPack, error) { name := strings.TrimSpace(os.Getenv(stickerPackNameEnv)) if name == "" { - name = defaultStickerPackName + return stickerPack{}, errNoPackName } ownerID, err := strconv.ParseInt(strings.TrimSpace(os.Getenv(stickerPackOwnerEnv)), 10, 64) if err != nil || ownerID == 0 { diff --git a/internal/modules/sticker/sticker_pack_test.go b/internal/modules/sticker/sticker_pack_test.go index 9227f4e..662952a 100644 --- a/internal/modules/sticker/sticker_pack_test.go +++ b/internal/modules/sticker/sticker_pack_test.go @@ -106,26 +106,36 @@ func TestPackTitle_AtNameLengthCap(t *testing.T) { } func TestLoadStickerPack(t *testing.T) { - t.Run("defaults the name and requires an owner", func(t *testing.T) { + t.Run("reads the name and owner", func(t *testing.T) { t.Setenv("OWNER_ID", "42") - t.Setenv("STICKER_PACK_NAME", "") + t.Setenv("STICKER_PACK_NAME", "miti99_by_miti99bot") pack, err := loadStickerPack() if err != nil { t.Fatalf("loadStickerPack: %v", err) } - if pack.Name != defaultStickerPackName { - t.Errorf("name = %q, want %q", pack.Name, defaultStickerPackName) + if pack.Name != "miti99_by_miti99bot" { + t.Errorf("name = %q, want miti99_by_miti99bot", pack.Name) } if pack.OwnerID != 42 { t.Errorf("ownerID = %d, want 42", pack.OwnerID) } }) + // No fallback name: an unset STICKER_PACK_NAME is a misconfiguration. + t.Run("requires a name", func(t *testing.T) { + t.Setenv("OWNER_ID", "42") + t.Setenv("STICKER_PACK_NAME", "") + if _, err := loadStickerPack(); !errors.Is(err, errNoPackName) { + t.Errorf("loadStickerPack() err = %v, want errNoPackName", err) + } + }) + // A zero owner is the unset case, not a valid user: AddStickerToSet needs a // real account, so it must fail here rather than at the API. for _, owner := range []string{"", "0", "not-a-number"} { t.Run("rejects owner "+owner, func(t *testing.T) { t.Setenv("OWNER_ID", owner) + t.Setenv("STICKER_PACK_NAME", "miti99_by_miti99bot") if _, err := loadStickerPack(); !errors.Is(err, errNoPackOwner) { t.Errorf("loadStickerPack() err = %v, want errNoPackOwner", err) } diff --git a/renderer/.env.example b/renderer/.env.example index 2e9fb45..f8083fa 100644 --- a/renderer/.env.example +++ b/renderer/.env.example @@ -1,3 +1,5 @@ +# Every RENDERER_* setting is required; the renderer has no fallback values. +# The root compose.yml supplies these same values as its defaults. RENDERER_PORT=3000 RENDERER_HOST=0.0.0.0 NODE_ENV=production diff --git a/renderer/README.md b/renderer/README.md index f6f0510..54dbf32 100644 --- a/renderer/README.md +++ b/renderer/README.md @@ -108,9 +108,11 @@ npm install npm run browser:ensure ``` -Start the local API: +Start the local API. Every `RENDERER_*` setting is required, so copy the +template first; `npm run dev` loads `.env`: ```sh +cp .env.example .env npm run dev ``` @@ -178,7 +180,7 @@ dependencies, and FFmpeg/compositor support. ```sh docker build -t miti99bot-renderer . -docker run --rm -p 3000:3000 miti99bot-renderer +docker run --rm -p 3000:3000 --env-file .env.example miti99bot-renderer ``` Recommended starting resources: 1-2 vCPU and 1-2 GB RAM, with diff --git a/renderer/docs/deployment.md b/renderer/docs/deployment.md index 9e60694..a8e5db0 100644 --- a/renderer/docs/deployment.md +++ b/renderer/docs/deployment.md @@ -28,7 +28,7 @@ Avoid for v1: ## Runtime -Env vars, all optional (defaults shown): +Env vars, all required (standard values shown): ```sh RENDERER_PORT=3000 @@ -39,9 +39,11 @@ RENDERER_MAX_OPTIONS=32 RENDERER_MAX_OPTION_CHARS=40 ``` -An unset or empty variable uses the default above. The root `compose.yml` -fixes `RENDERER_HOST` and `RENDERER_PORT` and passes the tuning values through -without defaults, so leaving them empty in Coolify uses these defaults. +The renderer has no fallback values: it refuses to start and lists every +missing or invalid variable. The root `compose.yml` owns the defaults — it +fixes `RENDERER_HOST` and `RENDERER_PORT` and gives each tuning value a +`${VAR:-default}`, so leaving them empty in Coolify uses these values. For a +local run, copy `.env.example` to `.env`; `npm run dev` and `npm start` load it. Start with 1-2 vCPU and 1-2 GB RAM. Increase only after render benchmarks show the service is CPU-bound or concurrency-limited. diff --git a/renderer/package.json b/renderer/package.json index b43c8ce..dd4a875 100644 --- a/renderer/package.json +++ b/renderer/package.json @@ -7,8 +7,8 @@ "node": ">=24 <25" }, "scripts": { - "dev": "node --watch src/server.js", - "start": "node src/server.js", + "dev": "node --env-file-if-exists=.env --watch src/server.js", + "start": "node --env-file-if-exists=.env src/server.js", "lint": "eslint .", "typecheck": "tsc --noEmit", "test": "vitest run", diff --git a/renderer/src/config.js b/renderer/src/config.js index 4c72922..4b6fd67 100644 --- a/renderer/src/config.js +++ b/renderer/src/config.js @@ -11,33 +11,58 @@ export const minRenderTimeoutMs = 7000; */ /** - * @param {string | undefined} value - * @param {number} fallback + * Reads a required positive integer setting, recording a problem instead of + * falling back: compose.yml owns every default, so a missing value is a + * deployment mistake to report, not to paper over. + * + * @param {NodeJS.ProcessEnv} env + * @param {string} name + * @param {string[]} problems * @returns {number} */ -const parsePositiveInt = (value, fallback) => { - if (!value) { - return fallback; +const requirePositiveInt = (env, name, problems) => { + const raw = env[name]?.trim(); + if (!raw) { + problems.push(`${name} is required`); + return 0; } - - const parsed = Number.parseInt(value, 10); - return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback; + const parsed = Number(raw); + if (!Number.isInteger(parsed) || parsed <= 0) { + problems.push(`${name} must be a positive integer, got "${raw}"`); + return 0; + } + return parsed; }; /** + * Loads the renderer settings. Every RENDERER_* variable is required; the + * deployment (compose.yml, or .env for local runs) supplies the values. + * * @param {NodeJS.ProcessEnv} [env] * @returns {AppConfig} */ export const loadConfig = (env = process.env) => { - return { - host: env.RENDERER_HOST || '0.0.0.0', - port: parsePositiveInt(env.RENDERER_PORT, 3000), - maxConcurrentRenders: parsePositiveInt(env.RENDERER_MAX_CONCURRENT_RENDERS, 1), + /** @type {string[]} */ + const problems = []; + const host = env.RENDERER_HOST?.trim() ?? ''; + if (!host) { + problems.push('RENDERER_HOST is required'); + } + const config = { + host, + port: requirePositiveInt(env, 'RENDERER_PORT', problems), + maxConcurrentRenders: requirePositiveInt(env, 'RENDERER_MAX_CONCURRENT_RENDERS', problems), + // Remotion's browser timeout cannot go below 7000ms, so lower values are + // raised to that floor rather than rejected. renderTimeoutMs: Math.max( minRenderTimeoutMs, - parsePositiveInt(env.RENDERER_RENDER_TIMEOUT_MS, 15000), + requirePositiveInt(env, 'RENDERER_RENDER_TIMEOUT_MS', problems), ), - maxOptions: parsePositiveInt(env.RENDERER_MAX_OPTIONS, 32), - maxOptionChars: parsePositiveInt(env.RENDERER_MAX_OPTION_CHARS, 40), + maxOptions: requirePositiveInt(env, 'RENDERER_MAX_OPTIONS', problems), + maxOptionChars: requirePositiveInt(env, 'RENDERER_MAX_OPTION_CHARS', problems), }; + if (problems.length > 0) { + throw new Error(`invalid renderer configuration: ${problems.join('; ')}`); + } + return config; }; diff --git a/renderer/test/config.test.js b/renderer/test/config.test.js index 42375d9..9274519 100644 --- a/renderer/test/config.test.js +++ b/renderer/test/config.test.js @@ -1,10 +1,48 @@ import {describe, expect, test} from 'vitest'; import {loadConfig, minRenderTimeoutMs} from '../src/config.js'; +const validEnv = { + RENDERER_HOST: '0.0.0.0', + RENDERER_PORT: '3000', + RENDERER_MAX_CONCURRENT_RENDERS: '1', + RENDERER_RENDER_TIMEOUT_MS: '15000', + RENDERER_MAX_OPTIONS: '32', + RENDERER_MAX_OPTION_CHARS: '40', +}; + describe('loadConfig', () => { + test('reads every RENDERER_* setting', () => { + expect(loadConfig(validEnv)).toEqual({ + host: '0.0.0.0', + port: 3000, + maxConcurrentRenders: 1, + renderTimeoutMs: 15000, + maxOptions: 32, + maxOptionChars: 40, + }); + }); + test('keeps render timeout compatible with Remotion browser timeout limits', () => { - const config = loadConfig({RENDERER_RENDER_TIMEOUT_MS: '500'}); + const config = loadConfig({...validEnv, RENDERER_RENDER_TIMEOUT_MS: '500'}); expect(config.renderTimeoutMs).toBe(minRenderTimeoutMs); }); + + test('has no fallback: missing settings are all reported', () => { + expect(() => loadConfig({})).toThrow( + /RENDERER_HOST is required.*RENDERER_PORT is required.*RENDERER_MAX_OPTION_CHARS is required/, + ); + }); + + test('treats an empty value as missing', () => { + expect(() => loadConfig({...validEnv, RENDERER_MAX_OPTIONS: ''})).toThrow( + /RENDERER_MAX_OPTIONS is required/, + ); + }); + + test.each(['0', '-1', '1.5', 'abc'])('rejects non-positive-integer %s', (value) => { + expect(() => loadConfig({...validEnv, RENDERER_MAX_OPTIONS: value})).toThrow( + /RENDERER_MAX_OPTIONS must be a positive integer/, + ); + }); });