[therealbibson]

Closes #248

• Document every runtime environment variable in .env.example without real credentials.
• Validate required database URLs and conditional mainnet RPC configuration before loading application modules; report all missing values together.
• Add tests for env documentation coverage and startup validation behavior.

Validation: targeted env tests and TypeScript build pass. The full npm test run encounters the pre-existing OpenAPI snapshot mismatch noted in #248 and did not complete before hanging.

[drips-wave]

@therealbibson Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[Miracle656]

Validating the environment at startup is the right thing and src/validateEnv.ts is a good shape for it. One blocker, which is your own test failing against your own change.

● docker-compose.yml › declares every env key .env.example documents

    - Array []
    + Array [
    +   "DATABASE_REPLICAS",
    +   "NODE_ENV",
    +   "SOROBAN_RPC_URL_TESTNET",
    +   "SOROBAN_RPC_URL_MAINNET",
    …38 in total

The PR touches .env.example, src/validateEnv.ts, src/index.ts and the test — not docker-compose.yml. So it documents 38 keys and simultaneously asserts that compose declares every documented key, which it does not. 731 tests, 1 failed, and it is this one.

Two honest ways out, and I do not mind which:

Add the keys to compose. If the intent is that a docker compose up gives you a working service, then every key an operator must set belongs there with a sensible default or an explicit empty. That is 38 entries and a judgement call per key about what the default should be — which is why I have not just done it for you.

Or scope the test to the keys that are actually required. A compose file does not need to declare every optional variable — DATABASE_REPLICAS and NODE_ENV have reasonable defaults and an operator who never sets them is fine. If validateEnv.ts already knows which keys are required in production, that set is the right one for this assertion, and it makes the test say something true and useful rather than something aspirational.

The second is probably the better test: "compose declares everything you must set" is a property worth enforcing forever; "compose mentions every variable that exists" will be false again the next time someone adds an optional flag.

One thing I would like either way: .env.example is the file operators read, and it is also the file the sibling repo's drift guard checks. Whatever the test ends up asserting, make sure a key that is required in production is impossible to miss — validating at startup is only half of it if the example file does not say the variable exists.

Everything else passes — 730 of 731 — and tsc --noEmit is clean. Pick either route and I will merge it.

[Miracle656]

Merged, with three changes — the first is the one worth reading.

src/index.ts: I kept main's imports rather than your require block. Converting every import to const x = require(...) as typeof import(...) does achieve ordering, but your block predated #215 and so omitted startOhlcRefreshWorker and getOhlcRefreshIntervalMs — taking it would have silently unwired the OHLC worker. That is the quiet hazard of rewriting an entrypoint's whole import list in a PR about something else: it freezes the file as it looked when you branched.

Worth knowing for next time: this project compiles with module: commonjs, where an import statement is a sequential require. So a side-effecting import placed first gets you the same ordering guarantee in one line, without restyling the file.

validateEnv() moved into main(). At module scope it threw whenever anything imported the entrypoint, which broke all 19 cases of ohlcStartup.test.ts — that suite boots the real index.ts. Inside main() it still runs before any listener opens, which is your stated goal.

docker-compose.yml had drifted from .env.example by 36 keys, which dockerCompose.test.ts caught on the merge. I added them as unset passthroughs (KEY: ${KEY:-}), so a compose run matches a host run and no secret value is committed.

ohlcStartup.test.ts now sets the database URLs and a mainnet RPC in its beforeEach — it starts the real entrypoint, so it has to supply what a real start requires. Nothing connects to either; every module that would is mocked.

The feature itself is right: validateEnv as a pure function taking env as a parameter is what makes it testable without touching the process environment, and failing at startup with the name of what is missing beats a connection error later that says nothing about configuration.

Verified on the merge result: tsc --noEmit silent, 998 tests across 65 suites.

[Miracle656]

Follow-up, and the fault here is mine rather than yours.

Merging this took the deployed service down. Build succeeded, then npm start exited 1 in a loop:

Error: [wraith] Missing required env vars: SOROBAN_RPC_URL_MAINNET
    at validateEnv (dist/validateEnv.js:24:15)
    at main (dist/index.js:30:35)

The deployment runs with mainnet among its NETWORKS and no SOROBAN_RPC_URL_MAINNET, and has done for months. The validator made that fatal, so it took down the HTTP API, every network's reads, and the naira rails — none of which touch Soroban RPC at all.

This repo had already fixed exactly this, and left a comment saying so, in planNetworks() in src/indexer.ts:

validateNetworkConfig(networks) threw for the whole set if any one entry was misconfigured, which broke the isolation promised in startAllIndexers in the worst way: an unset SOROBAN_RPC_URL_MAINNET killed the process during main(), taking testnet indexing AND the HTTP API with it. The API does not index. The naira rails are plain HTTP to Linq and never touch Soroban RPC, so a missing mainnet endpoint was stopping /ngn from answering for a reason that has nothing to do with it.

Serving reads on the networks that are configured, while saying loudly which one is not, beats refusing to serve anything.

A stricter gate in front of that undid it. I reviewed your function on its own merits and never checked it against the live configuration — that is the review I owed you and did not do.

Fixed in ea34724: validateEnv now checks only DATABASE_URL and DIRECT_DATABASE_URL, with the reasoning and the rule for future additions in its doc comment. The useful principle, which your PR is still the right vehicle for: a boot-time check may only cover what makes the whole process useless. The database qualifies — every route reads or writes. A per-network endpoint does not, because the networks are independent. Anything partial belongs in planNetworks(), where it degrades and reports rather than refusing.

Two of your tests needed inverting, and one of them is worth pointing at:

it("requires a network-specific RPC URL when indexing multiple networks", () => {
  // STELLAR_NETWORK: testnet, NETWORKS: testnet,mainnet, SOROBAN_RPC_URL only
  ).toThrow(/SOROBAN_RPC_URL_MAINNET/);

That is the production configuration, asserted as invalid. It passed, so it read as a test doing its job — but a test asserting a config is invalid is a claim about reality, and this one was wrong about it. Both tests now pin the opposite: a missing per-network RPC must not block startup.

The env-validation idea stands and the rest of the PR is untouched. Sorry for the disruption — that one was on the review, not the contribution.