mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-14 14:23:14 -06:00
Fix a load-time crash that took down any install collecting statistics (#279)
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.
This commit is contained in:
parent
b13f11af13
commit
8cb67122ad
21
.github/workflows/ci.yml
vendored
21
.github/workflows/ci.yml
vendored
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)');
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue