Merge master: add per-package HTTP clients, status code check, tests

rusq · rusq · commit 44a1655e1e89 · 2026-03-19T18:34:36.000+10:00
diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
@@ -14,10 +14,10 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v6
 
     - name: Set up Go
-      uses: actions/setup-go@v5
+      uses: actions/setup-go@v6
       with:
         go-version: 1.25
 
diff --git a/.gitignore b/.gitignore
@@ -2,6 +2,7 @@ aklrubbish
 *~
 .vscode
 .idea
+.env
 
 # binary files
 aklapi
diff --git a/AGENTS.md b/AGENTS.md
@@ -9,7 +9,7 @@ This document provides guidance for agentic coding assistants operating in this
 `aklapi` is a Go library and HTTP server that exposes Auckland Council APIs
 (rubbish collection schedules, property address lookup) as a simple REST service.
 
-- **Module:** `github.com/rusq/aklapi` (`go 1.24`)
+- **Module:** `github.com/rusq/aklapi` (`go 1.25`)
 - **Library package:** root (`aklapi`)
 - **Binary:** `cmd/aklapi/` — standard HTTP server on port 8080
 - **Language:** Go only — no TypeScript, JavaScript, or Node tooling
@@ -54,6 +54,7 @@ docker build -t aklapi .
 
 # Make targets
 make server   # go build -o server ./cmd/aklapi
+make test     # go test ./... -race
 make docker   # docker build -t aklapi .
 ```
 
diff --git a/Makefile b/Makefile
@@ -2,10 +2,15 @@ SHELL=/bin/sh
 
 IMAGE=aklapi
 
-SRC=main.go $(wildcard aklapi/*.go)
+SRC=$(wildcard aklapi/*.go)
+PKG=./cmd/aklapi
 
 server: $(SRC) 
-	go build -o $@
+	go build -o $@ $(PKG)
+
+test:
+	go test ./... -race
+.PHONY: test
 
 docker:
 	docker build -t $(IMAGE) .
diff --git a/addr.go b/addr.go
@@ -13,6 +13,8 @@ import (
 var (
 	// defined as a variable so it can be overridden in tests.
 	addrURI = `https://experience.aucklandcouncil.govt.nz/nextapi/property`
+	// defined as a variable so tests can replace it.
+	addrHTTPClient = &http.Client{Timeout: 15 * time.Second, Transport: &browserTransport{wrapped: http.DefaultTransport}}
 )
 
 // AddrRequest is the address request.
@@ -61,7 +63,7 @@ func MatchingPropertyAddresses(ctx context.Context, addrReq *AddrRequest) (*Addr
 	req.URL.RawQuery = q.Encode()
 
 	start := time.Now()
-	resp, err := aklClient.Do(req)
+	resp, err := addrHTTPClient.Do(req)
 	if err != nil {
 		return nil, err
 	}
diff --git a/aklapi.go b/aklapi.go
@@ -14,12 +14,6 @@ var (
 // http client, so we send a browser-compatible value instead.
 const userAgent = "Mozilla/5.0 (compatible; aklapi/1.0)"
 
-// aklClient is a shared HTTP client that injects the required headers on every
-// outgoing request so that the Auckland Council CDN does not reject them.
-var aklClient = &http.Client{
-	Transport: &browserTransport{wrapped: http.DefaultTransport},
-}
-
 // browserTransport is an http.RoundTripper that adds browser-like headers to
 // every request before forwarding it to the underlying transport.
 type browserTransport struct {
diff --git a/cmd/aklapi/handlers.go b/cmd/aklapi/handlers.go
@@ -30,6 +30,7 @@ func respond(w http.ResponseWriter, data any, code int) {
 	b, err := json.Marshal(data)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
 	}
 	w.Header().Set("Content-Type", "application/json; charset=UTF-8")
 	w.WriteHeader(code)
diff --git a/go.mod b/go.mod
@@ -1,8 +1,6 @@
 module github.com/rusq/aklapi
 
-go 1.24.0
-
-toolchain go1.24.2
+go 1.25.0
 
 require (
 	github.com/PuerkitoBio/goquery v1.11.0
@@ -15,6 +13,6 @@ require (
 	github.com/andybalholm/cascadia v1.3.3 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	golang.org/x/net v0.48.0 // indirect
+	golang.org/x/net v0.51.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
diff --git a/go.sum b/go.sum
@@ -46,6 +46,8 @@ golang.org/x/net v0.39.0 h1:ZCu7HMWDxpXpaiKdhzIfaltL9Lp31x/3fCP11bc6/fY=
 golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E=
 golang.org/x/net v0.48.0 h1:zyQRTTrjc33Lhh0fBgT/H3oZq9WuvRR5gPC70xpDiQU=
 golang.org/x/net v0.48.0/go.mod h1:+ndRgGjkh8FGtu1w1FGbEC31if4VrNVMuKTgcAAnQRY=
+golang.org/x/net v0.51.0 h1:94R/GTO7mt3/4wIKpcR5gkGmRLOuE/2hNGeWq/GBIFo=
+golang.org/x/net v0.51.0/go.mod h1:aamm+2QF5ogm02fjy5Bb7CQ0WMt1/WVM7FtyaTLlA9Y=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
diff --git a/rubbish.go b/rubbish.go
@@ -23,6 +23,8 @@ const (
 var (
 	// defined as a variable so it can be overridden in tests.
 	collectionDayURI = `https://www.aucklandcouncil.govt.nz/en/rubbish-recycling/rubbish-recycling-collections/rubbish-recycling-collection-days/%s.html`
+	// defined as a variable so tests can replace it.
+	collectionHTTPClient = &http.Client{Timeout: 15 * time.Second, Transport: &browserTransport{wrapped: http.DefaultTransport}}
 )
 
 var errSkip = errors.New("skip this date")
@@ -118,11 +120,14 @@ func fetchandparse(ctx context.Context, addressID string) (*CollectionDayDetailR
 	if err != nil {
 		return nil, err
 	}
-	resp, err := aklClient.Do(req)
+	resp, err := collectionHTTPClient.Do(req)
 	if err != nil {
 		return nil, err
 	}
 	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("collection API returned status code: %d", resp.StatusCode)
+	}
 	return parse(resp.Body)
 }
 
diff --git a/rubbish_test.go b/rubbish_test.go
@@ -163,6 +163,29 @@ func TestCollectionDayDetail(t *testing.T) {
 	}
 }
 
+func TestFetchAndParse_StatusCodeError(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusServiceUnavailable)
+		w.Write([]byte("temporary error"))
+	}))
+	defer srv.Close()
+
+	oldURI := collectionDayURI
+	oldClient := collectionHTTPClient
+	defer func() {
+		collectionDayURI = oldURI
+		collectionHTTPClient = oldClient
+	}()
+
+	collectionDayURI = srv.URL + "/rubbish/%s"
+	collectionHTTPClient = srv.Client()
+
+	_, err := fetchandparse(t.Context(), "42")
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+}
+
 func TestCollectionDayDetailResult_NextRubbish(t *testing.T) {
 	type fields struct {
 		Collections []RubbishCollection

-Original file line number
+Diff line change
 *~
 .vscode
 .idea
 +.env
 # binary files
 aklapi
Original file line number	Diff line number	Diff line change
`@@ -30,6 +30,7 @@ func respond(w http.ResponseWriter, data any, code int) {`
`30`	`30`	`b, err := json.Marshal(data)`
`31`	`31`	`if err != nil {`
`32`	`32`	`http.Error(w, err.Error(), http.StatusInternalServerError)`
	`33`	`+ return`
`33`	`34`	`}`
`34`	`35`	`w.Header().Set("Content-Type", "application/json; charset=UTF-8")`
`35`	`36`	`w.WriteHeader(code)`
Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,8 @@ const (`
`23`	`23`	`var (`
`24`	`24`	`// defined as a variable so it can be overridden in tests.`
`25`	`25`	collectionDayURI = `https://www.aucklandcouncil.govt.nz/en/rubbish-recycling/rubbish-recycling-collections/rubbish-recycling-collection-days/%s.html`
	`26`	`+ // defined as a variable so tests can replace it.`
	`27`	`+ collectionHTTPClient = &http.Client{Timeout: 15 * time.Second, Transport: &browserTransport{wrapped: http.DefaultTransport}}`
`26`	`28`	`)`
`27`	`29`
`28`	`30`	`var errSkip = errors.New("skip this date")`
`@@ -118,11 +120,14 @@ func fetchandparse(ctx context.Context, addressID string) (*CollectionDayDetailR`
`118`	`120`	`if err != nil {`
`119`	`121`	`return nil, err`
`120`	`122`	`}`
`121`		`- resp, err := aklClient.Do(req)`
	`123`	`+ resp, err := collectionHTTPClient.Do(req)`
`122`	`124`	`if err != nil {`
`123`	`125`	`return nil, err`
`124`	`126`	`}`
`125`	`127`	`defer resp.Body.Close()`
	`128`	`+ if resp.StatusCode != http.StatusOK {`
	`129`	`+ return nil, fmt.Errorf("collection API returned status code: %d", resp.StatusCode)`
	`130`	`+ }`
`126`	`131`	`return parse(resp.Body)`
`127`	`132`	`}`
`128`	`133`