Validate Weather API endpoints and retries

docs/config.md

@@ -45,7 +45,7 @@ All omitted fields use their built-in defaults.
 
 | Field | Default | Rules |
 | --- | --- | --- |
-| `base_url` | empty | Absolute Weather API URL. Required for collection and generation. |
+| `base_url` | empty | Absolute HTTP(S) Weather API URL. Required for collection and generation. |
 | `timeout` | `10s` | Must be greater than zero. |
 | `precision` | `0` | Must be zero or greater. Sent as the Weather API precision query value. |
 | `units` | `us` | Required Weather API units query value; `--units` overrides it for one command. |

docs/integrations/weatherapi.md

@@ -9,9 +9,9 @@ are documented in [Weather data internals](../internal/weather-data.md) and
 
 ## Base URL And Requests
 
-`weather_api.base_url` must be an absolute URL. Weatherreporter joins each
+`weather_api.base_url` must be an absolute HTTP(S) URL. Weatherreporter joins
-endpoint path to the configured base URL path, so a service hosted under a path
+each endpoint path to the configured base URL path, so a service hosted under a
-prefix must keep that prefix available. Requests use `GET` and carry the
+path prefix must keep that prefix available. Requests use `GET` and carry the
 configured timeout on every HTTP attempt.
 
 Every request sends `format` and, except where noted below, `units`. The

docs/internal/collect.md

@@ -11,10 +11,10 @@ semantics belong in [weather-data internals](weather-data.md).
 `config.Config`. It constructs the Weather API adapter from that configuration,
 calls `FetchBundle`, and returns `Result{Bundle: *weatherdata.Bundle}`.
 
-The package wraps adapter construction failures as weather-collection setup
+The package wraps adapter construction failures, including an invalid Weather
-errors and fetch failures as bundle-collection errors. It does not retry,
+API base URL, as weather-collection setup errors and fetch failures as
-persist, select reports, derive facts, build modules, invoke Promptkit, or
+bundle-collection errors. It does not retry, persist, select reports, derive
-notify Distributor.
+facts, build modules, invoke Promptkit, or notify Distributor.
 
 ## Application Composition
 

internal/adapters/weatherapi/client.go

@@ -75,6 +75,9 @@ func New(cfg config.Config, opts ...Option) (*Client, error) {
 	if err != nil || baseURL.Scheme == "" || baseURL.Host == "" {
 		return nil, fmt.Errorf("weather_api.base_url must be an absolute URL")
 	}
+	if !strings.EqualFold(baseURL.Scheme, "http") && !strings.EqualFold(baseURL.Scheme, "https") {
+		return nil, fmt.Errorf("weather_api.base_url must use http or https")
+	}
 
 	timeout := cfg.WeatherAPI.Timeout
 	if timeout <= 0 {
@@ -453,22 +456,27 @@ func (c *Client) warmup(ctx context.Context) error {
 	}
 	attempts := positiveAttemptCount(c.warmupAttempts)
 	var lastErr error
+	var lastRetryable bool
 	for attempt := 1; attempt <= attempts; attempt++ {
 		if err := ctx.Err(); err != nil {
 			return fmt.Errorf("warm up weather API via %s: %w", endpoint, err)
 		}
 		if err := c.warmupOnce(ctx, endpoint); err != nil {
 			lastErr = err
+			lastRetryable = isRetryableRequestError(err)
 		} else {
 			return nil
 		}
-		if attempt == attempts {
+		if !lastRetryable || attempt == attempts {
 			break
 		}
 		if err := waitForRetry(ctx, c.warmupDelay); err != nil {
 			return fmt.Errorf("warm up weather API via %s after %d attempt(s): %w", endpoint, attempt, err)
 		}
 	}
+	if !lastRetryable {
+		return lastErr
+	}
 	return fmt.Errorf("warm up weather API via %s failed after %d attempts: %w", endpoint, attempts, lastErr)
 }
 
@@ -481,16 +489,28 @@ func (c *Client) warmupOnce(ctx context.Context, endpoint string) error {
 
 	resp, err := c.httpClient.Do(req)
 	if err != nil {
-		return fmt.Errorf("fetch %s: %w", endpoint, err)
+		err = fmt.Errorf("fetch %s: %w", endpoint, err)
+		if ctx.Err() != nil {
+			return err
+		}
+		return retryableRequestError{err: err}
 	}
 	defer resp.Body.Close()
 
 	body, err := io.ReadAll(io.LimitReader(resp.Body, 10<<20))
 	if err != nil {
-		return fmt.Errorf("read %s response: %w", endpoint, err)
+		err = fmt.Errorf("read %s response: %w", endpoint, err)
+		if ctx.Err() != nil {
+			return err
+		}
+		return retryableRequestError{err: err}
 	}
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		return fmt.Errorf("fetch %s: unexpected HTTP status %d: %s", endpoint, resp.StatusCode, strings.TrimSpace(string(body)))
+		err := fmt.Errorf("fetch %s: unexpected HTTP status %d: %s", endpoint, resp.StatusCode, strings.TrimSpace(string(body)))
+		if isRetryableHTTPStatus(resp.StatusCode) {
+			return retryableRequestError{err: err}
+		}
+		return err
 	}
 	return nil
 }

internal/adapters/weatherapi/client_test.go

@@ -3,6 +3,7 @@ package weatherapi
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -15,6 +16,12 @@ import (
 	"gitea.maximumdirect.net/eric/weatherreporter/internal/weatherdata"
 )
 
+type roundTripperFunc func(*http.Request) (*http.Response, error)
+
+func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) {
+	return f(req)
+}
+
 func TestFetchBundleFromFixtures(t *testing.T) {
 	var requested []string
 	server := fixtureServer(t, nil, &requested)
@@ -265,6 +272,25 @@ func TestWarmupFailureStopsBeforeSourceFetches(t *testing.T) {
 	}
 }
 
