weather: check HTTP status before decoding open-meteo replies (V-589)

Neither CurrentWeather nor geocodeOne checked resp.StatusCode, so a
non-200 forecast reply decoded into a zero-value struct reported as a
real 0-degree answer, and a non-200 geocode reply decoded into an empty
result list and was reported as ErrLocationUnknown — blaming the owner
for a service outage. Both now check http.StatusOK first, matching the
sibling kiwix and websearch clients, and return a wrapped error naming
the status instead.

Adds httptest coverage for a 500 from the forecast endpoint, a 500 from
the geocode endpoint, and a genuine empty geocode result, asserting each
takes a different path.
This commit is contained in:
2026-08-06 02:47:24 +04:00
parent 1fa14e95a4
commit e8ece874b1
2 changed files with 89 additions and 0 deletions
+6
View File
@@ -83,6 +83,9 @@ func (p *OpenMeteoProvider) CurrentWeather(ctx context.Context, location string)
return Weather{}, fmt.Errorf("open-meteo: http: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return Weather{}, fmt.Errorf("open-meteo forecast: http %d", resp.StatusCode)
}
body, err := io.ReadAll(resp.Body)
if err != nil {
@@ -192,6 +195,9 @@ func (p *OpenMeteoProvider) geocodeOne(ctx context.Context, location string) (la
return 0, 0, "", err
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return 0, 0, "", fmt.Errorf("open-meteo geocode: http %d", resp.StatusCode)
}
body, err := io.ReadAll(resp.Body)
if err != nil {
+83
View File
@@ -3,9 +3,11 @@ package weather
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"
)
@@ -76,6 +78,87 @@ func (t *mockTransport) RoundTrip(r *http.Request) (*http.Response, error) {
return http.DefaultTransport.RoundTrip(req)
}
// TestOpenMeteoProvider_ForecastHTTPError — a non-200 forecast reply must not
// decode into a zero-value Weather{Temperature: 0, Condition: "неизвестно"}
// reported as success (Vikunja #589). It must be a distinct error, not
// ErrLocationUnknown, so the caller does not blame the owner's city.
func TestOpenMeteoProvider_ForecastHTTPError(t *testing.T) {
mock := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/v1/search" {
json.NewEncoder(w).Encode(geoResponse{
Results: []geoResult{{Name: "Moscow", Latitude: 55.7558, Longitude: 37.6173, Country: "Russia"}},
})
return
}
w.WriteHeader(http.StatusInternalServerError)
}))
defer mock.Close()
p := NewOpenMeteoProvider()
p.SetClient(&http.Client{Transport: &mockTransport{
geoURL: mock.URL + "/v1/search",
forecastURL: mock.URL + "/v1/forecast",
}})
w, err := p.CurrentWeather(context.Background(), "Moscow")
if err == nil {
t.Fatalf("CurrentWeather: want error, got Weather{%+v}", w)
}
if errors.Is(err, ErrLocationUnknown) {
t.Fatalf("CurrentWeather: want a service error, got ErrLocationUnknown: %v", err)
}
if !strings.Contains(err.Error(), "500") {
t.Errorf("CurrentWeather: error %q does not name the status", err.Error())
}
}
// TestOpenMeteoProvider_GeocodeHTTPError — a non-200 geocode reply must not be
// read as "no such city" (ErrLocationUnknown). A 500 is a service outage, and
// the owner hears a different sentence for each (Vikunja #589).
func TestOpenMeteoProvider_GeocodeHTTPError(t *testing.T) {
mock := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
}))
defer mock.Close()
p := NewOpenMeteoProvider()
p.SetClient(&http.Client{Transport: &mockTransport{
geoURL: mock.URL + "/v1/search",
forecastURL: mock.URL + "/v1/forecast",
}})
_, err := p.CurrentWeather(context.Background(), "Уфе")
if err == nil {
t.Fatal("CurrentWeather: want error")
}
if errors.Is(err, ErrLocationUnknown) {
t.Fatalf("CurrentWeather: want a service error, got ErrLocationUnknown: %v", err)
}
if !strings.Contains(err.Error(), "500") {
t.Errorf("CurrentWeather: error %q does not name the status", err.Error())
}
}
// TestOpenMeteoProvider_GeocodeEmptyResult — a genuine 200 reply with no
// results is still ErrLocationUnknown, distinct from a service failure.
func TestOpenMeteoProvider_GeocodeEmptyResult(t *testing.T) {
mock := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
json.NewEncoder(w).Encode(geoResponse{Results: nil})
}))
defer mock.Close()
p := NewOpenMeteoProvider()
p.SetClient(&http.Client{Transport: &mockTransport{
geoURL: mock.URL + "/v1/search",
forecastURL: mock.URL + "/v1/forecast",
}})
_, err := p.CurrentWeather(context.Background(), "Атлантида")
if !errors.Is(err, ErrLocationUnknown) {
t.Fatalf("CurrentWeather: want ErrLocationUnknown, got %v", err)
}
}
func TestStubProvider(t *testing.T) {
p := NewStubProvider()
_, err := p.CurrentWeather(context.Background(), "Moscow")