From 4f3eaa390720ad66d39bf2635a5964479e1e478f Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 13 Feb 2026 01:51:42 +0000 Subject: [PATCH] cachers: add HTTPClient.BestEffortHTTP In prep for further simplifying tailscale.com/cmd/cigocacher, making it remove this logic and use it in a common place. Updates #cleanup Updates tailscale/corp#21262 Signed-off-by: Brad Fitzpatrick --- cachers/http.go | 20 ++++++++ cachers/http_test.go | 111 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/cachers/http.go b/cachers/http.go index 6a30d5f..19c08a1 100644 --- a/cachers/http.go +++ b/cachers/http.go @@ -37,6 +37,11 @@ type HTTPClient struct { // AccessToken optionally specifies a Bearer access token to include // in requests to the server. AccessToken string + + // BestEffortHTTP, when true, makes all HTTP errors non-fatal. + // Get returns a cache miss and Put returns the local disk result, + // silently ignoring any HTTP failures (connection errors, server errors, etc.). + BestEffortHTTP bool } func (c *HTTPClient) httpClient() *http.Client { @@ -103,6 +108,9 @@ func (c *HTTPClient) Get(ctx context.Context, actionID string) (outputID, diskPa res, err := c.httpClient().Do(req) if err != nil { + if c.BestEffortHTTP { + return "", "", nil + } return "", "", err } defer res.Body.Close() @@ -113,6 +121,9 @@ func (c *HTTPClient) Get(ctx context.Context, actionID string) (outputID, diskPa if res.StatusCode != http.StatusOK { msg := tryReadErrorMessage(res) log.Printf("error GET /action/%s: %v, %s", actionID, res.Status, msg) + if c.BestEffortHTTP { + return "", "", nil + } return "", "", fmt.Errorf("unexpected GET /action/%s status %v", actionID, res.Status) } @@ -149,6 +160,9 @@ func (c *HTTPClient) Get(ctx context.Context, actionID string) (outputID, diskPa req.Header.Set("Accept-Encoding", "lz4") res, err = c.httpClient().Do(req) if err != nil { + if c.BestEffortHTTP { + return "", "", nil + } return "", "", err } defer res.Body.Close() @@ -159,6 +173,9 @@ func (c *HTTPClient) Get(ctx context.Context, actionID string) (outputID, diskPa if res.StatusCode != http.StatusOK { msg := tryReadErrorMessage(res) log.Printf("error GET /output/%s: %v, %s", outputID, res.Status, msg) + if c.BestEffortHTTP { + return "", "", nil + } return "", "", fmt.Errorf("unexpected GET /output/%s status %v", outputID, res.Status) } putBody, _, err = responseBody(res) @@ -222,6 +239,9 @@ func (c *HTTPClient) Put(ctx context.Context, actionID, outputID string, size in log.Printf("HTTPClient.Put local disk error: %v", diskErr) return "", diskErr } + if c.BestEffortHTTP { + return v.(string), nil + } return v.(string), httpErr case <-ctx.Done(): return "", ctx.Err() diff --git a/cachers/http_test.go b/cachers/http_test.go index c28fbd1..548a5e1 100644 --- a/cachers/http_test.go +++ b/cachers/http_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/binary" "encoding/json" + "net" "net/http" "net/http/httptest" "os" @@ -191,3 +192,113 @@ func TestHTTPClientPutServerRejectsBody(t *testing.T) { }) } } + +func TestHTTPClientBestEffort(t *testing.T) { + const ( + testActionID = "aabbccdd" + testOutputID = "eeff0011" + ) + testData := []byte("hello, this is cached build output data for testing") + + // closedPortURL returns a URL pointing at a TCP port that immediately + // refuses connections (RST). + closedPortURL := func(t *testing.T) string { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + addr := ln.Addr().String() + ln.Close() + return "http://" + addr + } + + tests := []struct { + name string + httpErr string // "rst" or "500" + }{ + {"connection_refused", "rst"}, + {"server_500", "500"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var baseURL string + if tt.httpErr == "rst" { + baseURL = closedPortURL(t) + } else { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "internal server error", http.StatusInternalServerError) + })) + defer ts.Close() + baseURL = ts.URL + } + + t.Run("Get", func(t *testing.T) { + hc := &HTTPClient{ + BaseURL: baseURL, + Disk: &DiskCache{Dir: t.TempDir()}, + BestEffortHTTP: true, + } + + outputID, diskPath, err := hc.Get(context.Background(), testActionID) + if err != nil { + t.Fatalf("BestEffortHTTP Get returned error: %v", err) + } + if outputID != "" || diskPath != "" { + t.Errorf("expected cache miss, got outputID=%q, diskPath=%q", outputID, diskPath) + } + }) + + t.Run("Get_without_best_effort", func(t *testing.T) { + hc := &HTTPClient{ + BaseURL: baseURL, + Disk: &DiskCache{Dir: t.TempDir()}, + BestEffortHTTP: false, + } + + _, _, err := hc.Get(context.Background(), testActionID) + if err == nil { + t.Fatal("expected error from Get without BestEffortHTTP, got nil") + } + }) + + t.Run("Put", func(t *testing.T) { + hc := &HTTPClient{ + BaseURL: baseURL, + Disk: &DiskCache{Dir: t.TempDir()}, + BestEffortHTTP: true, + } + + diskPath, err := hc.Put(context.Background(), testActionID, testOutputID, int64(len(testData)), bytes.NewReader(testData)) + if err != nil { + t.Fatalf("BestEffortHTTP Put returned error: %v", err) + } + if diskPath == "" { + t.Fatal("diskPath is empty") + } + + got, err := os.ReadFile(diskPath) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, testData) { + t.Errorf("disk content = %q, want %q", got, testData) + } + }) + + t.Run("Put_without_best_effort", func(t *testing.T) { + hc := &HTTPClient{ + BaseURL: baseURL, + Disk: &DiskCache{Dir: t.TempDir()}, + BestEffortHTTP: false, + } + + _, err := hc.Put(context.Background(), testActionID, testOutputID, int64(len(testData)), bytes.NewReader(testData)) + if err == nil { + t.Fatal("expected error from Put without BestEffortHTTP, got nil") + } + }) + }) + } +}