diff --git a/public/observer-detail.js b/public/observer-detail.js index 6d2d9c7f..5c6e9435 100644 --- a/public/observer-detail.js +++ b/public/observer-detail.js @@ -181,20 +181,42 @@ window.ObserverDetailNaiveBanner = { const days = currentDays; try { destroyCharts(); - chartDefaults(); - const [obs, analytics, obsSkewArr] = await Promise.all([ - api('/observers/' + encodeURIComponent(observerId)), - api('/observers/' + encodeURIComponent(observerId) + '/analytics?days=' + days), - api('/observers/clock-skew', { ttl: 30000 }).catch(function() { return []; }), - ]); + // Start all requests together, but only identity gates the first render. + // Capture analytics errors immediately, even if identity is still pending. + const observerRequest = api('/observers/' + encodeURIComponent(observerId)); + const analyticsRequest = api('/observers/' + encodeURIComponent(observerId) + '/analytics?days=' + days) + .then(data => ({ data }), error => ({ error })); + const skewRequest = api('/observers/clock-skew', { ttl: 30000 }).catch(function() { return []; }); + const obs = await observerRequest; // Only the latest load of the current visit owns the detail view. if (generation !== detailGeneration) return; - // Find this observer's calibration data. - var obsSkew = null; - (Array.isArray(obsSkewArr) ? obsSkewArr : []).forEach(function(s) { - if (s && s.observerID === observerId) obsSkew = s; + renderDetail(obs); + analyticsRequest.then(result => { + if (generation !== detailGeneration) return; + const el = document.getElementById('obsAnalytics'); + if (!el) return; + try { + if (result.error) throw result.error; + renderAnalytics(result.data); + } catch (error) { + destroyCharts(); + el.innerHTML = '
'; + el.firstChild.textContent = 'Error: ' + error.message; + } + el.setAttribute('aria-busy', 'false'); + }); + skewRequest.then(obsSkewArr => { + if (generation !== detailGeneration) return; + // Preserve the last matching calibration from the bulk response. + var obsSkew = null; + (Array.isArray(obsSkewArr) ? obsSkewArr : []).forEach(function(s) { + if (s && s.observerID === observerId) obsSkew = s; + }); + renderClockOffset(obsSkew); + }).catch(() => { + if (generation !== detailGeneration) return; + renderClockOffset(null); }); - renderDetail(obs, analytics, obsSkew); } catch (e) { if (generation !== detailGeneration) return; // SECURITY (OBS-2, PR #1539): use textContent for error messages. @@ -209,7 +231,7 @@ window.ObserverDetailNaiveBanner = { } } - function renderDetail(obs, analytics, obsSkew) { + function renderDetail(obs) { const el = document.getElementById('obsDetailContent'); if (!el) return; @@ -317,7 +339,15 @@ window.ObserverDetailNaiveBanner = {
ID: ${escapeHtml(obs.id)}
- ${obsSkew && obsSkew.samples > 0 ? ` +
Loading clock calibration…
+ ${window.ObserverDetailRadioHistory ? window.ObserverDetailRadioHistory.render(obs.radio_history) : ''} +
Loading analytics…
`; + } + + function renderClockOffset(obsSkew) { + const el = document.getElementById('obsClockOffset'); + if (!el) return; + el.innerHTML = obsSkew && obsSkew.samples > 0 ? `

Clock Offset

@@ -328,8 +358,15 @@ window.ObserverDetailNaiveBanner = {
How this is computed: when this observer and another observer see the same packet, we compare their receive timestamps. The median deviation across all multi-observer packets is this observer's offset.
-
` : ''} - ${window.ObserverDetailRadioHistory ? window.ObserverDetailRadioHistory.render(obs.radio_history) : ''} +
` : ''; + el.setAttribute('aria-busy', 'false'); + } + + function renderAnalytics(analytics) { + const el = document.getElementById('obsAnalytics'); + if (!el) return; + chartDefaults(); + el.innerHTML = `

Packets Over Time

diff --git a/tests/e2e/test-observer-detail-ownership-e2e.js b/tests/e2e/test-observer-detail-ownership-e2e.js index 5f939c66..c76113e1 100644 --- a/tests/e2e/test-observer-detail-ownership-e2e.js +++ b/tests/e2e/test-observer-detail-ownership-e2e.js @@ -10,9 +10,9 @@ const IDS = { A: 'AA'.repeat(32), B: 'BB'.repeat(32) }; const ARTIFACTS = process.env.ARTIFACT_DIR; if (ARTIFACTS) fs.mkdirSync(ARTIFACTS, { recursive: true }); const cases = []; -function test(name, run) { cases.push({ name, run }); } +function test(name, run, options = {}) { cases.push({ name, run, options }); } -async function setup(browser, viewport, theme) { +async function setup(browser, viewport, theme, options) { const context = await browser.newContext({ viewport, colorScheme: theme }); const page = await context.newPage(); page.setDefaultTimeout(10000); @@ -22,9 +22,11 @@ async function setup(browser, viewport, theme) { const ready = new Map(); const visits = { A: 0, B: 0 }; const network = []; + let holdClock = false; function observer(letter, visit = 0) { return { id: IDS[letter], name: 'Observer ' + letter + (visit ? ' visit ' + visit : ''), - last_seen: letter === 'B' ? new Date().toISOString() : null, packet_count: 5 }; + last_seen: letter === 'B' ? new Date().toISOString() : null, packet_count: 5, + model: 'Test radio', radio: '910.0,250,10,5' }; } await page.route('**/api/observers**', async route => { const url = new URL(route.request().url()); @@ -34,6 +36,11 @@ async function setup(browser, viewport, theme) { return route.fulfill({ json: { observers: ['A', 'B'].map(letter => observer(letter)), server_time: new Date().toISOString() } }); } if (url.pathname === '/api/observers/clock-skew') { + if (holdClock) { + pending.set('clock', route); + if (ready.has('clock')) ready.get('clock')(); + return; + } return route.fulfill({ json: [ { observerID: IDS.A, offsetSec: 1, samples: 11 }, { observerID: IDS.B, offsetSec: 2, samples: 22 }, @@ -42,6 +49,7 @@ async function setup(browser, viewport, theme) { } for (const letter of ['A', 'B']) { if (url.pathname === '/api/observers/' + IDS[letter]) { + if (options.identityFailure) return route.fulfill({ status: 404, json: { error: 'fixture identity failure' } }); return route.fulfill({ json: observer(letter, ++visits[letter]) }); } if (url.pathname === '/api/observers/' + IDS[letter] + '/analytics') { @@ -55,6 +63,10 @@ async function setup(browser, viewport, theme) { }); await page.goto(BASE + '/?theme=' + theme + '#/observers', { waitUntil: 'domcontentloaded' }); await page.waitForSelector('#obsTable tbody tr[data-value]'); + // The list also requests calibration. Hold only the detail request, after + // clearing that completed list cache entry (preserve real in-flight dedup). + holdClock = !!options.holdClock; + if (holdClock) await page.evaluate(() => invalidateApiCache('/observers/clock-skew')); // Observe the real api() promises (including its in-flight deduplication), // so assertions wait for loadDetail's continuations, never a guessed sleep. await page.evaluate(() => { @@ -67,43 +79,63 @@ async function setup(browser, viewport, theme) { }; }); async function open(letter) { - if (await page.locator('#obsTitle').count()) { - await page.locator('a[title="Back to Observers"]').click(); - } - await page.locator('#obsTable tbody tr[data-value="#/observers/' + IDS[letter] + '"]').click(); - if (viewport.width < 768) { - await page.locator('.slide-over-panel a').filter({ hasText: 'Open full detail' }).click(); - await page.keyboard.press('Escape'); - await page.locator('.slide-over-panel[data-state="open"]').waitFor({ state: 'hidden' }); + if (holdClock && await page.locator('#obsTitle').count()) { + // Navigate through the real router without making the list wait for + // this deliberately held, shared calibration request. + await page.evaluate(id => { location.hash = '#/observers/' + id; }, IDS[letter]); + } else { + if (await page.locator('#obsTitle').count()) { + await page.locator('a[title="Back to Observers"]').click(); + } + await page.locator('#obsTable tbody tr[data-value="#/observers/' + IDS[letter] + '"]').click(); + if (viewport.width < 768) { + await page.locator('.slide-over-panel a').filter({ hasText: 'Open full detail' }).click(); + await page.keyboard.press('Escape'); + await page.locator('.slide-over-panel[data-state="open"]').waitFor({ state: 'hidden' }); + } } await page.waitForSelector('#obsDaysSelect'); await page.waitForFunction(id => window.__observerRequests.some(r => r.path.startsWith('/observers/' + id + '/analytics')), IDS[letter]); await page.evaluate(id => Promise.allSettled(window.__observerRequests.filter(r => r.path === '/observers/' + id).map(r => r.promise)), IDS[letter]); } - async function release(letter, days, status = 200) { - const key = letter + ':' + days; + async function held(key) { // Wait until this exact route is intercepted, without a timing assumption. if (!pending.has(key)) { let timeout; try { await new Promise((resolve, reject) => { ready.set(key, resolve); - timeout = setTimeout(() => reject(new Error('Missing held analytics request: ' + key)), 10000); + timeout = setTimeout(() => reject(new Error('Missing held request: ' + key)), 10000); }); } finally { clearTimeout(timeout); ready.delete(key); } } const route = pending.get(key); - assert.ok(route, 'analytics request is held: ' + key); + assert.ok(route, 'request is held: ' + key); pending.delete(key); + return route; + } + async function settled(path) { + await page.evaluate(async path => { + await Promise.allSettled(window.__observerRequests.filter(r => r.path === path).map(r => r.promise)); + await new Promise(requestAnimationFrame); + }, path); + } + async function releaseClock(status = 200) { + const route = await held('clock'); + await route.fulfill({ status, json: status === 200 ? [ + { observerID: IDS.A, offsetSec: 3, samples: 33 }, + { observerID: IDS.B, offsetSec: 2, samples: 22 }, + ] : { error: 'fixture calibration failure' } }); + await settled('/observers/clock-skew'); + } + async function release(letter, days, status = 200) { + const route = await held(letter + ':' + days); const label = letter + ' ' + days + ' days'; await route.fulfill({ status, json: status === 200 ? { timeline: [{ label, count: days }], packetTypes: { 4: days }, nodesTimeline: [{ label, count: 1 }], snrDistribution: [{ range: '0 to 5', count: days }], recentPackets: [], } : { error: 'fixture failure' } }); - await page.evaluate(async suffix => { - await Promise.allSettled(window.__observerRequests.filter(r => r.path.endsWith(suffix) || !r.path.includes('/analytics?')).map(r => r.promise)); - await new Promise(requestAnimationFrame); - }, '/observers/' + IDS[letter] + '/analytics?days=' + days); + await settled('/observers/' + IDS[letter] + '/analytics?days=' + days); } async function snapshot() { return page.evaluate(() => ({ @@ -122,9 +154,82 @@ async function setup(browser, viewport, theme) { assert.match(state.content, letter === 'B' ? /22 samples/ : /33 samples/); assert.strictEqual(await page.locator('#obsDaysSelect').inputValue(), String(days)); } - return { page, context, errors, open, release, snapshot, current, network }; + return { page, context, errors, open, release, releaseClock, snapshot, current, network }; } +for (const first of ['analytics', 'clock']) test('progressive-' + first + '-first', async h => { + await h.open('B'); + assert.strictEqual(await h.page.locator('#obsTitle').textContent(), 'Observer B visit 1', + 'identity must be visible with both secondary responses held'); + const info = h.page.locator('.obs-info-grid'); + assert.match(await info.innerText(), /Online/); + assert.match(await info.innerText(), /Test radio/); + assert.match(await info.innerText(), /910.0 MHz/); + assert.strictEqual((await h.snapshot()).charts.length, 0); + assert.strictEqual(await h.page.locator('#obsAnalytics').getAttribute('aria-busy'), 'true'); + await h.page.evaluate(() => { window.__observerMetadata = document.querySelector('.obs-info-grid'); }); + if (ARTIFACTS) await h.page.screenshot({ path: path.join(ARTIFACTS, h.label + '-pending.png'), fullPage: true }); + if (first === 'analytics') { + await h.release('B', 7); + assert.strictEqual((await h.snapshot()).charts.length, 4, 'analytics does not wait for calibration'); + await h.releaseClock(); + } else { + await h.releaseClock(); + assert.match((await h.snapshot()).content, /22 samples/, 'calibration does not wait for analytics'); + assert.strictEqual((await h.snapshot()).charts.length, 0); + await h.release('B', 7); + } + await h.current('B', 7); + assert.strictEqual(await h.page.locator('#obsAnalytics').getAttribute('aria-busy'), 'false'); + assert.strictEqual(await h.page.evaluate(() => window.__observerMetadata === document.querySelector('.obs-info-grid')), true, + 'secondary replies preserve the actual metadata DOM node'); + assert.strictEqual(h.network.filter(url => url.includes('/analytics?')).length, 1); + assert.strictEqual(h.network.filter(url => url === '/api/observers/clock-skew').length, 2, + 'one list calibration request plus one detail request'); + if (ARTIFACTS) await h.page.screenshot({ path: path.join(ARTIFACTS, h.label + '-complete.png'), fullPage: true }); +}, { holdClock: true }); + +test('analytics-error-keeps-identity', async h => { + await h.open('B'); + await h.release('B', 7, 500); + assert.strictEqual(await h.page.locator('#obsTitle').textContent(), 'Observer B visit 1'); + assert.match(await h.page.locator('.obs-info-grid').innerText(), /Online/); + assert.match(await h.page.locator('#obsAnalytics').innerText(), /Error: API 500:/); + await h.releaseClock(); + assert.match((await h.snapshot()).content, /22 samples/); + assert.strictEqual((await h.snapshot()).charts.length, 0); +}, { holdClock: true }); + +test('calibration-error-keeps-identity', async h => { + await h.open('B'); + await h.releaseClock(500); + assert.match(await h.page.locator('.obs-info-grid').innerText(), /Test radio/); + await h.release('B', 7); + assert.strictEqual((await h.snapshot()).charts.length, 4); + assert.strictEqual(await h.page.locator('.skew-detail-section').count(), 0); +}, { holdClock: true }); + +test('identity-error-with-late-secondary-results', async h => { + await h.open('B'); + const before = await h.snapshot(); + assert.match(before.content, /Error: API 404:/); + await h.release('B', 7); + await h.releaseClock(); + assert.deepStrictEqual(await h.snapshot(), before, 'secondary success cannot hide a failed identity'); +}, { holdClock: true, identityFailure: true }); + +test('early-identity-route-with-held-calibration', async h => { + await h.open('A'); + assert.strictEqual(await h.page.locator('#obsTitle').textContent(), 'Observer A visit 1'); + await h.open('B'); + await h.release('B', 7); + await h.releaseClock(); + await h.current('B', 7); + const before = await h.snapshot(); + await h.release('A', 7); + assert.deepStrictEqual(await h.snapshot(), before, 'old observer cannot mutate independently completed sections'); +}, { holdClock: true }); + test('late-success', async h => { await h.open('A'); await h.open('B'); @@ -229,9 +334,10 @@ test('dispose-and-controls', async h => { try { for (const [device, viewport] of Object.entries({ desktop: { width: 1400, height: 900 }, mobile: { width: 390, height: 844 } })) { for (const theme of ['light', 'dark']) { - for (const { name, run } of cases) { + for (const { name, run, options } of cases) { const label = device + '-' + theme + '-' + name; - const h = await setup(browser, viewport, theme); + const h = await setup(browser, viewport, theme, options); + h.label = label; try { await run(h); assert.deepStrictEqual(h.errors, [], 'no uncaught browser errors'); diff --git a/tests/unit/test-observer-detail-ownership.js b/tests/unit/test-observer-detail-ownership.js index ea2747ef..f091c847 100644 --- a/tests/unit/test-observer-detail-ownership.js +++ b/tests/unit/test-observer-detail-ownership.js @@ -11,15 +11,24 @@ function harness() { const charts = []; let page; function element(id) { - const el = { textContent: '', firstChild: { textContent: '' }, listeners: {}, + const children = []; + const el = { textContent: '', firstChild: { textContent: '' }, listeners: {}, htmlWrites: 0, + remove() { children.forEach(child => child.remove()); elements.delete(id); }, addEventListener(name, fn) { this.listeners[name] = fn; }, setAttribute() {} }; let html = ''; Object.defineProperty(el, 'innerHTML', { get() { return html; }, set(value) { + el.htmlWrites++; + children.forEach(child => child.remove()); + children.length = 0; html = value; el.firstChild = { textContent: '' }; - for (const match of value.matchAll(/id="([^"]+)"/g)) elements.set(match[1], element(match[1])); + for (const match of value.matchAll(/id="([^"]+)"/g)) { + const child = element(match[1]); + children.push(child); + elements.set(match[1], child); + } }, }); return el; @@ -55,19 +64,29 @@ function harness() { } function snapshot() { return { title: elements.get('obsTitle').textContent, - html: elements.get('obsDetailContent').innerHTML, + // Include independently updated child regions, as a real DOM does. + html: Array.from(elements.values()).map(el => el.innerHTML).join('\n'), error: elements.get('obsDetailContent').firstChild.textContent, charts: charts.filter(c => !c.destroyed) }; } return { page, app, elements, requests, charts, visit, days, snapshot }; } +function identity(id, label = id) { + return { id, name: label, last_seen: null, model: 'Test radio', radio: '910.0,250,10,5' }; +} +function analytics(label) { + return { timeline: [{ label, count: 1 }], packetTypes: { 4: 1 }, + nodesTimeline: [{ label, count: 1 }], snrDistribution: [{ range: '0 to 5', count: 1 }], recentPackets: [] }; +} +function calibration(id) { + return [{ observerID: 'other', offsetSec: 99, samples: 99 }, + { observerID: id, offsetSec: 1, samples: 1 }, { observerID: id, offsetSec: 2, samples: 2 }]; +} function complete(requests, id, label = id) { - requests[0].resolve({ id, name: label, last_seen: null }); - requests[1].resolve({ timeline: [{ label, count: 1 }], packetTypes: { 4: 1 }, - nodesTimeline: [{ label, count: 1 }], snrDistribution: [{ range: '0 to 5', count: 1 }], recentPackets: [] }); - requests[2].resolve([{ observerID: 'other', offsetSec: 99, samples: 99 }, - { observerID: id, offsetSec: 1, samples: 1 }, { observerID: id, offsetSec: 2, samples: 2 }]); + requests[0].resolve(identity(id, label)); + requests[1].resolve(analytics(label)); + requests[2].resolve(calibration(id)); } // Yield one event-loop turn after resolving promises, rather than guessing a delay. const settled = () => new Promise(resolve => setImmediate(resolve)); @@ -78,6 +97,92 @@ async function test(name, fn) { } (async () => { + for (const order of [[0, 1, 2], [0, 2, 1], [1, 2, 0], [2, 1, 0], [1, 0, 2], [2, 0, 1]]) { + await test('progressive detail handles completion order ' + order.join(','), async () => { + const h = harness(); + const pending = h.visit('A'); + assert.strictEqual(pending.length, 3, 'all requests start before any resolves'); + let identityWrites; + const done = new Set(); + for (const index of order) { + pending[index].resolve([identity('A'), analytics('current'), calibration('A')][index]); + done.add(index); + await settled(); + const state = h.snapshot(); + if (done.has(0)) { + assert.strictEqual(state.title, 'A', 'identity does not wait for secondary responses'); + assert.match(state.html, /Offline/); + assert.match(state.html, /Test radio/); + assert.match(state.html, /910.0 MHz/); + if (identityWrites === undefined) identityWrites = h.elements.get('obsDetailContent').htmlWrites; + assert.strictEqual(h.elements.get('obsDetailContent').htmlWrites, identityWrites, + 'secondary responses never rebuild the metadata DOM'); + assert.strictEqual(state.charts.length, done.has(1) ? 4 : 0); + assert.strictEqual(/2 seconds/.test(state.html), done.has(2), 'calibration renders independently'); + } else { + assert.strictEqual(state.charts.length, 0, 'secondary results wait for a valid identity'); + } + } + assert.strictEqual(h.charts.length, 4, 'charts are only constructed once'); + assert.strictEqual(h.requests.length, 3, 'no extra requests'); + }); + } + + for (const failBeforeIdentity of [false, true]) { + await test('analytics failure preserves identity, including early failure=' + failBeforeIdentity, async () => { + const h = harness(); + const pending = h.visit('A'); + if (failBeforeIdentity) pending[1].reject(new Error('')); + pending[0].resolve(identity('A')); + await settled(); + if (!failBeforeIdentity) pending[1].reject(new Error('')); + pending[2].resolve(calibration('A')); + await settled(); + assert.strictEqual(h.snapshot().title, 'A'); + assert.match(h.snapshot().html, /Test radio/); + assert.match(h.snapshot().html, /2 seconds/); + assert.strictEqual(h.elements.get('obsAnalytics')?.firstChild.textContent, 'Error: '); + assert.doesNotMatch(h.snapshot().html, //, 'error uses textContent'); + assert.strictEqual(h.snapshot().charts.length, 0); + }); + } + + await test('identity failure stays visible when secondary responses settle later', async () => { + const h = harness(); + const pending = h.visit('A'); + pending[0].reject(new Error('')); + await settled(); + const before = h.snapshot(); + assert.strictEqual(before.error, 'Error: '); + assert.doesNotMatch(before.html, //); + pending[1].resolve(analytics('unused')); + pending[2].reject(new Error('unused calibration failure')); + await settled(); + assert.deepStrictEqual(h.snapshot(), before); + }); + + for (const leave of ['route', 'days', 'revisit', 'destroy']) { + for (const failure of [false, true]) { + await test('early identity then ' + leave + ' rejects stale secondary ' + (failure ? 'errors' : 'results'), async () => { + const h = harness(); + const old = h.visit('A'); + old[0].resolve(identity('A', 'old identity')); + await settled(); + assert.strictEqual(h.snapshot().title, 'old identity'); + if (leave === 'route') complete(h.visit('B'), 'B'); + if (leave === 'days') complete(h.days(1), 'A', 'current window'); + if (leave === 'revisit') { complete(h.visit('B'), 'B'); complete(h.visit('A'), 'A', 'new visit'); } + if (leave === 'destroy') h.page.destroy(); + await settled(); + const before = h.snapshot(); + if (failure) { old[1].reject(new Error('stale analytics')); old[2].reject(new Error('stale clock')); } + else { old[1].resolve(analytics('stale')); old[2].resolve(calibration('A')); } + await settled(); + assert.deepStrictEqual(h.snapshot(), before); + }); + } + } + await test('current success renders four charts and the last matching calibration without extra calls', async () => { const h = harness(); complete(h.visit('A /?'), 'A /?', 'Current observer');