@@ -585,6 +595,24 @@ export async function render(container) { Everything that would be sent, in full:
${esc(JSON.stringify(info.payload, null, 2))}
+
+ ${on ? 'Sent once a day to' : 'When enabled, sent once a day to'}
+ ${esc(info.endpoint || '')}. If this server's outbound
+ traffic is filtered, that address has to be allowed or the reports never arrive.
+
+ A second copy also goes to your own collector at
+ ${esc(info.extra_endpoint)}, configured on this server
+ with TELEMETRY_EXTRA_ENDPOINT. That is in addition to
+ the above, not instead of it — turn the switch off if you want your own statistics without
+ sharing.
+
+ The last attempt (${esc(new Date(info.last_error.at * 1000).toLocaleString())}) did not get + through — ${why}. Check that outbound HTTPS to that address is permitted. +
` : ''}No names, addresses, content, or user details. The ID is random and identifies the install only so repeat reports aren't counted twice. ${esc(sent)} @@ -594,8 +622,12 @@ export async function render(container) { document.getElementById('telemetryToggle')?.addEventListener('change', async (e) => { const enabled = e.target.checked; try { - await api.adminSetTelemetry(enabled); - showToast(enabled ? 'Sharing install statistics — thank you' : 'Install statistics off', 'success'); + // Turning it on sends immediately, so a blocked firewall is reported here and now rather + // than failing quietly tonight — say so plainly instead of a cheerful success toast. + const r = await api.adminSetTelemetry(enabled); + if (!enabled) showToast('Install statistics off', 'success'); + else if (r.first_report && r.first_report.sent) showToast('Shared — thank you', 'success'); + else showToast('Saved, but the first report did not get through — see below', 'error'); loadTelemetry(); } catch { e.target.checked = !enabled; diff --git a/server/lib/telemetry.js b/server/lib/telemetry.js index 40c7201..b26170a 100644 --- a/server/lib/telemetry.js +++ b/server/lib/telemetry.js @@ -35,9 +35,24 @@ const appSettings = require('./app-settings'); const KEY_ID = 'telemetry_instance_id'; const KEY_ENABLED = 'telemetry_enabled'; // unset = never asked -const KEY_LAST = 'telemetry_last_report'; +const KEY_LAST = 'telemetry_last_report'; // last SUCCESSFUL send +const KEY_LAST_ERROR = 'telemetry_last_error';// last FAILED attempt — see getLastError -const DEFAULT_ENDPOINT = 'https://stats.screentinker.com/api/telemetry/report'; +/* + * Where reports go. TWO independent destinations, deliberately: + * + * SCREENTINKER_ENDPOINT hard-wired, and reached only when the operator has switched sharing on. + * Not overridable — an "override" that silently redirected the shared + * report would make the opt-in mean something different from what it says. + * + * TELEMETRY_EXTRA_ENDPOINT an operator's OWN collector, for their own fleet numbers. Additional, + * never a replacement, and named so it cannot be mistaken for one. It is + * sent independently of the sharing toggle: it is their server posting to + * their host, so our opt-in has no business gating it. An operator who + * wants internal statistics and nothing leaving for us sets this and + * leaves sharing off — that combination is supported on purpose. + */ +const SCREENTINKER_ENDPOINT = 'https://stats.screentinker.com/api/telemetry/report'; const REPORT_INTERVAL_MS = 24 * 60 * 60 * 1000; // daily; this is a count, not a metric const FIRST_REPORT_DELAY_MS = 5 * 60 * 1000; // let boot settle before any outbound call @@ -88,6 +103,24 @@ function countScreens(db) { } } +/* The address an operator may need to allowlist for the shared report. Hard-wired. */ +function endpoint() { return SCREENTINKER_ENDPOINT; } + +/* The operator's own collector, if they configured one. Null when they have not. */ +function extraEndpoint() { return process.env.TELEMETRY_EXTRA_ENDPOINT || null; } + +/* + * Everywhere this report is going, right now, and why — so the UI can list every destination + * rather than implying there is only one. Sharing gates OUR endpoint alone. + */ +function destinations() { + const out = []; + if (state() === 'on') out.push({ url: endpoint(), kind: 'screentinker' }); + const extra = extraEndpoint(); + if (extra) out.push({ url: extra, kind: 'extra' }); + return out; +} + /* What was last sent, and when. Surfaced in Settings so an operator can check rather than trust. */ function getLastReport() { const raw = appSettings.get(KEY_LAST, null); @@ -95,30 +128,65 @@ function getLastReport() { try { return JSON.parse(raw); } catch (_) { return null; } } +/* + * The last FAILED attempt, kept separately from the last success. + * + * A self-hosted server frequently sits behind egress filtering, so "enabled but nothing arrives" + * is the normal failure and it is otherwise completely silent — the operator sees "nothing has + * been sent" and has no way to tell a blocked firewall from a broken feature. Recording the + * failure lets the UI name the host that needs unblocking instead. + */ +function getLastError() { + const raw = appSettings.get(KEY_LAST_ERROR, null); + if (!raw) return null; // '' is how a success clears it + try { return JSON.parse(raw); } catch (_) { return null; } +} + /* * Send one report. Returns {sent:false, reason} rather than throwing — a stats call must never be * able to affect the running server, so every failure path here is quiet and local. */ -async function report(db, { endpoint = process.env.TELEMETRY_ENDPOINT || DEFAULT_ENDPOINT } = {}) { - if (state() !== 'on') return { sent: false, reason: 'not_enabled' }; - - const body = payload(db); +async function postTo(url, body) { try { - const res = await fetch(endpoint, { + const res = await fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), signal: AbortSignal.timeout(10000), }); - if (!res.ok) return { sent: false, reason: `http_${res.status}`, body }; - appSettings.set(KEY_LAST, JSON.stringify({ at: Math.floor(Date.now() / 1000), body })); - return { sent: true, body }; + return res.ok ? { sent: true } : { sent: false, reason: `http_${res.status}` }; } catch (err) { - // Offline, DNS failure, blocked egress — all normal for a self-hosted box, none of them news. - return { sent: false, reason: err && err.name === 'TimeoutError' ? 'timeout' : 'network', body }; + // Offline, DNS failure, blocked egress — all normal for a self-hosted box, none of them news + // in the log, but all worth surfacing in the UI so the operator can act on it. + return { sent: false, reason: err && err.name === 'TimeoutError' ? 'timeout' : 'network' }; } } +async function report(db, { urls = null } = {}) { + // `urls` is a test seam. Normal callers get destinations() — sharing gates ours, an operator's + // own collector is independent of it. + const targets = urls || destinations(); + if (!targets.length) return { sent: false, reason: 'not_enabled', results: [] }; + + const now = () => Math.floor(Date.now() / 1000); + const body = payload(db); + + // Every destination is attempted, independently. One unreachable collector must not stop the + // other from receiving — a blocked corporate firewall on their host should not cost us the + // shared count, and our endpoint being down should not cost them their own fleet numbers. + const results = []; + for (const t of targets) results.push({ ...t, ...(await postTo(t.url, body)) }); + + const failed = results.filter(r => !r.sent); + if (results.some(r => r.sent)) appSettings.set(KEY_LAST, JSON.stringify({ at: now(), body, results })); + // Keep only a LIVE complaint: record what is still failing, and clear it once nothing is. + appSettings.set(KEY_LAST_ERROR, failed.length + ? JSON.stringify({ at: now(), reason: failed[0].reason, url: failed[0].url, failed }) + : ''); + + return { sent: failed.length === 0, results, body, reason: failed[0]?.reason }; +} + function start(db) { if (timer) return; const tick = () => { report(db).catch(() => {}); }; @@ -129,4 +197,4 @@ function start(db) { function stop() { if (timer) { clearInterval(timer); timer = null; } } -module.exports = { instanceId, state, setEnabled, payload, report, getLastReport, start, stop }; +module.exports = { instanceId, state, setEnabled, payload, report, endpoint, extraEndpoint, destinations, getLastReport, getLastError, start, stop }; diff --git a/server/routes/admin.js b/server/routes/admin.js index 1d45c62..bfaabb9 100644 --- a/server/routes/admin.js +++ b/server/routes/admin.js @@ -436,17 +436,38 @@ router.get('/telemetry', requirePlatformAdmin, (req, res) => { res.json({ state: telemetry.state(), // 'unasked' | 'on' | 'off' payload: telemetry.payload(db), // what WOULD be sent, right now + endpoint: telemetry.endpoint(), // ours — the host an operator may need to allowlist + extra_endpoint: telemetry.extraEndpoint(),// their own collector, if configured + destinations: telemetry.destinations(), // everywhere it actually goes, right now last_report: telemetry.getLastReport(), // what was actually sent, and when + last_error: telemetry.getLastError(), // why the last attempt failed, if it did }); }); -router.put('/telemetry', requirePlatformAdmin, (req, res) => { +router.put('/telemetry', requirePlatformAdmin, async (req, res) => { // Both answers are recorded. Declining must persist as 'off' rather than staying 'unasked', // or the prompt returns after every update — which is how telemetry earns its bad name. const enabled = !!req.body.enabled; const state = telemetry.setEnabled(enabled); logActivity(req.user.id, 'admin_set_telemetry', `enabled: ${enabled}`, null, getClientIp(req), null); - res.json({ state, payload: telemetry.payload(db) }); + + // Send once, now, rather than waiting for the next daily tick. Two reasons: the operator is + // standing right here and "nothing has been sent" for the next 24h reads as broken, and an + // egress-filtered network fails HERE where we can name the host to unblock — instead of + // failing silently tonight where nobody is watching. + let first = null; + if (enabled) first = await telemetry.report(db); + + res.json({ + state, + payload: telemetry.payload(db), + endpoint: telemetry.endpoint(), + extra_endpoint: telemetry.extraEndpoint(), + destinations: telemetry.destinations(), + first_report: first && { sent: first.sent, reason: first.reason || null }, + last_report: telemetry.getLastReport(), + last_error: telemetry.getLastError(), + }); }); // ===================== Version update indicator ===================== diff --git a/server/test/telemetry.test.js b/server/test/telemetry.test.js index 51de6ec..934fe6a 100644 --- a/server/test/telemetry.test.js +++ b/server/test/telemetry.test.js @@ -32,7 +32,7 @@ test('an install that has not been asked reports nothing', async () => { const spy = mock.method(globalThis, 'fetch', async () => { throw new Error('must not be called'); }); try { const r = await telemetry.report(db); - assert.deepEqual(r, { sent: false, reason: 'not_enabled' }); + assert.equal(r.sent, false); assert.equal(r.reason, 'not_enabled'); assert.equal(spy.mock.callCount(), 0, 'no outbound request may be made before consent'); } finally { spy.mock.restore(); } }); @@ -50,7 +50,7 @@ test('a declined install still reports nothing', async () => { telemetry.setEnabled(false); const spy = mock.method(globalThis, 'fetch', async () => { throw new Error('must not be called'); }); try { - assert.deepEqual(await telemetry.report(db), { sent: false, reason: 'not_enabled' }); + const d = await telemetry.report(db); assert.equal(d.sent, false); assert.equal(d.reason, 'not_enabled'); assert.equal(spy.mock.callCount(), 0); } finally { spy.mock.restore(); } }); @@ -96,7 +96,7 @@ test('when enabled it sends exactly the payload, and records what it sent', asyn return { ok: true, status: 200 }; }); try { - const r = await telemetry.report(db, { endpoint: 'https://example.test/report' }); + const r = await telemetry.report(db, { urls: [{ url: 'https://example.test/report', kind: 'screentinker' }] }); assert.equal(r.sent, true); assert.equal(seen.method, 'POST'); assert.equal(seen.url, 'https://example.test/report'); @@ -110,12 +110,112 @@ test('when enabled it sends exactly the payload, and records what it sent', asyn } finally { spy.mock.restore(); } }); +test('a blocked outbound connection is recorded, with the address that was blocked', async () => { + // Egress filtering is the normal failure on a self-hosted box and is otherwise invisible: the + // operator sees nothing arriving and cannot tell a firewall from a broken feature. The UI can + // only name the host to allowlist if the failure is recorded here. + reset(); + telemetry.setEnabled(true); + const spy = mock.method(globalThis, 'fetch', async () => { throw new Error('ECONNREFUSED'); }); + try { + await telemetry.report(db, { urls: [{ url: 'https://stats.example.test/report', kind: 'screentinker' }] }); + const err = telemetry.getLastError(); + assert.ok(err, 'a failed attempt must be recorded, or the operator has nothing to act on'); + assert.equal(err.reason, 'network'); + assert.equal(err.url, 'https://stats.example.test/report', 'must record the address actually tried'); + assert.equal(typeof err.at, 'number'); + } finally { spy.mock.restore(); } +}); + +test('a later success clears the stale failure', async () => { + reset(); + telemetry.setEnabled(true); + const bad = mock.method(globalThis, 'fetch', async () => { throw new Error('ECONNREFUSED'); }); + try { await telemetry.report(db, { urls: [{ url: 'https://example.test/report', kind: 'screentinker' }] }); } finally { bad.mock.restore(); } + assert.ok(telemetry.getLastError(), 'precondition: a failure was recorded'); + + const good = mock.method(globalThis, 'fetch', async () => ({ ok: true, status: 200 })); + try { + await telemetry.report(db, { urls: [{ url: 'https://example.test/report', kind: 'screentinker' }] }); + assert.equal(telemetry.getLastError(), null, + 'a stale firewall warning must not outlive the problem it describes'); + } finally { good.mock.restore(); } +}); + +test('an operator collector is ADDITIONAL — it never replaces the shared report', async () => { + // The whole point of naming it EXTRA rather than ENDPOINT: configuring your own collector must + // not silently redirect the report the operator agreed to share. If this ever becomes a + // redirect, the opt-in stops meaning what the UI says it means. + reset(); + telemetry.setEnabled(true); + const original = process.env.TELEMETRY_EXTRA_ENDPOINT; + process.env.TELEMETRY_EXTRA_ENDPOINT = 'https://mine.example.test/collect'; + try { + const dests = telemetry.destinations(); + assert.equal(dests.length, 2, 'sharing on + own collector = both, never one'); + assert.deepEqual(dests.map(d => d.kind).sort(), ['extra', 'screentinker']); + + const hits = []; + const spy = mock.method(globalThis, 'fetch', async (url) => { hits.push(url); return { ok: true, status: 200 }; }); + try { + await telemetry.report(db); + assert.equal(hits.length, 2, 'both destinations must receive the report'); + assert.ok(hits.includes('https://mine.example.test/collect')); + assert.ok(hits.some(u => u.includes('screentinker.com')), 'the shared report must still be sent'); + } finally { spy.mock.restore(); } + } finally { + if (original === undefined) delete process.env.TELEMETRY_EXTRA_ENDPOINT; + else process.env.TELEMETRY_EXTRA_ENDPOINT = original; + } +}); + +test('an operator can keep their own statistics while sharing nothing with us', async () => { + // Someone who wants internal fleet numbers but nothing leaving for us sets their own collector + // and leaves sharing off. Supported on purpose: it is their server posting to their host. + reset(); + telemetry.setEnabled(false); + const original = process.env.TELEMETRY_EXTRA_ENDPOINT; + process.env.TELEMETRY_EXTRA_ENDPOINT = 'https://mine.example.test/collect'; + try { + const hits = []; + const spy = mock.method(globalThis, 'fetch', async (url) => { hits.push(url); return { ok: true, status: 200 }; }); + try { + await telemetry.report(db); + assert.deepEqual(hits, ['https://mine.example.test/collect']); + assert.ok(!hits.some(u => u.includes('screentinker.com')), + 'sharing is off — nothing may reach us, whatever else is configured'); + } finally { spy.mock.restore(); } + } finally { + if (original === undefined) delete process.env.TELEMETRY_EXTRA_ENDPOINT; + else process.env.TELEMETRY_EXTRA_ENDPOINT = original; + } +}); + +test('one unreachable destination does not stop the other', async () => { + reset(); + telemetry.setEnabled(true); + const spy = mock.method(globalThis, 'fetch', async (url) => { + if (url.includes('broken')) throw new Error('ECONNREFUSED'); + return { ok: true, status: 200 }; + }); + try { + const r = await telemetry.report(db, { urls: [ + { url: 'https://broken.example.test/a', kind: 'extra' }, + { url: 'https://working.example.test/b', kind: 'screentinker' }, + ] }); + assert.equal(r.results.filter(x => x.sent).length, 1, 'the reachable one still receives it'); + assert.equal(r.results.filter(x => !x.sent).length, 1); + assert.equal(telemetry.getLastError().url, 'https://broken.example.test/a', + 'the failure names the destination that actually failed'); + } finally { spy.mock.restore(); } +}); + test('a failed send is quiet and local — never throws, never records a phantom report', async () => { reset(); telemetry.setEnabled(true); const spy = mock.method(globalThis, 'fetch', async () => { throw new Error('ECONNREFUSED'); }); try { - const r = await telemetry.report(db, { endpoint: 'https://example.test/report' }); + const r = await telemetry.report(db, { urls: [{ url: 'https://example.test/report', kind: 'screentinker' }] }); assert.equal(r.sent, false); assert.equal(r.reason, 'network'); assert.equal(telemetry.getLastReport(), null, 'a failed send must not look like a successful one'); @@ -124,7 +224,7 @@ test('a failed send is quiet and local — never throws, never records a phantom // An HTTP error is likewise not a success. const spy2 = mock.method(globalThis, 'fetch', async () => ({ ok: false, status: 503 })); try { - const r = await telemetry.report(db, { endpoint: 'https://example.test/report' }); + const r = await telemetry.report(db, { urls: [{ url: 'https://example.test/report', kind: 'screentinker' }] }); assert.equal(r.sent, false); assert.equal(r.reason, 'http_503'); assert.equal(telemetry.getLastReport(), null);