diff options
Diffstat (limited to 'internal/api')
| -rw-r--r-- | internal/api/context_audit_test.go | 87 | ||||
| -rw-r--r-- | internal/api/google_calendar.go | 65 |
2 files changed, 138 insertions, 14 deletions
diff --git a/internal/api/context_audit_test.go b/internal/api/context_audit_test.go new file mode 100644 index 0000000..da073f5 --- /dev/null +++ b/internal/api/context_audit_test.go @@ -0,0 +1,87 @@ +package api + +import ( + "go/ast" + "go/parser" + "go/token" + "path/filepath" + "strings" + "testing" +) + +// TestGoogleAPICallsHaveContext is a static regression guard for the +// 2026-08-04 incident: three .Do() calls in google_calendar.go accepted a +// ctx parameter but never chained .Context(ctx) into the actual Google API +// SDK builder, so the caller's timeout/cancellation silently never reached +// the network call -- one hung call wedged the production server for three +// days. Go's compiler allows this (both Context() and Do() are optional +// builder methods on the generated SDK types), so this walks the AST of +// every google_*.go file instead and fails if any .Do() call's method +// chain doesn't include a .Context(...) call anywhere in it. +// +// Scoped to files named google_*.go, not the whole package: internal/api's +// hand-rolled BaseClient (http.go) bakes ctx into the request object itself +// via http.NewRequestWithContext before calling HTTPClient.Do(req), which +// is a structurally different (and already-safe) pattern that this check +// would false-positive on if it ran there too. +func TestGoogleAPICallsHaveContext(t *testing.T) { + files, err := filepath.Glob("google_*.go") + if err != nil { + t.Fatalf("glob: %v", err) + } + + fset := token.NewFileSet() + found := 0 + for _, file := range files { + if strings.HasSuffix(file, "_test.go") { + continue + } + f, err := parser.ParseFile(fset, file, nil, 0) + if err != nil { + t.Fatalf("parse %s: %v", file, err) + } + ast.Inspect(f, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Do" { + return true + } + found++ + if !chainHasContext(sel.X) { + pos := fset.Position(call.Pos()) + t.Errorf("%s:%d: .Do() call has no .Context(ctx) anywhere in its method chain -- "+ + "cancellation and timeouts set by the caller will be silently ignored "+ + "(this is exactly the 2026-08-04 incident's root cause)", pos.Filename, pos.Line) + } + return true + }) + } + + if found == 0 { + t.Fatal("found zero .Do() calls in google_*.go -- this check may have stopped matching " + + "the code it's meant to guard (file renamed? SDK call style changed?), which would " + + "make it silently pass without checking anything") + } +} + +// chainHasContext walks backward through a method-chain expression (e.g. the +// receiver of a .Do() call) looking for a .Context(...) call anywhere in it. +func chainHasContext(expr ast.Expr) bool { + for { + call, ok := expr.(*ast.CallExpr) + if !ok { + return false + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + if sel.Sel.Name == "Context" { + return true + } + expr = sel.X + } +} diff --git a/internal/api/google_calendar.go b/internal/api/google_calendar.go index f441a45..44457bf 100644 --- a/internal/api/google_calendar.go +++ b/internal/api/google_calendar.go @@ -6,6 +6,7 @@ import ( "log" "sort" "strings" + "sync" "time" "task-dashboard/internal/models" @@ -117,19 +118,19 @@ func NewGoogleCalendarClient(ctx context.Context, credentialsFile, calendarIDs, func (c *GoogleCalendarClient) GetUpcomingEvents(ctx context.Context, maxResults int) ([]models.CalendarEvent, error) { t := time.Now().Format(time.RFC3339) - var allEvents []models.CalendarEvent - for _, calendarID := range c.calendarIDs { + fetchOne := func(calendarID string) []models.CalendarEvent { events, err := c.srv.Events.List(calendarID).ShowDeleted(false). - SingleEvents(true).TimeMin(t).MaxResults(int64(maxResults)).OrderBy("startTime").Do() + SingleEvents(true).TimeMin(t).MaxResults(int64(maxResults)).OrderBy("startTime"). + Context(ctx).Do() if err != nil { log.Printf("Warning: failed to fetch events from calendar %s: %v", calendarID, err) - continue + return nil } - + parsed := make([]models.CalendarEvent, 0, len(events.Items)) for _, item := range events.Items { start, end := c.parseEventTime(item) - allEvents = append(allEvents, models.CalendarEvent{ + parsed = append(parsed, models.CalendarEvent{ ID: item.Id, Summary: item.Summary, Description: item.Description, @@ -139,8 +140,11 @@ func (c *GoogleCalendarClient) GetUpcomingEvents(ctx context.Context, maxResults RecurringEventID: item.RecurringEventId, }) } + return parsed } + allEvents := c.fetchAllCalendars(fetchOne) + uniqueEvents := deduplicateEvents(allEvents) if len(uniqueEvents) > maxResults { uniqueEvents = uniqueEvents[:maxResults] @@ -148,22 +152,54 @@ func (c *GoogleCalendarClient) GetUpcomingEvents(ctx context.Context, maxResults return uniqueEvents, nil } +// fetchAllCalendars runs fetchOne for every configured calendar concurrently +// and merges the results. This must be concurrent, not sequential: every +// caller shares a single deadline-bound ctx (see aggregateData's per-fetch +// context.WithTimeout in internal/handlers/handlers.go), and a sequential +// loop would let an early calendar's slow response starve the deadline +// budget for calendars later in the list -- exactly the failure mode this +// function exists to prevent (see incident notes 2026-08-04). Running them +// concurrently means every calendar gets an equal shot at the same shared +// deadline instead of eating into the next one's share of it. +func (c *GoogleCalendarClient) fetchAllCalendars(fetchOne func(calendarID string) []models.CalendarEvent) []models.CalendarEvent { + var wg sync.WaitGroup + var mu sync.Mutex + var allEvents []models.CalendarEvent + + for _, calendarID := range c.calendarIDs { + wg.Add(1) + go func(calendarID string) { + defer wg.Done() + events := fetchOne(calendarID) + if len(events) == 0 { + return + } + mu.Lock() + allEvents = append(allEvents, events...) + mu.Unlock() + }(calendarID) + } + wg.Wait() + + return allEvents +} + func (c *GoogleCalendarClient) GetEventsByDateRange(ctx context.Context, start, end time.Time) ([]models.CalendarEvent, error) { timeMin := start.Format(time.RFC3339) timeMax := end.Format(time.RFC3339) - var allEvents []models.CalendarEvent - for _, calendarID := range c.calendarIDs { + fetchOne := func(calendarID string) []models.CalendarEvent { events, err := c.srv.Events.List(calendarID).ShowDeleted(false). - SingleEvents(true).TimeMin(timeMin).TimeMax(timeMax).OrderBy("startTime").Do() + SingleEvents(true).TimeMin(timeMin).TimeMax(timeMax).OrderBy("startTime"). + Context(ctx).Do() if err != nil { log.Printf("Warning: failed to fetch events from calendar %s: %v", calendarID, err) - continue + return nil } - + parsed := make([]models.CalendarEvent, 0, len(events.Items)) for _, item := range events.Items { evtStart, evtEnd := c.parseEventTime(item) - allEvents = append(allEvents, models.CalendarEvent{ + parsed = append(parsed, models.CalendarEvent{ ID: item.Id, Summary: item.Summary, Description: item.Description, @@ -173,9 +209,10 @@ func (c *GoogleCalendarClient) GetEventsByDateRange(ctx context.Context, start, RecurringEventID: item.RecurringEventId, }) } + return parsed } - return deduplicateEvents(allEvents), nil + return deduplicateEvents(c.fetchAllCalendars(fetchOne)), nil } // SetCalendarIDs updates the calendar IDs used by the client @@ -185,7 +222,7 @@ func (c *GoogleCalendarClient) SetCalendarIDs(ids []string) { // GetCalendarList returns all calendars accessible to the user func (c *GoogleCalendarClient) GetCalendarList(ctx context.Context) ([]models.CalendarInfo, error) { - list, err := c.srv.CalendarList.List().Do() + list, err := c.srv.CalendarList.List().Context(ctx).Do() if err != nil { return nil, fmt.Errorf("failed to fetch calendar list: %w", err) } |