fix(visor): bound service-health probes so one dead endpoint can't stall visor state

pkg/visor/api_services.go

@@ -122,13 +122,32 @@ func (v *Visor) ServiceHealth() ([]ServiceHealthEntry, error) {
 	return results, nil
 }
 
+// healthProbeTimeout bounds a single /health probe. ServiceHealth fans every
+// probe out in parallel and wg.Wait()s on them, so an unresponsive endpoint
+// (notably a dmsg server whose /health does not answer over the discovery-routed
+// path) would otherwise stall the WHOLE call — and every caller of it — for the
+// underlying dmsg-HTTP client's full timeout (~10s). That in turn made the
+// curated `visor state` snapshot (which folds ServiceHealth in) take ~10s and
+// time out under load, misreported downstream as empty/zero. A per-probe bound
+// caps the blast radius: a dead endpoint costs at most this long, not the whole
+// client timeout, while healthy services (normally ~300ms) are unaffected.
+const healthProbeTimeout = 4 * time.Second
+
 // doHealthProbe performs a single GET {baseURL}/health and populates a ServiceHealthEntry.
 func doHealthProbe(client *http.Client, name, baseURL, transport string) ServiceHealthEntry { //nolint:unparam
 	entry := ServiceHealthEntry{Name: name, URL: baseURL, Transport: transport}
 
 	reqURL := strings.TrimSuffix(baseURL, "/") + "/health"
+	ctx, cancel := context.WithTimeout(context.Background(), healthProbeTimeout)
+	defer cancel()
 	start := time.Now()
-	resp, err := client.Get(reqURL) //nolint:gosec
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
+	if err != nil {
+		entry.Status = "DOWN"
+		entry.Error = err.Error()
+		return entry
+	}
+	resp, err := client.Do(req) //nolint:gosec
 	entry.LatencyMs = time.Since(start).Milliseconds()
 
 	if err != nil {

pkg/visor/api_services_probe_test.go

@@ -0,0 +1,65 @@
+// Package visor pkg/visor/api_services_probe_test.go c3-vis-core
+package visor
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+)
+
+// TestDoHealthProbe_BoundsHangingEndpoint pins the fix for the ~10s ServiceHealth
+// stall: a single unresponsive /health endpoint must NOT block the probe for the
+// underlying client's full timeout. doHealthProbe caps each request at
+// healthProbeTimeout, so a dead endpoint returns DOWN promptly instead of
+// stalling ServiceHealth (and the `visor state` snapshot that folds it in).
+func TestDoHealthProbe_BoundsHangingEndpoint(t *testing.T) {
+	// A server that never responds within the probe budget: it blocks until the
+	// request context is canceled — which is exactly what doHealthProbe's timeout
+	// does to the underlying connection. That both exercises the bound and lets
+	// httptest tear down promptly once the probe gives up.
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		<-r.Context().Done()
+	}))
+	defer srv.Close()
+
+	start := time.Now()
+	entry := doHealthProbe(srv.Client(), "Hanging", srv.URL, "http")
+	elapsed := time.Since(start)
+
+	// Must return well before the server would have (10s past the budget),
+	// bounded by healthProbeTimeout plus a little scheduling slack.
+	if elapsed > healthProbeTimeout+2*time.Second {
+		t.Fatalf("probe took %v; expected it bounded near healthProbeTimeout (%v)", elapsed, healthProbeTimeout)
+	}
+	if entry.Status != "DOWN" {
+		t.Fatalf("hanging endpoint status = %q; want DOWN", entry.Status)
+	}
+}
+
+// TestDoHealthProbe_HealthyIsUnaffected confirms the bound does not penalize a
+// normally-responding service: it returns OK quickly with the version parsed.
+func TestDoHealthProbe_HealthyIsUnaffected(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		if _, err := w.Write([]byte(`{"build_info":{"version":"v1.2.3"}}`)); err != nil {
+			t.Errorf("write health body: %v", err)
+		}
+	}))
+	defer srv.Close()
+
+	start := time.Now()
+	entry := doHealthProbe(srv.Client(), "Healthy", srv.URL, "http")
+	elapsed := time.Since(start)
+
+	if elapsed > healthProbeTimeout {
+		t.Fatalf("healthy probe took %v; should be near-instant", elapsed)
+	}
+	if entry.Status != "OK" {
+		t.Fatalf("healthy status = %q; want OK", entry.Status)
+	}
+	if entry.Version != "v1.2.3" {
+		t.Fatalf("version = %q; want v1.2.3", entry.Version)
+	}
+}