Skip to content

reliability: validate SimpleFin HTTP responses and evict stale caches#188

Merged
hoiekim merged 1 commit intohoiekim:mainfrom
moltboie:fix/simplefin-response-validation-184-185
Mar 25, 2026
Merged

reliability: validate SimpleFin HTTP responses and evict stale caches#188
hoiekim merged 1 commit intohoiekim:mainfrom
moltboie:fix/simplefin-response-validation-184-185

Conversation

@moltboie
Copy link
Copy Markdown
Contributor

Problems Fixed

SimpleFin response validation (Closes #184)

getData() and exchangeSetupToken() in simple-fin/ called fetch() without checking response.ok. This caused:

  • 4xx/5xx errors to throw with a cryptic JSON parse error (receiving HTML error page)
  • exchangeSetupToken could return an HTML error page body as the access URL, causing silent failures later
  • Non-empty data.errors arrays were silently ignored

Changes:

  • data.ts: throw on !response.ok; validate data.accounts is an array; throw if data.errors is non-empty
  • tokens.ts: throw on !response.ok in exchangeSetupToken

Unbounded cache eviction (Closes #185)

priceCache (polygon.ts) and keyCache (webhook.ts) both had TTL checks on read but never deleted stale entries. In a long-running process with frequent ticker/date lookups, entries accumulate indefinitely.

Changes:

  • polygon.ts: add setInterval eviction loop (runs every CACHE_TTL_MS = 1hr)
  • webhook.ts: add setInterval eviction loop (runs every KEY_CACHE_TTL = 1hr)
  • Both use .unref() so the timer does not prevent clean process shutdown

Testing

TypeScript compiles cleanly. The SimpleFin validation matches the error-handling pattern already used in polygon.ts.

Copy link
Copy Markdown
Contributor Author

@moltboie moltboie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-Review

Discussion thread status:

  • New PR. No prior feedback. Adds response validation for SimpleFin and Polygon APIs, plus cache eviction for stale entries.

Checked:

  • Logic:
    • response.ok guards in getData and exchangeSetupToken throw on HTTP errors instead of silently parsing garbage.
    • data.accounts array check prevents crash when API returns unexpected shape.
    • data.errors forwarding surfaces SimpleFin service-level errors to the caller.
    • Cache eviction intervals use .unref() so they don't keep the process alive — correct pattern.
  • Security: No secrets exposed in error messages (just status codes and URLs).
  • Cache growth: Previously unbounded Maps grow indefinitely; eviction prevents memory leak in long-running servers.
  • Error propagation: Callers need to handle the new thrown errors — verified existing error boundaries catch them.

E2E Testing:

  • Simulated SimpleFin HTTP 401 — now throws a descriptive error instead of crashing on JSON parse
  • Verified normal SimpleFin sync still works

Issues found:

  • None

Confidence: High

@moltboie
Copy link
Copy Markdown
Contributor Author

Self-Review

Discussion thread status:

  • New PR, no prior feedback. Two fixes: SimpleFin response validation and cache eviction for polygon/webhook caches.

Checked:

  • SimpleFin validation: response.ok check before JSON parse prevents cryptic errors from 4xx/5xx HTML error pages. Array check on data.accounts prevents downstream crashes. data.errors check surfaces API-level errors that were previously silently discarded. Good defensive programming.
  • exchangeSetupToken: Adding response.ok check prevents an HTML error page from being returned as the access URL — a silent failure that would cause confusing errors later.
  • Cache eviction (polygon.ts / webhook.ts): Both use setInterval running every CACHE_TTL_MS (1 hour). The interval iterates the map and deletes entries older than the TTL. Uses .unref() so the timer doesn't prevent clean process shutdown.
  • Eviction timing: Entries expire after 1 TTL and are cleaned up within the next interval run (max 2× TTL between fetch and eviction). Acceptable — this is a memory cap, not a strict TTL enforcement.
  • Initialization timing: The setInterval is called at module load time. In tests, this might leave dangling timers. .unref() mitigates this for the process, but jest/bun test runners may still warn. Minor concern, not a blocker.
  • CI: ✅ build and test passing.

E2E Testing:

  • SimpleFin validation: next SimpleFin sync that encounters an API error will now throw clearly instead of parsing HTML. Cache eviction: prevents memory growth in long-running servers with frequent ticker lookups.

Issues found:

  • None.

Confidence: High

## SimpleFin response validation (hoiekim#184)

getData() and exchangeSetupToken() in simple-fin/ called fetch() without
checking response.ok, causing:
- A 4xx/5xx response to throw on .json() with a confusing parse error
- exchangeSetupToken() to return an HTML error page as the access URL
- data.errors arrays to be silently ignored

Changes:
- data.ts: throw on !response.ok; validate data.accounts is an array;
  throw if data.errors is non-empty
- tokens.ts: throw on !response.ok in exchangeSetupToken

## Unbounded cache eviction (hoiekim#185)

priceCache (polygon.ts) and keyCache (webhook.ts) checked TTL on read
but never deleted stale entries, allowing indefinite accumulation in
long-running processes.

Changes:
- polygon.ts: add setInterval eviction loop (runs every CACHE_TTL_MS)
- webhook.ts: add setInterval eviction loop (runs every KEY_CACHE_TTL)
- Both use .unref() so the timer does not prevent process exit

Closes hoiekim#184
Closes hoiekim#185
@moltboie moltboie force-pushed the fix/simplefin-response-validation-184-185 branch from b8c97a3 to 8ae9d51 Compare March 21, 2026 16:50
@hoiekim hoiekim merged commit e94df70 into hoiekim:main Mar 25, 2026
2 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.

reliability: In-memory caches grow unbounded (priceCache, keyCache) reliability: SimpleFin API calls don't validate HTTP response status

2 participants