fix(tracing): reuse the dashboard port only when it is our own viewer

raven/cli/tracing_commands.py

@@ -16,10 +16,12 @@
 
 from __future__ import annotations
 
+import json
 import os
 import socket
 import subprocess
 import time
+import urllib.request
 import webbrowser
 from pathlib import Path
 
@@ -42,6 +44,32 @@ def _port_live(port: int) -> bool:
         return sock.connect_ex(("127.0.0.1", port)) == 0
 
 
+def _viewer_health(port: int) -> bool:
+    """True only if *our* tracing viewer is serving on ``port``.
+
+    A live port is not enough: a stale viewer from an older layout, or an
+    unrelated server (e.g. another observability tool), can hold it and 404
+    every request. The viewer answers ``/api/health`` with ``{"ok": true}``;
+    a foreign server does not, so this distinguishes reuse from a clash.
+    """
+    try:
+        with urllib.request.urlopen(f"http://127.0.0.1:{port}/api/health", timeout=0.5) as resp:
+            if resp.status != 200:
+                return False
+            data = json.loads(resp.read().decode("utf-8"))
+    except Exception:  # noqa: BLE001 — any failure means "not our viewer"
+        return False
+    return isinstance(data, dict) and data.get("ok") is True
+
+
+def _find_free_port(start: int, span: int = 20) -> int | None:
+    """First free port in ``[start, start+span)``, or ``None`` if all are taken."""
+    for candidate in range(start, start + span):
+        if not _port_live(candidate):
+            return candidate
+    return None
+
+
 def _viewer_env(port: int) -> dict:
     env = dict(os.environ)
     # Both server.js and log-store.js resolve the trace dir from this var.
@@ -73,11 +101,27 @@ def _server_js() -> Path:
 
 
 def _open_dashboard(port: int) -> None:
-    url = f"http://127.0.0.1:{port}/"
     if _port_live(port):
-        console.print(f"Tracing dashboard already running at [cyan]{url}[/cyan]")
-        webbrowser.open(url)
-        return
+        if _viewer_health(port):
+            url = f"http://127.0.0.1:{port}/"
+            console.print(f"Tracing dashboard already running at [cyan]{url}[/cyan]")
+            webbrowser.open(url)
+            return
+        # Port is held by something that is NOT our viewer (a stale instance or
+        # an unrelated server) — reusing it would open a broken/foreign page.
+        # Move to the next free port instead of clashing.
+        free = _find_free_port(port + 1)
+        if free is None:
+            console.print(
+                f"[red]Port {port} is in use by another process, and no free port "
+                f"was found nearby.[/red] Stop it, or pass --port to pick one."
+            )
+            raise typer.Exit(1)
+        console.print(
+            f"[yellow]Port {port} is held by another process (not the tracing viewer); "
+            f"starting on {free} instead.[/yellow]"
+        )
+        port = free
 
     node = _resolve_node()
     server_js = _server_js()
@@ -93,11 +137,12 @@ def _open_dashboard(port: int) -> None:
         start_new_session=True,
     )
 
-    for _ in range(24):  # wait up to ~6s for the server to bind
-        if _port_live(port):
+    for _ in range(24):  # wait up to ~6s for the server to actually serve
+        if _viewer_health(port):
             break
         time.sleep(0.25)
 
+    url = f"http://127.0.0.1:{port}/"
     console.print(f"Tracing dashboard at [cyan]{url}[/cyan]")
     webbrowser.open(url)
 

tests/test_cli_tracing_commands.py

@@ -0,0 +1,80 @@
+"""Unit tests for ``raven/cli/tracing_commands.py`` viewer-launch helpers.
+
+Focus: the port-reuse guard. A live port must only be reused when it is *our*
+tracing viewer (answers ``/api/health`` with ``{"ok": true}``); a foreign or
+stale server holding the port must be detected so the launcher can move on.
+"""
+
+from __future__ import annotations
+
+import json
+import socket
+import threading
+from http.server import BaseHTTPRequestHandler, HTTPServer
+
+from raven.cli import tracing_commands as tc
+
+
+def _free_port() -> int:
+    s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    s.bind(("127.0.0.1", 0))
+    port = s.getsockname()[1]
+    s.close()
+    return port
+
+
+class _Handler(BaseHTTPRequestHandler):
+    def do_GET(self):  # noqa: N802 — BaseHTTPRequestHandler API
+        if self.path == "/api/health" and getattr(self.server, "health_ok", False):
+            body = json.dumps({"ok": True, "port": 0, "stateDir": "/x"}).encode()
+            self.send_response(200)
+            self.send_header("Content-Type", "application/json")
+            self.end_headers()
+            self.wfile.write(body)
+        else:
+            self.send_response(404)
+            self.end_headers()
+            self.wfile.write(b"Not found")
+
+    def log_message(self, *_a):  # silence test server logging
+        pass
+
+
+def _serve(port: int, health_ok: bool) -> HTTPServer:
+    srv = HTTPServer(("127.0.0.1", port), _Handler)
+    srv.health_ok = health_ok  # type: ignore[attr-defined]
+    threading.Thread(target=srv.serve_forever, daemon=True).start()
+    return srv
+
+
+def test_viewer_health_false_when_nothing_listening():
+    assert tc._viewer_health(_free_port()) is False
+
+
+def test_viewer_health_false_for_foreign_server():
+    port = _free_port()
+    srv = _serve(port, health_ok=False)
+    try:
+        assert tc._viewer_health(port) is False
+    finally:
+        srv.shutdown()
+
+
+def test_viewer_health_true_for_our_viewer():
+    port = _free_port()
+    srv = _serve(port, health_ok=True)
+    try:
+        assert tc._viewer_health(port) is True
+    finally:
+        srv.shutdown()
+
+
+def test_find_free_port_skips_occupied():
+    port = _free_port()
+    srv = _serve(port, health_ok=False)  # occupy `port`
+    try:
+        got = tc._find_free_port(port)
+        assert got is not None
+        assert got != port
+    finally:
+        srv.shutdown()