diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index f23dc97a..a568d9bc 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -384,6 +384,7 @@ jobs: BASE_URL=http://localhost:13581 node test-table-fluid-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-charts-fluid-1058-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-slideover-1056-e2e.js 2>&1 | tee -a e2e-output.txt + BASE_URL=http://localhost:13581 node test-issue-1692-packets-init-parallel-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-slideover-1168-munger-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-logo-pulse-1173-e2e.js 2>&1 | tee -a e2e-output.txt BASE_URL=http://localhost:13581 node test-issue-1122-packets-filter-ux-e2e.js 2>&1 | tee -a e2e-output.txt diff --git a/public/packets.js b/public/packets.js index a6948dcb..5c6c0ebc 100644 --- a/public/packets.js +++ b/public/packets.js @@ -139,6 +139,10 @@ // columns and removing the pill mutate layout and re-trigger ResizeObserver, // which would otherwise immediately stomp on the reveal the user just asked for. const lastWrapW = new WeakMap(); + // Parallel map of tbody MutationObservers per wired table. Separate from + // `wired` (which stores the ResizeObserver) so the existing sweep/dispose + // contract on `wired` stays unchanged for callers reading its shape. + const wiredMO = new WeakMap(); // Sweep tables that have been detached from the DOM (e.g. SPA destroyed // their page) and release their ResizeObserver. Called opportunistically on // every register() — cheap O(n) over wired tables, n is tiny in practice. @@ -146,6 +150,8 @@ wired.forEach((ro, t) => { if (!t || !t.isConnected) { if (ro) { try { ro.disconnect(); } catch (_) {} } + const mo = wiredMO.get(t); + if (mo) { try { mo.disconnect(); } catch (_) {} wiredMO.delete(t); } wired.delete(t); } }); @@ -172,6 +178,29 @@ ro.observe(wrap); } } + // #1693 r3: re-apply on tbody mutations SYNCHRONOUSLY. Incremental + // renderTableRows() calls (observer-decorated re-render, HopResolver + // re-render, etc.) add fresh s after register() ran. Without + // re-running apply() those new cells miss the `col-hidden` class until + // the next resize/window event, which made the mobile E2E flake + // (`td.col-observer` visible at 375px). + // + // r2 used a 100ms setTimeout debounce; the mobile E2E test + // (test-observer-iata-1188) reads `td.col-observer` display IMMEDIATELY + // after waitForSelector(first row) — the debounce hadn't fired yet, so + // the test still saw the unhidden column. Run apply() synchronously on + // each mutation: apply() is cheap (single pass over header + cells, + // toggling a class) and idempotent, so consecutive mutations in a + // render burst just re-run the same cheap loop. + const tbody = table.querySelector('tbody'); + if (tbody && typeof MutationObserver !== 'undefined') { + const mo = new MutationObserver(() => { + if (!table.isConnected) return; + apply(table); + }); + mo.observe(tbody, { childList: true }); + wiredMO.set(table, mo); + } wired.set(table, ro); apply(table); } @@ -683,6 +712,10 @@ let pauseBuffer = []; let observers = []; let observerMap = new Map(); // id → observer for O(1) lookups (#383) + // #1693 — hook set by renderLeft() so loadObservers() can refresh the + // observer-filter dropdown when observers resolve after the filter UI is + // already built (decouples observer fetch from row render). + let _rebuildObserverMenu = null; let regionMap = {}; const TYPE_NAMES = { 0:'Request', 1:'Response', 2:'Direct Msg', 3:'ACK', 4:'Advert', 5:'Channel Msg', 6:'Group Data', 7:'Anon Req', 8:'Path', 9:'Trace', 10:'Multipart', 11:'Control', 15:'Raw Custom' }; function typeName(t) { return TYPE_NAMES[t] ?? `Type ${t}`; } @@ -1086,8 +1119,13 @@ document.getElementById('pktRight').addEventListener('click', function(e) { if (e.target.closest('.panel-close-btn')) closeDetailPanel(); }); - await loadObservers(); - loadPackets(); + // #1692 — parallelize loadObservers + loadPackets. Both hit independent + // API endpoints (/api/observers, /api/packets) and have no data + // dependency between the two fetches; only the post-fetch renderLeft() + // code reads `observers` (for the observer filter dropdown), so awaiting + // them together rather than in series halves worst-case time-to-first-row + // on slow links / loaded CI runners without changing render contracts. + await Promise.all([loadObservers(), loadPackets()]); // Auto-select packet detail when arriving via hash URL if (directPacketHash) { @@ -1327,6 +1365,13 @@ const data = await api('/observers', { ttl: CLIENT_TTL.observers }); observers = data.observers || []; observerMap = new Map(observers.map(o => [o.id, o])); + // #1693 — observers now resolve asynchronously relative to row render. + // If the filter UI was already built with an empty observer list, + // re-populate the observer-multi-select dropdown so the filter is + // usable as soon as observers arrive. + if (typeof _rebuildObserverMenu === 'function') { + try { _rebuildObserverMenu(); } catch {} + } } catch {} } @@ -1401,31 +1446,32 @@ totalCount = flat.length; } - // Pre-resolve from server-side resolved_path (preferred, no client-side disambiguation needed) - await cacheResolvedPaths(packets); - - // Pre-resolve all path hops to node names (fallback for packets without resolved_path) - const allHops = new Set(); - for (const p of packets) { - try { getParsedPath(p).forEach(h => allHops.add(h)); } catch {} - } - if (allHops.size) await resolveHops([...allHops]); - - // Per-observer batch resolve for ambiguous hops (context-aware disambiguation) - const hopsByObserver = {}; - for (const p of packets) { - if (!p.observer_id) continue; + // #1693 — hop resolution is OFF the critical path. Previously + // cacheResolvedPaths() + resolveHops() each chained through + // ensureHopResolver(), which calls api('/observers'). Under the + // packets-init race (#1692) the observers fetch is still in-flight + // (parallelised but slow), and the api() in-flight dedupe maps the + // hop-resolver's /observers call onto the same pending promise. + // Result: rows can't render until observers resolves, defeating the + // #1692 parallelisation. Hop *names* are a presentation enhancement + // (paths render as hex prefixes until names arrive), so resolve them + // in the background and re-render rows when done. + const hopJob = (async () => { try { - const path = getParsedPath(p); - const ambiguous = path.filter(h => hopNameCache[h]?.ambiguous); - if (ambiguous.length) { - if (!hopsByObserver[p.observer_id]) hopsByObserver[p.observer_id] = new Set(); - ambiguous.forEach(h => hopsByObserver[p.observer_id].add(h)); + await cacheResolvedPaths(packets); + const allHops = new Set(); + for (const p of packets) { + try { getParsedPath(p).forEach(h => allHops.add(h)); } catch {} } - } catch {} - } - // Ambiguous hops are already resolved by HopResolver client-side - // No need for per-observer server API calls + if (allHops.size) await resolveHops([...allHops]); + // Re-render rows so resolved hop names replace hex prefixes. + if (filtersBuilt) renderTableRows(); + } catch (e) { + console.warn('[packets] background hop resolution failed:', e); + } + })(); + // Don't await — let rows render with hex hops, then upgrade in place. + void hopJob; // Restore expanded group children (parallel fetch, Map lookup) if (groupByHash && expandedHashes.size > 0) { @@ -1652,12 +1698,21 @@ function buildObserverMenu() { const allChecked = selectedObservers.size === 0; let html = ``; - for (const o of observers) { - const checked = selectedObservers.has(String(o.id)) ? 'checked' : ''; - html += ``; + if (observers.length === 0) { + // #1693 — observers fetch may still be in flight (decoupled from + // packet row render). Show a disabled placeholder until loadObservers + // resolves and calls _rebuildObserverMenu(). + html += ``; + } else { + for (const o of observers) { + const checked = selectedObservers.has(String(o.id)) ? 'checked' : ''; + html += ``; + } } obsMenu.innerHTML = html; } + // #1693 — expose for loadObservers() to refresh on resolve. + _rebuildObserverMenu = () => { buildObserverMenu(); updateObsTrigger(); }; function updateObsTrigger() { if (selectedObservers.size === 0 || selectedObservers.size === observers.length) { obsTrigger.textContent = 'All Observers ▾'; diff --git a/test-issue-1692-packets-init-parallel-e2e.js b/test-issue-1692-packets-init-parallel-e2e.js new file mode 100644 index 00000000..ca216111 --- /dev/null +++ b/test-issue-1692-packets-init-parallel-e2e.js @@ -0,0 +1,87 @@ +/** + * E2E (#1692): /packets init() must NOT serialize loadObservers → loadPackets. + * + * Before #1692 fix: init() does `await loadObservers(); loadPackets();` so + * /api/packets cannot start until /api/observers resolves. Under a slow + * observers fetch (CI load, large fixture DB, slow link) the first table row + * does not appear until well after the observers call returns. + * + * Repro strategy: stub /api/observers with a deterministic 4s delay via + * page.route(), then measure wallclock time from navigation start to the + * first `tr[data-hash]` appearance in #pktBody. Serial behavior pushes + * first-row past 4s. Parallel (fixed) behavior should land first-row well + * under 3s — the /api/packets call runs in parallel. + * + * Acceptance budget: first-row < 3000ms. Headroom of ~1s below the 4s + * observers-stub delay so a serial implementation reliably trips the gate + * even with timing jitter, and a parallel implementation comfortably passes. + * + * Usage: BASE_URL=http://localhost:13581 node test-issue-1692-packets-init-parallel-e2e.js + */ +'use strict'; +const { chromium } = require('playwright'); + +const BASE = process.env.BASE_URL || 'http://localhost:13581'; +const OBSERVERS_DELAY_MS = 4000; +const FIRST_ROW_BUDGET_MS = 3000; + +let passed = 0, failed = 0; +async function step(name, fn) { + try { await fn(); passed++; console.log(' \u2713 ' + name); } + catch (e) { failed++; console.error(' \u2717 ' + name + ': ' + e.message); } +} +function assert(c, m) { if (!c) throw new Error(m || 'assertion failed'); } + +(async () => { + const browser = await chromium.launch({ + headless: true, + executablePath: process.env.CHROMIUM_PATH || undefined, + args: ['--no-sandbox', '--disable-gpu', '--disable-dev-shm-usage'], + }); + const ctx = await browser.newContext({ viewport: { width: 1400, height: 900 } }); + const page = await ctx.newPage(); + page.setDefaultTimeout(15000); + page.on('pageerror', (e) => console.error('[pageerror]', e.message)); + + console.log(`\n=== #1692 packets init() parallel-fetch E2E against ${BASE} ===`); + + // Stub /api/observers with a 4s artificial delay. The real response body + // is fetched and forwarded so the page still gets a valid observer list — + // only the latency is amplified. This isolates the init() sequencing + // behavior from network/IO variance on the CI runner. + await page.route('**/api/observers**', async (route) => { + const req = route.request(); + let body = '{"observers":[]}'; + let contentType = 'application/json'; + try { + const upstream = await page.context().request.fetch(req); + body = await upstream.text(); + contentType = upstream.headers()['content-type'] || contentType; + } catch (_) { /* fall through with empty observers */ } + await new Promise(r => setTimeout(r, OBSERVERS_DELAY_MS)); + await route.fulfill({ status: 200, contentType, body }); + }); + + await step(`first table row appears < ${FIRST_ROW_BUDGET_MS}ms despite ${OBSERVERS_DELAY_MS}ms /api/observers stub`, async () => { + // Clean SPA state — mirrors gotoPackets() pattern from test-e2e-playwright.js. + await page.goto(BASE, { waitUntil: 'domcontentloaded' }); + await page.evaluate(() => { + localStorage.removeItem('meshcore-groupbyhash'); + localStorage.setItem('meshcore-time-window', '525600'); + }); + + const t0 = Date.now(); + await page.goto(BASE + '/#/packets', { waitUntil: 'domcontentloaded' }); + await page.waitForSelector('#pktBody tr[data-hash]', { timeout: 10000 }); + const elapsed = Date.now() - t0; + console.log(` first-row elapsed: ${elapsed}ms (budget ${FIRST_ROW_BUDGET_MS}ms, observers stub ${OBSERVERS_DELAY_MS}ms)`); + assert(elapsed < FIRST_ROW_BUDGET_MS, + `first tr[data-hash] took ${elapsed}ms, expected < ${FIRST_ROW_BUDGET_MS}ms — packets.js::init() is serializing loadObservers → loadPackets (#1692)`); + }); + + await page.unroute('**/api/observers**'); + await browser.close(); + + console.log(`\nResults: ${passed} passed, ${failed} failed`); + process.exit(failed ? 1 : 0); +})();