diff --git a/raven/cli/tracing_commands.py b/raven/cli/tracing_commands.py index e83edae2..7bf30c77 100644 --- a/raven/cli/tracing_commands.py +++ b/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) diff --git a/tests/test_cli_tracing_commands.py b/tests/test_cli_tracing_commands.py new file mode 100644 index 00000000..48cb5e0b --- /dev/null +++ b/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()