engine: honour Retry-After when retrying a rate-limited module fetch

FIDELITY.md

@@ -38,6 +38,15 @@ What limits the network is `maxPerHostConcurrency` (`transport.go`): every Tailw
 - **Correction (round 153)**: the 429 comparison above used two runs at cap 2 (0 429s) and two at cap 4 (3 each). Eight runs at the baseline cap of 2 produced 429s in six of them (3–5 each), so Wikimedia's 429s are not a function of this cap, and the cap-4 "regression" was a sampling artifact. The cap is not the lever for Wikimedia's limit. Its current baseline rate needs a separate investigation (retry/backoff behaviour, Retry-After handling, and whether the retried images still render).
 - **Adaptive limiter tried in round 153, not shipped**: start at 2, grow to 4 after a run of clean responses, drop back on a 429. Growing after 10 clean responses still produced 429s (5 in one of three runs); growing after 40 never grows on a single Wikipedia page, so it cannot help there, and its Tailwind gain was within noise. Parked, not merged.
 
+## 2026-10-04 (round 160) — module fetches ignored a rate-limited origin's Retry-After and retried inside its window, which could drop a module and break the page (engine#260, patch)
+
+A survey of how other renderers accelerate loading recommended honouring Retry-After in module fetches. Checked against the code before acting, because the survey also claimed that no Retry-After parsing existed and that image fetches had no 429 retry. Both claims were wrong: `doWithRateLimitRetry` and `retryAfterDelay` in `images.go` already honour Retry-After for images and fonts (three retries). The real gap was narrower: `fetchModuleSource` in `modules.go` retried after a fixed 150ms then 300ms, ignoring the header.
+
+- **Why it matters beyond latency**: a module refused with `Retry-After: 1` was retried 150ms later, drew another 429, and was dropped after the attempts ran out. A dropped module is a broken page, not a slow one.
+- **Fixed**: `fetchModuleOnce` returns the Retry-After delay for a 429, and `fetchModuleSource` waits that long when present, keeping the fixed backoff otherwise. The same helper as the image path is reused.
+- **Test**: `TestFetchModuleSourceHonoursRetryAfter` serves 429 with `Retry-After: 1` for the first second. With the fix it succeeds after about 1.0s. Stash-verified: the old code drops the module at 0.46s.
+- **Corpus effect**: none measured. The saved Tailwind, Wikipedia and Hacker News renders are pixel-identical before and after, because no module on those pages was rate-limited. The change corrects a case the corpus does not exercise.
+
 ## 2026-10-04 (round 158) — the settle loop is a small lever: about 0.26s of CPU on tailwindcss.com, all of it full re-cascades, bounded by the network wait (docs only, no behaviour change)
 
 Third candidate after the cascade and the image path: the settle-then-render loop. Profiling one Tailwind render shows `settle` at about 12% of CPU samples, and of that the cascade (`CascadeMediaContainers`) is almost all of it, about 0.26s. JavaScript execution is about 0.07s. A render performs five full cascades. Each is a whole-document pass.

modules.go

@@ -304,48 +304,56 @@ func (e *Engine) fetchModuleSource(ctx context.Context, abs string) (string, boo
 		return "", false
 	}
 	for attempt := 0; ; attempt++ {
-		src, retry, ok := e.fetchModuleOnce(ctx, abs)
+		src, retry, ok, after := e.fetchModuleOnce(ctx, abs)
 		if ok {
 			return src, true
 		}
 		if !retry || attempt >= moduleFetchRetries || ctx.Err() != nil {
 			return "", false
 		}
-		// Small backoff to let a rate-limited origin recover, bounded by ctx.
+		// A rate-limited origin says how long to wait (Retry-After); retrying
+		// sooner just draws another 429. Otherwise a small backoff bounded by ctx.
+		wait := after
+		if wait == 0 {
+			wait = time.Duration(attempt+1) * 150 * time.Millisecond
+		}
 		select {
 		case <-ctx.Done():
 			return "", false
-		case <-time.After(time.Duration(attempt+1) * 150 * time.Millisecond):
+		case <-time.After(wait):
 		}
 	}
 }
 
 // fetchModuleOnce does a single fetch attempt. retry reports whether the failure
 // looks transient (worth another attempt): a network error or a 429/5xx.
-func (e *Engine) fetchModuleOnce(ctx context.Context, abs string) (body string, retry, ok bool) {
+func (e *Engine) fetchModuleOnce(ctx context.Context, abs string) (body string, retry, ok bool, after time.Duration) {
 	rctx, cancel := context.WithTimeout(ctx, moduleFetchTimeout)
 	defer cancel()
 	req, err := http.NewRequestWithContext(rctx, http.MethodGet, abs, nil)
 	if err != nil {
-		return "", false, false
+		return "", false, false, 0
 	}
 	if e.UserAgent != "" {
 		req.Header.Set("User-Agent", e.UserAgent)
 	}
 	resp, err := e.Client.Do(req)
 	if err != nil {
-		return "", true, false // network/timeout: transient
+		return "", true, false, 0 // network/timeout: transient
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		transient := resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode >= 500
-		return "", transient, false
+		if resp.StatusCode == http.StatusTooManyRequests && resp.Header.Get("Retry-After") != "" {
+			after = retryAfterDelay(resp.Header.Get("Retry-After"))
+		}
+		return "", transient, false, after
 	}
 	raw, err := io.ReadAll(io.LimitReader(resp.Body, maxModuleSourceBytes))
 	if err != nil {
-		return "", true, false
+		return "", true, false, 0
 	}
-	return string(raw), false, true
+	return string(raw), false, true, 0
 }
 
 // awaitBounded runs fn in its own goroutine and returns its result with ok

modules_retryafter_test.go

@@ -0,0 +1,45 @@
+// Copyright (c) the go-webengine/engine authors.
+// SPDX-License-Identifier: BSD-3-Clause
+
+package engine
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"sync"
+	"testing"
+	"time"
+)
+
+// A module answered 429 with Retry-After: 1 must be fetched again only after
+// that second has passed. The fixed 150ms/300ms backoff retried inside the
+// window, drew another 429, ran out of attempts and dropped the module.
+func TestFetchModuleSourceHonoursRetryAfter(t *testing.T) {
+	var mu sync.Mutex
+	var first time.Time
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		mu.Lock()
+		defer mu.Unlock()
+		if first.IsZero() {
+			first = time.Now()
+		}
+		if time.Since(first) < time.Second {
+			w.Header().Set("Retry-After", "1")
+			w.WriteHeader(http.StatusTooManyRequests)
+			return
+		}
+		_, _ = w.Write([]byte("export const x = 1;"))
+	}))
+	defer srv.Close()
+	e := New()
+	start := time.Now()
+	src, ok := e.fetchModuleSource(context.Background(), srv.URL+"/chunk.js")
+	elapsed := time.Since(start)
+	if !ok || src != "export const x = 1;" {
+		t.Fatalf("module not fetched: ok=%v src=%q", ok, src)
+	}
+	if elapsed < 900*time.Millisecond {
+		t.Errorf("retried after %v, want to wait for Retry-After (about 1s)", elapsed)
+	}
+}