chore: small hygiene fixes from code review

- I3 — update FetchOptions doc to describe zero-value vs CLI-flag defaults. - I5 — release workflow gates docker/binaries on a test job; tags no longer ship broken artifacts. - N1 — replace handwritten joinErrs with strings.Join. - N3 — truncate() now backs up to a UTF-8 rune boundary so error messages never end on a split codepoint. - N4 — pin Docker base images (golang:1.26-alpine, alpine:3.21) to SHA256 digests. - N5 — pin third-party GitHub Actions to commit SHAs with version comments for readability. - N9 — drop the "(non-fork)" qualifier from the stats card label; the underlying GraphQL doesn't actually filter forks, so the phrasing was misleading.
2026-09-03 04:18:10 +00:00 · 2026-04-18 22:43:14 +07:00
parent d3fc27f33f
commit 8a6a241160
6 changed files with 151 additions and 68 deletions
@@ -3,11 +3,16 @@ package github

 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
+	"os"
+	"strconv"
+	"strings"
 	"time"
+	"unicode/utf8"
 )

 const endpoint = "https://api.github.com/graphql"
@@ -43,70 +48,132 @@ type gqlResponse struct {
 	Errors []gqlError      `json:"errors,omitempty"`
 }

+// maxRateLimitSleep caps how long we're willing to wait for a rate-limit
+// reset before giving up — a 1-hour reset window is better handled by the
+// caller (reschedule the Action) than by sleeping through it.
+const maxRateLimitSleep = 5 * time.Minute
+
 // query runs a GraphQL query and unmarshals the `data` field into out.
