From c67b4adc45e263e3fcb2127e60f3c013df1ed452 Mon Sep 17 00:00:00 2001 From: lakshit verma Date: Mon, 24 Aug 2026 00:38:54 +0530 Subject: [PATCH] loglist: thread context through FetchAll and fetchOne FetchAll issued three sequential 30s-timeout HTTP fetches with no context, so shutdown could stall up to ~90s waiting for the hourly sync loop. runTailers already owns a cancellation context; pass it through so cancelation aborts in-flight list fetches immediately. First tests for the package: context cancellation and error status. --- internal/loglist/loglist.go | 19 +++++++++++---- internal/loglist/loglist_test.go | 41 ++++++++++++++++++++++++++++++++ main.go | 2 +- 3 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 internal/loglist/loglist_test.go diff --git a/internal/loglist/loglist.go b/internal/loglist/loglist.go index b8f33b2..6f030ed 100644 --- a/internal/loglist/loglist.go +++ b/internal/loglist/loglist.go @@ -1,6 +1,7 @@ package loglist import ( + "context" "encoding/json" "fmt" "io" @@ -83,7 +84,7 @@ func (l *Log) Endpoint() string { } } -func FetchAll(client *http.Client) ([]Log, error) { +func FetchAll(ctx context.Context, client *http.Client) ([]Log, error) { if client == nil { client = &http.Client{Timeout: 30 * time.Second} } @@ -92,7 +93,10 @@ func FetchAll(client *http.Client) ([]Log, error) { var order []string var fetched int for _, u := range urls { - list, err := fetchOne(client, u) + if ctx.Err() != nil { + break + } + list, err := fetchOne(ctx, client, u) if err != nil { continue } @@ -125,6 +129,9 @@ func FetchAll(client *http.Client) ([]Log, error) { out = append(out, *seen[id]) } if fetched == 0 { + if ctx.Err() != nil { + return nil, fmt.Errorf("log list fetch canceled: %w", ctx.Err()) + } return nil, fmt.Errorf("all %d log list sources failed", len(urls)) } if fetched < len(urls) { @@ -133,8 +140,12 @@ func FetchAll(client *http.Client) ([]Log, error) { return out, nil } -func fetchOne(client *http.Client, url string) (*List, error) { - resp, err := client.Get(url) +func fetchOne(ctx context.Context, client *http.Client, url string) (*List, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, err + } + resp, err := client.Do(req) if err != nil { return nil, err } diff --git a/internal/loglist/loglist_test.go b/internal/loglist/loglist_test.go new file mode 100644 index 0000000..9c0ac39 --- /dev/null +++ b/internal/loglist/loglist_test.go @@ -0,0 +1,41 @@ +package loglist + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + "time" +) + +func TestFetchOneHonorsContext(t *testing.T) { + release := make(chan struct{}) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-release + w.Write([]byte(`{}`)) + })) + defer func() { + close(release) + srv.Close() + }() + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + start := time.Now() + if _, err := fetchOne(ctx, srv.Client(), srv.URL); err == nil { + t.Fatal("fetchOne with canceled context should fail") + } + if elapsed := time.Since(start); elapsed > time.Second { + t.Fatalf("canceled fetch took %v; want immediate return", elapsed) + } +} + +func TestFetchOneRejectsBadStatus(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "nope", http.StatusInternalServerError) + })) + defer srv.Close() + if _, err := fetchOne(context.Background(), srv.Client(), srv.URL); err == nil { + t.Fatal("fetchOne should fail on 500") + } +} diff --git a/main.go b/main.go index bbc2c1b..5fc3735 100644 --- a/main.go +++ b/main.go @@ -208,7 +208,7 @@ func cmdTail(args []string) error { func runTailers(ctx context.Context, st *store.Store, c *commonConfig, drain bool) { t := &tailer.Tailer{Store: st, Interval: c.interval, Window: c.window, Drain: drain} for { - logs, err := loglist.FetchAll(nil) + logs, err := loglist.FetchAll(ctx, nil) if err == nil { t.Sync(ctx, logs) log.Printf("loglist sync: %d logs known", len(logs))