🐛 fix(crux): surface the health 503 cause from WaitReady instead of a bare deadline error

changelog.d/fixed-crux-waitready-unavailable-cause.md

@@ -0,0 +1 @@
+- crux: WaitReady now surfaces the health route's last answer (e.g. "tmux not found on PATH") instead of a bare deadline error, and returns promptly on a 503 unavailable verdict.

src/crux/api/api.go

@@ -111,7 +111,8 @@ type Health struct {
 	// Error and Code are set only on a non-2xx answer and share their wire
 	// keys with api.Error, so the 503 body also decodes as the Error that
 	// Client surfaces in a *StatusError: a caller polling health (the
-	// supervisor's WaitReady) sees "tmux not found on PATH", not a bare 503.
+	// supervisor's WaitReady) sees "tmux not found on PATH", not a bare 503:
+	// WaitReady returns that 503 at once instead of polling to its deadline.
 	Error string `json:"error,omitempty"`
 	Code  string `json:"code,omitempty"`
 }

src/crux/api/client.go

@@ -135,6 +135,13 @@ func IsUnauthorized(err error) bool {
 	return errors.As(err, &se) && se.Status == http.StatusUnauthorized
 }
 
+// IsUnavailable reports whether err is a 503 carrying CodeUnavailable: a
+// server that is up but cannot serve.
+func IsUnavailable(err error) bool {
+	var se *StatusError
+	return errors.As(err, &se) && se.Status == http.StatusServiceUnavailable && se.Body.Code == CodeUnavailable
+}
+
 // Health is GET /v1/health.
 func (c *Client) Health(ctx context.Context) (Health, error) {
 	var out Health
@@ -358,21 +365,33 @@ func (a Address) target() string {
 }
 
 // WaitReady polls GET /v1/health until it answers or ctx ends; it is for a
-// caller that just started `crux serve` and needs to know when to begin.
+// caller that just started `crux serve` and needs to know when to begin. A
+// 401 or a 503 unavailable verdict (the server is up but cannot serve, e.g.
+// "tmux not found on PATH") returns at once. On any other failure it keeps
+// polling, and when ctx ends the error wraps ctx.Err() and names the last
+// health answer.
 func (c *Client) WaitReady(ctx context.Context, interval time.Duration) error {
 	if interval <= 0 {
 		interval = 100 * time.Millisecond
 	}
 	t := time.NewTicker(interval)
 	defer t.Stop()
+	var lastErr error
 	for {
 		if _, err := c.Health(ctx); err == nil {
 			return nil
-		} else if IsUnauthorized(err) {
+		} else if IsUnauthorized(err) || IsUnavailable(err) {
 			return err
+		} else if ctx.Err() == nil || lastErr == nil {
+			// A probe cut short by ctx ending is not a health answer; keep
+			// the last real one so the deadline error still names it.
+			lastErr = err
 		}
 		select {
 		case <-ctx.Done():
+			if lastErr != nil {
+				return fmt.Errorf("%w (last health answer: %v)", ctx.Err(), lastErr)
+			}
 			return ctx.Err()
 		case <-t.C:
 		}

src/crux/api/client_test.go

@@ -538,10 +538,56 @@ func TestClientHealthUnavailableNamesCause(t *testing.T) {
 	if !strings.Contains(err.Error(), "tmux not found on PATH") {
 		t.Fatalf("error does not name tmux: %v", err)
 	}
-	// WaitReady keeps polling a 503 (it is not 401) and ends with ctx.
-	ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+	// WaitReady returns the 503 unavailable verdict at once.
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
-	if err := c.WaitReady(ctx, time.Millisecond); !errors.Is(err, context.DeadlineExceeded) {
+	if err := c.WaitReady(ctx, time.Millisecond); !IsUnavailable(err) {
 		t.Fatalf("wait ready: %v", err)
 	}
 }
+
+// WaitReady returns promptly on a 503 unavailable verdict and names its cause.
+func TestWaitReadyUnavailableReturnsPromptly(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", ContentTypeJSON)
+		w.WriteHeader(http.StatusServiceUnavailable)
+		_, _ = w.Write([]byte(`{"error":"tmux not found on PATH: x","code":"unavailable"}`))
+	}))
+	t.Cleanup(srv.Close)
+	c, err := NewClient(srv.URL, "tok")
+	if err != nil {
+		t.Fatal(err)
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	start := time.Now()
+	err = c.WaitReady(ctx, time.Hour)
+	if err == nil || !strings.Contains(err.Error(), "tmux not found on PATH") {
+		t.Fatalf("wait ready: %v", err)
+	}
+	if time.Since(start) > 2*time.Second {
+		t.Fatalf("wait ready took %v", time.Since(start))
+	}
+}
+
+// A health route that keeps failing is polled until ctx ends, and the final
+// error carries the last answer next to the deadline.
+func TestWaitReadyDeadlineNamesLastAnswer(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	t.Cleanup(srv.Close)
+	c, err := NewClient(srv.URL, "tok")
+	if err != nil {
+		t.Fatal(err)
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer cancel()
+	err = c.WaitReady(ctx, 5*time.Millisecond)
+	if !errors.Is(err, context.DeadlineExceeded) {
+		t.Fatalf("wait ready: %v", err)
+	}
+	if !strings.Contains(err.Error(), "500") {
+		t.Fatalf("error does not name the last status: %v", err)
+	}
+}