+func TestWarmupDoesNotRetryPermanentStatus(t *testing.T) {
+	var requested []string
+	server := fixtureServer(t, map[string]handlerOverride{
+		defaultWarmupEndpoint: {status: http.StatusNotFound, body: `not found`},
+	}, &requested)
+	client := newTestClient(t, server.URL+"/", nil)
+
+	_, err := client.FetchBundle(context.Background())
+	if err == nil || !strings.Contains(err.Error(), "404") {
+		t.Fatalf("FetchBundle() error = %v, want non-retryable warmup status", err)
+	}
+	if got := countPath(requested, defaultWarmupEndpoint); got != 1 {
+		t.Fatalf("warmup requests = %d, want 1; all requests = %v", got, requested)
+	}
+	if containsPath(requested, "/observations") {
+		t.Fatalf("requested paths = %v, want warmup failure before source fetches", requested)
+	}
+}
+
 func TestFetchRetriesRetryableStatus(t *testing.T) {
 	var hourlyCalls int
 	server := fixtureServer(t, map[string]handlerOverride{
@@ -312,6 +338,40 @@ func TestFetchDoesNotRetryNonRetryableStatus(t *testing.T) {
 	}
 }
 
+func TestNewValidatesWeatherAPIBaseURLSchemeWithoutRequests(t *testing.T) {
+	requests := 0
+	httpClient := &http.Client{Transport: roundTripperFunc(func(*http.Request) (*http.Response, error) {
+		requests++
+		return nil, errors.New("unexpected request")
+	})}
+	tests := []struct {
+		name    string
+		baseURL string
+		wantErr string
+	}{
+		{name: "local HTTP", baseURL: "http://127.0.0.1:8080/weather/"},
+		{name: "local HTTPS", baseURL: "https://127.0.0.1:8443/weather/"},
+		{name: "unsupported scheme", baseURL: "ftp://weather.example.test/", wantErr: "weather_api.base_url must use http or https"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := testConfig(tt.baseURL)
+			_, err := New(cfg, WithHTTPClient(httpClient))
+			if tt.wantErr == "" {
+				if err != nil {
+					t.Fatalf("New() error = %v", err)
+				}
+			} else if err == nil || !strings.Contains(err.Error(), tt.wantErr) {
+				t.Fatalf("New() error = %v, want %q", err, tt.wantErr)
+			}
+		})
+	}
+	if requests != 0 {
+		t.Fatalf("HTTP requests = %d, want none", requests)
+	}
+}
+
 func TestFetchDoesNotRetryMalformedEnvelope(t *testing.T) {
 	var hourlyCalls int
 	server := fixtureServer(t, map[string]handlerOverride{

internal/config/config_test.go

@@ -82,6 +82,36 @@ func TestDefaults(t *testing.T) {
 	}
 }
 
+func TestWeatherAPIBaseURLValidation(t *testing.T) {
+	tests := []struct {
+		name    string
+		baseURL string
+		wantErr string
+	}{
+		{name: "local HTTP", baseURL: "http://127.0.0.1:8080/weather/"},
+		{name: "local HTTPS", baseURL: "https://127.0.0.1:8443/weather/"},
+		{name: "unsupported scheme", baseURL: "ftp://weather.example.test/", wantErr: "weather_api.base_url must use http or https"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := Defaults()
+			cfg.WeatherAPI.BaseURL = tt.baseURL
+
+			err := Validate(cfg)
+			if tt.wantErr == "" {
+				if err != nil {
+					t.Fatalf("Validate() error = %v", err)
+				}
+				return
+			}
+			if err == nil || !strings.Contains(err.Error(), tt.wantErr) {
+				t.Fatalf("Validate() error = %v, want %q", err, tt.wantErr)
+			}
+		})
+	}
+}
+
 func TestOutputDirectoryLoading(t *testing.T) {
 	tests := []struct {
 		name      string

internal/config/validate.go

@@ -23,6 +23,9 @@ func Validate(cfg Config) error {
 		if err != nil || parsed.Scheme == "" || parsed.Host == "" {
 			return fmt.Errorf("weather_api.base_url must be an absolute URL")
 		}
+		if !strings.EqualFold(parsed.Scheme, "http") && !strings.EqualFold(parsed.Scheme, "https") {
+			return fmt.Errorf("weather_api.base_url must use http or https")
+		}
 	}
 	if cfg.WeatherAPI.Timeout <= 0 {
 		return fmt.Errorf("weather_api.timeout must be greater than zero")