Code Review: dicechess-observer

I have reviewed the core files of the dicechess-observer microservice (config.ts, server.ts, service.ts, observer.ts, dicechess.ts, assemble.ts, ingest.ts).

Overall, the codebase is excellent. It is clean, strongly typed, and separates concerns well. Here are my specific observations:

🌟 The Good

  1. Clean Separation of Concerns: The logic is well partitioned.
    • dicechess.ts handles the quirk of Cloudflare blocking undici by cleanly wrapping curl via execFile.
    • observer.ts perfectly encapsulates the polling loop state and lifecycle.
    • normalize.ts / assemble.ts purely handles the domain logic of transforming raw states into the clean analytics schema.
  2. Graceful Shutdown: In service.ts, hooking into SIGINT and SIGTERM to call observer.stop() and waiting for the loop promise to resolve is a textbook example of how to implement graceful shutdown in Node.js.
  3. Security: Using execFile instead of exec in dicechess.ts prevents any shell injection attacks, even if jwt or gameId contained shell meta-characters. The headers and URL are passed safely as direct OS arguments.
  4. Idempotency & Resiliency: The observer.start() is cleanly idempotent, and errors inside the observer.loop() catch and log without crashing the whole process (incrementing stats.errors), ensuring the observer recovers on the next interval.

⚠️ Minor Issues / Suggestions

1. POLL_INTERVAL_MS empty string fallback

In src/config.ts:

pollIntervalMs: Number(process.env.POLL_INTERVAL_MS ?? 15_000),

If a user adds POLL_INTERVAL_MS= (empty string) to their .env, process.env.POLL_INTERVAL_MS will be "". The ?? operator only catches null or undefined, so it evaluates to Number(""), which is 0. This would cause sleep(0) and hammer the API continuously in a tight loop.

Fix Recommendation: Use the logical OR operator (||) instead of nullish coalescing (??), which treats "" (and 0) as falsy, ensuring it gracefully falls back to 15_000:

pollIntervalMs: Number(process.env.POLL_INTERVAL_MS || 15_000),

2. Typo in readJsonBody catch block

In src/server.ts, if the incoming HTTP request is aborted abruptly (req.on("error")), it resolves readJsonBody with {}. This is perfectly safe for this specific use-case, but generally, ignoring stream errors silently can sometimes hide network issues. Given this is just a local control panel, it is perfectly fine.

3. JWT validation

In src/config.ts, the DICECHESS_JWT is trimmed and the Bearer prefix is removed. This is very robust. You might also want to do a simple length check or format check on startup (in service.ts) just to warn the user early if the JWT looks visibly malformed, rather than waiting for curl to return a 403 or 401.

Conclusion

The architecture is solid and completely fit for purpose. The use of curl to bypass Cloudflare is an effective pragmatic choice. The minimal HTTP server (node:http) avoids dragging in heavy dependencies like Express for what is essentially a single HTML page and two API endpoints. Excellent work!