fix(emr): dashboard error-page encoding + loud port-conflict failure#1930
Conversation
The did-fail-load handler built a data:text/html URL from a raw template literal containing '#' characters (hex colors). A '#' in a URL starts the fragment, so the document was truncated mid-<style> and the Connection Error page rendered as an empty black window. Encode the HTML with encodeURIComponent so the full error page renders. Pre-existing on electron 42.3.3; reproduced identically on 42.3.3 and 43.0.0, so unrelated to the dependabot bump — split out as a follow-up.
gaia api defaults to the same port (8080) as gaia-emr dashboard. When the
port was taken, uvicorn printed one error line and exited 0 while the CLI
had already launched Electron at :8080 — the user saw a connection-error
window (or the wrong app), and the dashboard Settings badge blamed
Lemonade ("Not Running") for what is a dashboard-port problem.
run_dashboard now probes the port before starting the agent and raises an
actionable RuntimeError naming the conflict and the --port escape hatch;
cmd_dashboard prints it and exits 1 instead of launching Electron at a
dead URL. The probe mirrors asyncio's bind semantics (SO_REUSEADDR on
POSIX only — on Windows it would bind over live listeners).
itomek
changed the title
|
Verdict: Approve ✅ This PR fixes the "blank EMR dashboard" experience in two places: the Electron error page now URL-encodes its HTML so the 🔍 Technical detailsStrengths
🟢 Minor / non-blocking
|
|
🟡 The IPv4-only probe silently misfires if 🔍 Technical details
probe = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
...
probe.bind((host, port))
That message is wrong — the port may not be in use at all. Minimal fix: detect IPv6 hosts and use import errno
except OSError as e:
if e.errno in (errno.EADDRINUSE, errno.EADDRNOTAVAIL):
raise RuntimeError(
f"EMR dashboard cannot start: {host}:{port} is already in use ..."
) from e
raise # propagate socket-family / resolution errors unmaskedThe default |
The pre-flight probe was AF_INET-only and translated every bind error into "port already in use" — for an IPv6 --host (e.g. ::1) it misfired with EINVAL and reported the wrong cause. Pick the address family from the host, translate only EADDRINUSE into the actionable conflict error, and let uvicorn's own bind report anything else accurately.
|
Good catch on the IPv6 misfire — fixed in add851a. The probe now picks its address family from the host ( |
2 participants
Fixes #1932.
When the EMR dashboard can't be reached, users got a completely empty black window with no hint of what went wrong — and chasing that down surfaced a second, related failure. This PR fixes both halves of the "blank dashboard" experience:
did-fail-loadhandler built the page as an unencodeddata:text/html,URL; the first#in the hex colors starts the URL fragment, truncating the document mid-<style>(body rendered with 0 children). Now encoded withencodeURIComponent. Pre-existing bug (reproduced identically on electron 42.3.3 and 43.0.0) — surfaced while verifying chore(deps): bump electron from 42.3.3 to 43.0.0 in /hub/agents/python/emr/gaia_agent_emr/dashboard/electron in the emr-dashboard-dependencies group across 1 directory #1805.gaia apidefaults to the same port 8080 asgaia-emr dashboard. When the port is occupied, uvicorn printed one error line and exited with code 0, while the CLI had already launched Electron pointing at :8080 — the user saw the connection-error page (or the wrong app) with no clue why. The dashboard UI then made it worse: when the frontend can't reach its backend, the Settings badge falls back to "Lemonade Server: ○ Not Running", blaming Lemonade for a dashboard-port problem.run_dashboardnow probes the port up front and raises an actionable error naming the conflict and the--portescape hatch; the CLI exits 1 with that message instead of launching Electron at a dead URL.Evidence — verified on electron 43.0.0 (the #1805 bump, now merged into this branch)
Error page (launched
electron .with nothing on :8080, captured over CDP--remote-debugging-port=9333+Page.captureScreenshot):Happy path — the packaged
gaia-emr dashboard(editable install of this branch) serving the real dashboard, rendered by the electron 43 wrapper:Port conflict — real CLI, with
gaia api startgenuinely holding :8080 (Lemonade healthy on :13305 the whole time):Before this PR the same scenario printed uvicorn's
[Errno 48] address already in useline, exited 0, and launched Electron at the dead/wrong URL anyway.🔍 CDP DOM check for the error page (before → after, electron 43.0.0)
Before (unencoded, on main):
After (this PR):
The screenshots are referenced from SHA-pinned evidence commits in this PR's history; the branch tip tree is clean, so a squash merge brings in only the code fixes.
Test plan
cd hub/agents/python/emr && PYTHONPATH=. python -m pytest tests/ -q→ 57 passed (includes the two new port-conflict tests)cd hub/agents/python/emr/gaia_agent_emr/dashboard/electron && npm install && EMR_DASHBOARD_URL=http://localhost:8080 npx electron .→ styled "Connection Error" page with a Retry button (not a blank black window)gaia api start):gaia-emr dashboard→ immediate red error naming the conflict, exit 1, no Electron windowgaia-emr dashboard→ dashboard loads in the Electron windowKnown limitations (pre-existing, not addressed here)
main.jstreatsprocess.argv[2]as the dashboard URL, so extra CLI flags (e.g.--remote-debugging-port) get swallowed as the URL;EMR_DASHBOARD_URLworks around it.show: false, the window only appears onready-to-show— if the very first load fails before that fires, the window never becomes visible./api/init/statusreadshealth.get("context_size", 0), which Lemonade 10.7.0 no longer returns — tracked in EMR dashboard always shows Lemonade context size as unknown (reads a field Lemonade ≥10.x no longer returns) #1935 with a fix up in fix(emr): dashboard context size read from Lemonade ≥10.x health payload #1936.