Skip to content

refactor: unify duplicated route env checks into a shared requireEnv helper - #1

Merged
petermuazu merged 1 commit into
mainfrom
refactor/dedupe-require-env
Aug 27, 2026
Merged

refactor: unify duplicated route env checks into a shared requireEnv helper#1
petermuazu merged 1 commit into
mainfrom
refactor/dedupe-require-env

Conversation

@petermuazu

Copy link
Copy Markdown
Owner

Summary

The "503 on missing env config" logic was copy-pasted across seven route files — threshold, invoice-settings, treasury, release-escrow, compliance, invoices, and disputes — each with its own local requireEnv helper or inline check. Four of them defined a near-identical local requireEnv(res); the others inlined the same check. Because a fix to one copy isn't propagated to the others, behavior silently diverges.

This matters here because the backend is the primary REST/webhook surface for both frontends and the documented OpenAPI surface (docs/api-reference.md) that third-party merchants integrate against. The 503 misconfiguration response is effectively part of the public contract.

Changes

  • New src/lib/env.ts — one requireEnv(res, vars) helper:
    • Always validates SOROBAN_RPC_URL (returned as rpcUrl) and the network passphrase (returned as networkPassphrase).
    • Validates each mapped env var; on any missing value writes the identical 503 with Service misconfiguration: missing required environment variables and returns null.
    • Returns a fully typed { rpcUrl, networkPassphrase, ...vars } object via a generic mapped return type (e.g. TREASURY_CONTRACT_IDenv.treasuryContractId), so call sites stay type-safe and the object remains assignable to the env shapes the exported functions (e.g. setThreshold, releaseEscrow) expect.
  • Migrated all seven routes to the shared helper and deleted the local copies / inline blocks. Behavior is unchanged: same status, same message, same passphrase default.
  • Two pre-existing blockers required to run the test suite (fallout from the same feat: resolve issues #209 #210 #211 #212 WHEELBACK/COMEBACKHERE#344 merge that caused this duplication):
    • src/db/mongo.ts — removed a duplicated invoices index-creation block and a duplicate getInvoicesCollection export, which broke tsc and vitest's transform for every test importing mongo. The two index blocks were complementary, so all five indexes are preserved (merged into one block).
    • src/routes/invoices.ts — added the missing validateBody/validateParams and invoice schema imports the file already referenced but never imported (a ReferenceError at module load that broke every app-level test suite).

Verification

  • npx tsc --noEmit: no new errors from changed files (the repo has many pre-existing type errors on main, e.g. a conflicting duplicate InvoiceStatus interface in mongo.ts, lib/soroban.ts typings).
  • npm test (vitest): all env/503 route tests across the changed routes pass — e.g. compliance, invoice-settings, release-escrow, treasury env-check suites (70/71 in the directly affected set; the single failure is a pre-existing stellar-sdk 12.3 address-format issue in untouched business logic).
  • The suite was not runnable on main before this change (mongo.ts transform error + invoices.ts ReferenceError).

Remaining pre-existing test failures (not caused by this PR)

The full suite still shows ~24 failures on this branch, all pre-existing and unrelated:

  • env-validation.test.ts imports a validateEnv export that no longer exists in src/index.ts
  • indexer-redis-reconnect.test.ts — stellar-sdk 12.3 allowHttp + fake-timer issues
  • invoices.test.ts pagination — untouched GET /invoices list route vs. test contract mismatch
  • mongo-hardening.test.ts — mongodb driver constructor drift (no lockfile pins versions)
  • treasury.test.ts/treasury-balances-cache.test.ts — stellar-sdk 12.3 strict address validation + a fake-timer clock-reset bug in the TTL test

Notes for reviewers

  • The repo has no lockfile, so fresh installs pull newer minors (stellar-sdk 12.3) that break several tests; pinning versions is a separate follow-up.
  • src/db/mongo.ts still contains a conflicting duplicate InvoiceStatus interface from the same feat: resolve issues #209 #210 #211 #212 WHEELBACK/COMEBACKHERE#344 merge — deliberately left untouched since resolving it requires deciding the canonical shape.

…helper

The 503-on-missing-env logic was copy-pasted across seven routes
(threshold, invoice-settings, treasury, release-escrow, compliance,
invoices, disputes), each with its own local requireEnv or inline check.
A fix in one copy silently diverges from the rest on a surface that
third-party merchants integrate against directly.

Extract a single requireEnv(res, vars) into src/lib/env.ts that always
validates SOROBAN_RPC_URL and the network passphrase, returns 503 + null
when any required var is missing, and returns a fully typed
{ rpcUrl, networkPassphrase, ...vars } object. Migrate all seven routes
to it and delete the local copies. Behavior is unchanged: same 503
status, same error message, same passphrase default.

Also includes two pre-existing blockers the test suite needs (fallout
from the WHEELBACK#344 merge that caused this duplication):
- src/db/mongo.ts: remove the duplicated `invoices` index block and the
  duplicate getInvoicesCollection export (broke tsc and vitest transform)
- src/routes/invoices.ts: import validateBody/validateParams and the
  invoice schemas it already referenced (ReferenceError at module load)

Generated with Codebuff 🤖
Co-Authored-By: Codebuff <noreply@codebuff.com>
@petermuazu
petermuazu merged commit 104e17e into main Aug 27, 2026
0 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant