frontend: fetch City() concurrently with the page's primary API call
Some checks failed
shell-lint / shellcheck (pull_request) Failing after 10m43s
secrets-guard / encrypted (pull_request) Failing after 10m45s
PR build (required check) / changes (pull_request) Failing after 10m47s
PR build (required check) / build-backend (pull_request) Has been cancelled
PR build (required check) / build-frontend (pull_request) Has been cancelled
PR build (required check) / validate-observability (pull_request) Has been cancelled
PR build (required check) / gate (pull_request) Has been cancelled

MonthPage and RecordsPage each made two backend calls sequentially
(CityMonth/CityRecords, then City) where the second never depended on the
first's result -- both are independent single-slug lookups. Waiting for
one full round trip before even starting the other was pure latency with
nothing to show for it.

fetchWithCity launches both concurrently via goroutines and a WaitGroup.
Verified live against a stub with an injected 400ms delay on both
endpoints: city page (1 call) and month/records pages (2 calls each) all
cost ~0.404s now, not ~0.404s vs ~0.8s -- the two-call pages no longer pay
double.

Error priority is preserved exactly: if the primary call fails, its error
wins even when City also fails or hasn't finished, matching the old
sequential code (which never called City() once primary had already
failed). The one real trade-off, called out in the comment: City() is now
always launched even on a request that's about to 404 from primary, so an
invalid slug costs one extra (wasted, cheap) backend lookup it previously
skipped -- worth it since a bad slug is the rare path and a good one is
the common path this speeds up.

Tests: happy path; primary-error-wins and city-error-surfaces (both single
and combined failure); and a deterministic concurrency proof via
rendezvous channels rather than timing (the old sequential code would
deadlock this test, not just run it slower). Full suite green under
`-race -count=2`. Docker build (which runs `go test` inside the image)
passes.
This commit is contained in:
Emi Griffith 2026-07-23 23:04:17 -07:00
parent cf0fa01892
commit 2f0bda1e28
2 changed files with 164 additions and 14 deletions

View file

@ -19,6 +19,7 @@ import (
"path/filepath"
"strings"
"testing"
"time"
"thermograph/frontend/internal/config"
"thermograph/frontend/internal/contentapi"
@ -474,6 +475,116 @@ func TestMonthCtx(t *testing.T) {
}
}
// --- fetchWithCity -----------------------------------------------------------
func TestFetchWithCityHappyPath(t *testing.T) {
api := &fakeAPI{
city: func(slug, _ string) (*contentapi.CityPayload, error) {
return &contentapi.CityPayload{City: contentapi.CityInfo{Slug: slug, Name: "Testville"}}, nil
},
}
primary, city, err := fetchWithCity(api, "testville", func() (string, error) { return "primary-value", nil })
if err != nil {
t.Fatal(err)
}
if primary != "primary-value" {
t.Errorf("primary = %q", primary)
}
if city.Slug != "testville" || city.Name != "Testville" {
t.Errorf("city = %+v", city)
}
}
// When only primary fails, its error must win even though City() also ran
// (and, here, succeeded) -- matching the old sequential code's behavior,
// where City() was never even called once primary had already failed.
func TestFetchWithCityPrimaryErrorWins(t *testing.T) {
primaryErr := notFound("no such month")
api := &fakeAPI{
city: func(slug, _ string) (*contentapi.CityPayload, error) {
return &contentapi.CityPayload{City: contentapi.CityInfo{Slug: slug}}, nil
},
}
_, _, err := fetchWithCity(api, "testville", func() (string, error) { return "", primaryErr })
if err != primaryErr {
t.Errorf("err = %v, want the primary error", err)
}
}
// When primary succeeds but City() fails, City's error must surface -- same
// as the old sequential code (it called City() second and returned whatever
// that call did).
func TestFetchWithCityCityErrorSurfaces(t *testing.T) {
cityErr := notFound("no such city")
api := &fakeAPI{
city: func(_, _ string) (*contentapi.CityPayload, error) { return nil, cityErr },
}
_, _, err := fetchWithCity(api, "testville", func() (string, error) { return "primary-value", nil })
if err != cityErr {
t.Errorf("err = %v, want the city error", err)
}
}
// When BOTH fail, primary's error still wins -- the same priority as the
// single-failure case above, just with both goroutines erroring.
func TestFetchWithCityBothErrorPrimaryWins(t *testing.T) {
primaryErr, cityErr := notFound("primary"), notFound("city")
api := &fakeAPI{
city: func(_, _ string) (*contentapi.CityPayload, error) { return nil, cityErr },
}
_, _, err := fetchWithCity(api, "testville", func() (string, error) { return "", primaryErr })
if err != primaryErr {
t.Errorf("err = %v, want the primary error (city error must not win)", err)
}
}
// Proves the two calls actually run CONCURRENTLY rather than sequentially --
// deterministically, via a rendezvous, not by timing (which would be flaky
// under CI load). Each fake blocks until BOTH have started; if fetchWithCity
// ran them sequentially, the second would never start until the first
// returned, and this would deadlock and fail on the test's own timeout.
func TestFetchWithCityRunsConcurrently(t *testing.T) {
started := make(chan struct{}, 2)
release := make(chan struct{})
rendezvous := func() {
started <- struct{}{}
<-release
}
api := &fakeAPI{
city: func(_, _ string) (*contentapi.CityPayload, error) {
rendezvous()
return &contentapi.CityPayload{}, nil
},
}
done := make(chan struct{})
go func() {
fetchWithCity(api, "testville", func() (string, error) {
rendezvous()
return "", nil
})
close(done)
}()
// Both goroutines must reach the rendezvous before either can proceed --
// only possible if they were launched concurrently.
for range 2 {
select {
case <-started:
case <-time.After(2 * time.Second):
t.Fatal("timed out waiting for both calls to start -- they are not running concurrently")
}
}
close(release)
select {
case <-done:
case <-time.After(2 * time.Second):
t.Fatal("fetchWithCity did not return after release")
}
}
func TestMonthCtxUnknownLabelIsError(t *testing.T) {
h := newHandlers(t, fixtureAPI(t))
j := loadFixture[contentapi.MonthPayload](t, "month")

View file

@ -7,6 +7,7 @@ import (
"html/template"
"net/http"
"strings"
"sync"
"thermograph/frontend/internal/contentapi"
"thermograph/frontend/internal/contentdata"
@ -165,6 +166,49 @@ func (h *Handlers) cityCtx(j *contentapi.CityPayload) (map[string]any, format.Un
return ctx, unit, nil
}
// fetchWithCity runs a page's primary API call and the shared City lookup
// concurrently instead of sequentially: both are independent single-slug
// fetches (City never depends on primary's result), so waiting for one to
// fully round-trip before even starting the other was pure latency with
// nothing to show for it. Concurrent instead cuts that leg to roughly
// whichever call is slower.
//
// If primary fails, its error wins even when City also fails or hasn't
// finished — matching the old sequential code's behavior exactly (it never
// even called City() once primary had already failed). The one real
// trade-off: City() is now ALWAYS launched, even on a request that's about
// to 404 from primary, so an invalid slug costs one extra (wasted, cheap)
// backend lookup it previously skipped. Worth it: a bad slug is the rare
// path; a good one is the common path this speeds up.
func fetchWithCity[T any](api API, slug string, primary func() (T, error)) (T, contentapi.CityInfo, error) {
var (
pVal T
pErr error
cPay *contentapi.CityPayload
cErr error
group sync.WaitGroup
)
group.Add(2)
go func() {
defer group.Done()
pVal, pErr = primary()
}()
go func() {
defer group.Done()
cPay, cErr = api.City(slug, "") // no origin: the Python called city(slug) bare here too
}()
group.Wait()
if pErr != nil {
return pVal, contentapi.CityInfo{}, pErr
}
if cErr != nil {
var zero T
return zero, contentapi.CityInfo{}, cErr
}
return pVal, cPay.City, nil
}
// --- month page --------------------------------------------------------------
// MonthPage renders /climate/{slug}/{month} (content.py's month_page).
@ -176,17 +220,14 @@ func (h *Handlers) cityCtx(j *contentapi.CityPayload) (map[string]any, format.Un
// Jinja iterated (label, value) tuples).
func (h *Handlers) MonthPage(w http.ResponseWriter, r *http.Request) {
slug, month := r.PathValue("slug"), r.PathValue("month")
j, err := h.api.CityMonth(slug, month)
j, city, err := fetchWithCity(h.api, slug, func() (*contentapi.MonthPayload, error) {
return h.api.CityMonth(slug, month)
})
if err != nil {
h.apiError(w, err)
return
}
cp, err := h.api.City(slug, "") // no origin: the Python called city(slug) bare here
if err != nil {
h.apiError(w, err)
return
}
ctx, unit, err := h.monthCtx(j, cp.City)
ctx, unit, err := h.monthCtx(j, city)
if err != nil {
h.serverError(w, err)
return
@ -308,17 +349,15 @@ func fmtPeriodSide(unit format.Unit, s contentapi.PeriodSide) map[string]any {
// temperature metrics, nil otherwise, as the Python set them.
func (h *Handlers) RecordsPage(w http.ResponseWriter, r *http.Request) {
slug := r.PathValue("slug")
j, err := h.api.CityRecords(slug, origin(r))
o := origin(r)
j, city, err := fetchWithCity(h.api, slug, func() (*contentapi.RecordsPayload, error) {
return h.api.CityRecords(slug, o)
})
if err != nil {
h.apiError(w, err)
return
}
cp, err := h.api.City(slug, "") // no origin, same as month_page
if err != nil {
h.apiError(w, err)
return
}
ctx, unit, err := h.recordsCtx(j, cp.City)
ctx, unit, err := h.recordsCtx(j, city)
if err != nil {
h.serverError(w, err)
return