Commit d4ea40c36c
d4ea40c36cabc17bcc6881eaef769bffba821e97
parent: 03e34fb24d
Verified · cmc
cmc <hello@cleberg.net> · 2026-09-29 16:45 UTC
httpd: a command a limiter turned away is 503 with Retry-After on the API
Ref #308
Layout: unified · split
internal/control/control.go
+3
| @@ -71,6 +71,9 @@ type Ctx struct { |
| 71 | // Packs is the pack-generation limiter a command that runs git to |
71 | // Packs is the pack-generation limiter a command that runs git to |
| 72 | // produce an archive takes a slot from; nil is no limit. |
72 | // produce an archive takes a slot from; nil is no limit. |
| 73 | Packs *packlimit.Limiter |
73 | Packs *packlimit.Limiter |
| |
74 | // Busy is set when a limiter turned the command away, so the API |
| |
75 | // can answer 503 with Retry-After rather than a failure. |
| |
76 | Busy bool |
| 74 | } |
77 | } |
| 75 | |
78 | |
| 76 | // SourceWeb is Ctx.Source for a request from a browser session. Its |
79 | // SourceWeb is Ctx.Source for a request from a browser session. Its |
internal/control/explore.go
+1
| @@ -130,6 +130,7 @@ func runRepoDownload(c *Ctx, args []string) int { |
| 130 | transport = "api" |
130 | transport = "api" |
| 131 | } |
131 | } |
| 132 | c.Packs.Refused(transport, principal, err) |
132 | c.Packs.Refused(transport, principal, err) |
| |
133 | c.Busy = true |
| 133 | if errors.Is(err, packlimit.ErrBusy) { |
134 | if errors.Is(err, packlimit.ErrBusy) { |
| 134 | return c.fail(protocol.ExitFailure, "the server is busy: it is at its limit of concurrent clones and fetches; try again in a minute") |
135 | return c.fail(protocol.ExitFailure, "the server is busy: it is at its limit of concurrent clones and fetches; try again in a minute") |
| 135 | } |
136 | } |
internal/httpd/api.go
+8
| @@ -97,12 +97,20 @@ func (s *Server) apiCmd(w http.ResponseWriter, r *http.Request) { |
| 97 | body["stderr"] = msg |
97 | body["stderr"] = msg |
| 98 | } |
98 | } |
| 99 | w.Header().Set("Content-Type", "application/json") |
99 | w.Header().Set("Content-Type", "application/json") |
| |
100 | if ctx.Busy { |
| |
101 | status = http.StatusServiceUnavailable |
| |
102 | w.Header().Set("Retry-After", busyRetryAfter) |
| |
103 | } |
| 100 | w.WriteHeader(status) |
104 | w.WriteHeader(status) |
| 101 | json.NewEncoder(w).Encode(body) |
105 | json.NewEncoder(w).Encode(body) |
| 102 | } |
106 | } |
| 103 | |
107 | |
| 104 | // statusForExit maps a command's exit code onto an HTTP status, shared by |
108 | // statusForExit maps a command's exit code onto an HTTP status, shared by |
| 105 | // both API surfaces so they cannot answer the same failure differently. |
109 | // both API surfaces so they cannot answer the same failure differently. |
| |
110 | // busyRetryAfter is the Retry-After on a 503 for a command a limiter |
| |
111 | // turned away. |
| |
112 | const busyRetryAfter = "60" |
| |
113 | |
| 106 | func statusForExit(code int) int { |
114 | func statusForExit(code int) int { |
| 107 | switch code { |
115 | switch code { |
| 108 | case protocol.ExitOK: |
116 | case protocol.ExitOK: |
internal/httpd/apibusy_test.go
added
+52
| @@ -0,0 +1,52 @@ |
| |
1 | package httpd |
| |
2 | |
| |
3 | import ( |
| |
4 | "net/http" |
| |
5 | "net/http/httptest" |
| |
6 | "strings" |
| |
7 | "testing" |
| |
8 | |
| |
9 | "gitbay.org/gitbay/internal/control" |
| |
10 | "gitbay.org/gitbay/internal/gitutil" |
| |
11 | "gitbay.org/gitbay/internal/store" |
| |
12 | ) |
| |
13 | |
| |
14 | // repo download turned away by a full pack limit is 503 with |
| |
15 | // Retry-After on both API endpoints, not a 500. |
| |
16 | func TestAPIBusyDownloadIs503(t *testing.T) { |
| |
17 | s := busyServer(t) |
| |
18 | s.apiLimit = newAPILimiter(0) |
| |
19 | if err := gitutil.InitBare(control.RepoDir(s.cfg.Server.Root, "alice", "app"), "main", t.TempDir()); err != nil { |
| |
20 | t.Fatal(err) |
| |
21 | } |
| |
22 | seed(t, s, 10) |
| |
23 | alice, err := s.st.UserByUsername("alice") |
| |
24 | if err != nil { |
| |
25 | t.Fatal(err) |
| |
26 | } |
| |
27 | if err := s.st.CreateAPIToken(alice.ID, "t", store.HashToken("secret"), "full", nil, 0); err != nil { |
| |
28 | t.Fatal(err) |
| |
29 | } |
| |
30 | check := func(what string, w *httptest.ResponseRecorder) { |
| |
31 | t.Helper() |
| |
32 | if w.Code != http.StatusServiceUnavailable || w.Header().Get("Retry-After") != "60" || |
| |
33 | !strings.Contains(w.Body.String(), "busy") { |
| |
34 | t.Fatalf("%s: status %d, Retry-After %q, body %s", what, w.Code, w.Header().Get("Retry-After"), w.Body.String()) |
| |
35 | } |
| |
36 | if w.Header().Get("ETag") != "" { |
| |
37 | t.Fatalf("%s: a busy answer carries an ETag", what) |
| |
38 | } |
| |
39 | } |
| |
40 | |
| |
41 | r := httptest.NewRequest("POST", "/api/v1/cmd", strings.NewReader(`{"argv":["repo","download","alice/app"]}`)) |
| |
42 | r.Header.Set("Authorization", "Bearer secret") |
| |
43 | w := httptest.NewRecorder() |
| |
44 | s.apiCmd(w, r) |
| |
45 | check("POST", w) |
| |
46 | |
| |
47 | r = httptest.NewRequest("GET", "/api/v1/read?argv=repo&argv=download&argv=alice/app", nil) |
| |
48 | r.Header.Set("Authorization", "Bearer secret") |
| |
49 | w = httptest.NewRecorder() |
| |
50 | s.apiRead(w, r) |
| |
51 | check("GET", w) |
| |
52 | } |
internal/httpd/apiread.go
+9
| @@ -82,6 +82,15 @@ func (s *Server) apiRead(w http.ResponseWriter, r *http.Request) { |
| 82 | return |
82 | return |
| 83 | } |
83 | } |
| 84 | |
84 | |
| |
85 | if ctx.Busy { |
| |
86 | // Not cacheable: the next try may succeed. |
| |
87 | w.Header().Set("Content-Type", "application/json") |
| |
88 | w.Header().Set("Retry-After", busyRetryAfter) |
| |
89 | w.WriteHeader(http.StatusServiceUnavailable) |
| |
90 | w.Write(payload) |
| |
91 | return |
| |
92 | } |
| |
93 | |
| 85 | // Responses are authorized per account, so the ETag is salted with the |
94 | // Responses are authorized per account, so the ETag is salted with the |
| 86 | // caller: two users asking the same question may get different answers, |
95 | // caller: two users asking the same question may get different answers, |
| 87 | // and neither should ever be served the other's. |
96 | // and neither should ever be served the other's. |