feat: validate environment variables at startup with a typed config#121
Conversation
|
On it, @rubenhensen! I'm starting my review of your PR now — I'll check the zod-based env config validation for correctness and rule-compliance and post a consolidated review shortly. |
There was a problem hiding this comment.
Reviewed the typed env-config change. It's a clean, well-scoped refactor: a single zod-validated source of truth parsed once at startup, consumers routed through it, and the old === 'true' boolean semantics preserved exactly (case-sensitive; TRUE/unset → false). Tests cover defaults, coercion, and the required-DATABASE_URL rejection.
Verified locally: 94/94 unit tests pass, svelte-check is clean, and the prod build succeeds with a dummy DATABASE_URL. Build without DATABASE_URL fails, but main fails identically (the old db/index.ts threw the same) — pre-existing parity, not a regression, matching the PR's claim of no new required vars.
No rule-compliance issues found. One non-blocking nit below on the empty-string edge case for YIVI_SERVER_URL. Not blocking merge.
|
Reviewed PR #121 — no rule violations found. I left one non-blocking nit: using |
Closes #98.
Adds a single typed, validated source of truth for the server's environment config, so a missing/mistyped var fails fast at startup with one aggregated error instead of surfacing at first use.
What's added
src/lib/server/config.ts— azodschema parsed once at load.DATABASE_URLis required;YIVI_SERVER_URL/YIVI_SERVER_TOKEN/YIVI_DEMO_ATTRIBUTESand the elevenFF_*flags are coerced/defaulted. On failure it throws with every problem listed. Exports a typedconfigand theconfigSchema(for testing).db/index.ts(DATABASE_URL),auth/yivi.ts(YIVI_*, keeping its exported constants intact), andfeature-flags.ts(FF_*).tests/unit/config.test.ts— covers defaults, boolean coercion, and the missing/emptyDATABASE_URLrejection.Deliberate scoping / safety
DATABASE_URLis required — exactly whatdb/index.tsalready enforced — so the Docker build (which sets a dummyDATABASE_URL) and CI are unaffected.YIVI_*stay optional with the same defaults as before.=== 'true'semantics exactly (only the literal"true"; unset → false), so no flag silently flips.db/index.tsvia the existingintFromEnv(and its unit test from feat: configure explicit Postgres connection pool and timeouts #114) rather than moving them into config — avoids churning merged, tested code for no behavioural gain.Verified:
svelte-checkclean, all 94 unit tests pass locally with noDATABASE_URLset (config-touching tests mock$env).