From 8cb67122ad716835cf48e27886d24347733f5c74 Mon Sep 17 00:00:00 2001
From: screentinker <hello@screentinker.com>
Date: Fri, 14 Aug 2026 12:17:41 -0500
Subject: [PATCH] Fix a load-time crash that took down any install collecting
 statistics (#279)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

1.9.35 could not start with TELEMETRY_COLLECTOR=1. It threw before listening:

  ReferenceError: Cannot access 'db' before initialization
      at server.js:986

and systemd restarted it in a loop. Production was down until it was rolled back.

The mount passed the module-scope `db` to the collector's factory, but that binding is
declared ~275 lines further down. The inline handler this replaced only touched `db`
inside a request callback — which runs long after the binding exists — so moving the same
reference into a factory argument turned a lazy read into an eager one. Now resolved as
`require('./db/database').db`, the way every neighbouring call site in that region does it.

WHY NOTHING CAUGHT IT. The block is gated on a flag that only the statistics-collecting
deployment sets. It had therefore never executed in CI, on alpha, or in any test — 1676
tests, four green jobs, a clean alpha deploy, and the crashing line had still never run.
The unit tests mount the router directly and pass a db, which is precisely the part that
was fine.

So the boot smoke now boots WITH the collector enabled and asserts its routes answer:
/api/public/stats returns the expected shape, and a malformed report is refused with 400.
Booting alone would not be enough — the collector could mount and be broken.

Confirmed by reproduction: the released code fails to boot under that flag, and this does
not. 1676/1676 pass.
---
 .github/workflows/ci.yml | 21 +++++++++++++++++++++
 server/server.js         |  8 +++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index b701b1c..95581b6 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -109,6 +109,12 @@ jobs:
         working-directory: server
         env:
           SELF_HOSTED: 'true'
+          # Boot WITH the collector on. This block is config-gated and only the
+          # statistics-collecting deployment sets the flag, so it had never executed in CI,
+          # on alpha, or in any test - and a load-time crash inside it took production down
+          # while every check was green. Code only one deployment runs is exactly the code
+          # CI has to execute.
+          TELEMETRY_COLLECTOR: '1'
         run: |
           node server.js > "$RUNNER_TEMP/server.log" 2>&1 &
           echo $! > "$RUNNER_TEMP/server.pid"
@@ -131,6 +137,21 @@ jobs:
           test "$REPORTED" = "$EXPECTED"
           echo "OK: status ok, version $REPORTED matches VERSION"
 
+      # Booting is not enough on its own - the collector could be mounted and broken. Prove
+      # the routes it adds actually answer, so a fault inside that block fails here rather
+      # than on the single deployment that turns it on.
+      - name: Assert the collector routes answer when enabled
+        run: |
+          STATS="$(curl -sf http://localhost:3001/api/public/stats)"
+          echo "stats: $STATS"
+          test "$(echo "$STATS" | jq -r 'has("screens") and has("installs")')" = "true"
+          REPORT="$(curl -s -o /dev/null -w '%{http_code}' -X POST \
+            -H 'Content-Type: application/json' -d '{"bad":1}' \
+            http://localhost:3001/api/telemetry/report)"
+          echo "malformed report -> HTTP $REPORT"
+          test "$REPORT" = "400"
+          echo "OK: collector mounted and answering"
+
       - name: Stop server
         if: always()
         run: kill "$(cat "$RUNNER_TEMP/server.pid")" 2>/dev/null || true
diff --git a/server/server.js b/server/server.js
index 2bdb847..07249a0 100644
--- a/server/server.js
+++ b/server/server.js
@@ -983,7 +983,13 @@ app.use('/api/status', require('./routes/status'));
  * TELEMETRY_COLLECTOR=1, so a normal self-hosted install exposes neither.
  */
 if (process.env.TELEMETRY_COLLECTOR === '1') {
-  app.use('/api', require('./routes/telemetry-collector')(db));
+  /* `require('./db/database').db`, not the module-scope `db` — that binding is declared far
+     below this line, so naming it here throws "Cannot access 'db' before initialization" at
+     load and the process never starts. The inline handler this replaced only touched `db`
+     inside a request callback, which runs long after the binding exists; passing it to a
+     factory made the reference eager. Every neighbouring call site in this region resolves
+     the same lazy way. */
+  app.use('/api', require('./routes/telemetry-collector')(require('./db/database').db));
   console.log('[telemetry] collector enabled at POST /api/telemetry/report (+ GET /api/public/stats)');
 }