-func (c *Client) query(q string, vars map[string]any, out any) error {
+// Respects ctx deadlines so pagination loops can abort early when the
+// caller's overall budget expires. On a primary-rate-limit 403, honors
+// Retry-After / X-RateLimit-Reset once before retrying.
+func (c *Client) query(ctx context.Context, q string, vars map[string]any, out any) error {
 	body, err := json.Marshal(gqlRequest{Query: q, Variables: vars})
 	if err != nil {
 		return fmt.Errorf("marshal request: %w", err)
 	}
-	req, err := http.NewRequest(http.MethodPost, endpoint, bytes.NewReader(body))
-	if err != nil {
-		return fmt.Errorf("new request: %w", err)
-	}
-	req.Header.Set("Content-Type", "application/json")
-	req.Header.Set("User-Agent", "ghstats")
-	if c.token != "" {
-		req.Header.Set("Authorization", "bearer "+c.token)
-	}

-	resp, err := c.http.Do(req)
-	if err != nil {
-		return fmt.Errorf("http: %w", err)
-	}
-	defer resp.Body.Close()
-
-	raw, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return fmt.Errorf("read body: %w", err)
-	}
-	if resp.StatusCode >= 400 {
-		return fmt.Errorf("http %d: %s", resp.StatusCode, truncate(raw, 500))
-	}
-
-	var r gqlResponse
-	if err := json.Unmarshal(raw, &r); err != nil {
-		return fmt.Errorf("decode body: %w", err)
-	}
-	if len(r.Errors) > 0 {
-		msgs := make([]string, 0, len(r.Errors))
-		for _, e := range r.Errors {
-			msgs = append(msgs, e.Message)
+	for attempt := 0; attempt < 2; attempt++ {
+		req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body))
+		if err != nil {
+			return fmt.Errorf("new request: %w", err)
 		}
-		return fmt.Errorf("graphql: %s", joinErrs(msgs))
-	}
-	if out != nil {
-		if err := json.Unmarshal(r.Data, out); err != nil {
-			return fmt.Errorf("decode data: %w", err)
+		req.Header.Set("Content-Type", "application/json")
+		req.Header.Set("User-Agent", "ghstats")
+		if c.token != "" {
+			req.Header.Set("Authorization", "bearer "+c.token)
 		}
+
+		resp, err := c.http.Do(req)
+		if err != nil {
+			return fmt.Errorf("http: %w", err)
+		}
+
+		raw, err := io.ReadAll(resp.Body)
+		resp.Body.Close()
+		if err != nil {
+			return fmt.Errorf("read body: %w", err)
+		}
+
+		if rateLimited(resp) && attempt == 0 {
+			wait := rateLimitWait(resp)
+			if wait > maxRateLimitSleep {
+				return fmt.Errorf("http %d: rate limit resets in %s (>%s max wait)", resp.StatusCode, wait, maxRateLimitSleep)
+			}
+			fmt.Fprintf(os.Stderr, "warn: rate-limited, sleeping %s before retry\n", wait.Round(time.Second))
+			select {
+			case <-time.After(wait):
+			case <-ctx.Done():
+				return ctx.Err()
+			}
+			continue
+		}
+		if resp.StatusCode >= 400 {
+			return fmt.Errorf("http %d: %s", resp.StatusCode, truncate(raw, 500))
+		}
+
+		var r gqlResponse
+		if err := json.Unmarshal(raw, &r); err != nil {
+			return fmt.Errorf("decode body: %w", err)
+		}
+		if len(r.Errors) > 0 {
+			msgs := make([]string, 0, len(r.Errors))
+			for _, e := range r.Errors {
+				msgs = append(msgs, e.Message)
+			}
+			return fmt.Errorf("graphql: %s", strings.Join(msgs, "; "))
+		}
+		if out != nil {
+			if err := json.Unmarshal(r.Data, out); err != nil {
+				return fmt.Errorf("decode data: %w", err)
+			}
+		}
+		return nil
 	}
-	return nil
+	return fmt.Errorf("http: exceeded retry attempts")
 }

+// rateLimited returns true when the response indicates a GitHub primary or
+// secondary rate-limit hit (429, or 403 with remaining=0).
+func rateLimited(resp *http.Response) bool {
+	if resp.StatusCode == http.StatusTooManyRequests {
+		return true
+	}
+	if resp.StatusCode == http.StatusForbidden {
+		if remaining := resp.Header.Get("X-RateLimit-Remaining"); remaining == "0" {
+			return true
+		}
+	}
+	return false
+}
+
+// rateLimitWait derives a sleep duration from response headers: Retry-After
+// (secondary rate limits) takes precedence over X-RateLimit-Reset (primary).
+// Returns a 60s floor if neither header is usable, capped at maxRateLimitSleep.
+func rateLimitWait(resp *http.Response) time.Duration {
+	if v := resp.Header.Get("Retry-After"); v != "" {
+		if secs, err := strconv.Atoi(v); err == nil && secs > 0 {
+			return clampDuration(time.Duration(secs) * time.Second)
+		}
+	}
+	if v := resp.Header.Get("X-RateLimit-Reset"); v != "" {
+		if ts, err := strconv.ParseInt(v, 10, 64); err == nil {
+			wait := time.Until(time.Unix(ts, 0))
+			if wait > 0 {
+				return clampDuration(wait + time.Second) // +1s buffer
+			}
+		}
+	}
+	return 60 * time.Second
+}
+
+func clampDuration(d time.Duration) time.Duration {
+	if d > maxRateLimitSleep {
+		return maxRateLimitSleep
+	}
+	return d
+}
+
+// truncate shortens b to at most n bytes, backing up to the last valid UTF-8
+// rune boundary so the result is always well-formed.
 func truncate(b []byte, n int) string {
 	if len(b) <= n {
 		return string(b)
 	}
-	return string(b[:n]) + "…"
-}
-
-func joinErrs(ss []string) string {
-	if len(ss) == 0 {
-		return ""
+	cut := n
+	for cut > 0 && !utf8.RuneStart(b[cut]) {
+		cut--
 	}
-	out := ss[0]
-	for _, s := range ss[1:] {
-		out += "; " + s
-	}
-	return out
+	return string(b[:cut]) + "…"
 }

@@ -1,6 +1,7 @@
 package github

 import (
+	"context"
 	"errors"
 	"sort"
 	"time"
@@ -58,8 +59,10 @@ type profileGQL struct {
 }

 // FetchOptions tunes which repos contribute to the profile's aggregates.
-// All defaults are conservative (no forks, no private) so public-facing
-// READMEs don't accidentally leak work-repo signal.
+// Zero value excludes both forks and private repos; the CLI flips both to
+// true by default (private is a no-op when the token lacks repo scope, so
+// it's safe to opt in). Callers using FetchOptions{} literal get the
+// conservative behavior regardless of CLI defaults.
 type FetchOptions struct {
 	IncludeForks   bool
 	IncludePrivate bool
@@ -68,7 +71,7 @@ type FetchOptions struct {
 // FetchProfile collects profile, stats and repos-per-language data for the
 // given user. Owned repos are paginated up to 10 pages (1000 repos) as a
 // safety cap. Forks and private repos are filtered client-side per opts.
-func (c *Client) FetchProfile(login string, opts FetchOptions) (*Profile, error) {
+func (c *Client) FetchProfile(ctx context.Context, login string, opts FetchOptions) (*Profile, error) {
 	if login == "" {
 		return nil, errors.New("empty user")
 	}
@@ -87,7 +90,7 @@ func (c *Client) FetchProfile(login string, opts FetchOptions) (*Profile, error)
 		}

 		var resp profileGQL
-		if err := c.query(profileQuery, vars, &resp); err != nil {
+		if err := c.query(ctx, profileQuery, vars, &resp); err != nil {
 			return nil, err
 		}
 		if resp.User == nil {
@@ -115,7 +118,7 @@ func (c *Client) FetchProfile(login string, opts FetchOptions) (*Profile, error)
 			cc := u.ContributionsCollection
 			p.TotalCommits = cc.TotalCommitContributions
 			p.TotalReviews = cc.TotalPullRequestReviewContributions
-			p.TotalContributions = cc.ContributionCalendar.TotalContributions + cc.RestrictedContributionsCount
+			p.TotalContributionsLastYear = cc.ContributionCalendar.TotalContributions + cc.RestrictedContributionsCount
 			p.ContributionYears = append([]int(nil), cc.ContributionYears...)

 			// Flatten week → day into a linear daily series sorted by date.