From d2658b5c0c7bd8c6214a8e34582cdba23da8ce0d Mon Sep 17 00:00:00 2001 From: Julien Dessaux Date: Tue, 4 May 2021 16:19:42 +0200 Subject: Improved navitia api error handling and testing --- pkg/navitia_api_client/departures.go | 35 ++-- pkg/navitia_api_client/departures_test.go | 75 ++++++-- pkg/navitia_api_client/error.go | 70 +++++++ pkg/navitia_api_client/error_test.go | 17 ++ pkg/navitia_api_client/test_data/invalid_date.json | 209 +++++++++++++++++++++ 5 files changed, 372 insertions(+), 34 deletions(-) create mode 100644 pkg/navitia_api_client/error.go create mode 100644 pkg/navitia_api_client/error_test.go create mode 100644 pkg/navitia_api_client/test_data/invalid_date.json diff --git a/pkg/navitia_api_client/departures.go b/pkg/navitia_api_client/departures.go index 7447cdc..50ebe17 100644 --- a/pkg/navitia_api_client/departures.go +++ b/pkg/navitia_api_client/departures.go @@ -57,27 +57,32 @@ func (c *NavitiaClient) GetDepartures(trainStop string) (departures []model.Depa } req, err := http.NewRequest("GET", request, nil) if err != nil { - return nil, err + return nil, newHttpClientError("http.NewRequest error", err) } resp, err := c.httpClient.Do(req) if err != nil { - return nil, err + return nil, newHttpClientError("httpClient.Do error", err) } defer resp.Body.Close() - var data DeparturesResponse - if err = json.NewDecoder(resp.Body).Decode(&data); err != nil { - return nil, err - } - for i := 0; i < len(data.Departures); i++ { - t, err := time.Parse("20060102T150405", data.Departures[i].StopDateTime.ArrivalDateTime) - if err != nil { // TODO fail better - panic(err) + if resp.StatusCode == http.StatusOK { + var data DeparturesResponse + if err = json.NewDecoder(resp.Body).Decode(&data); err != nil { + return nil, newJsonDecodeError("GetDepartures "+trainStop, err) } - departures = append(departures, model.Departure{data.Departures[i].DisplayInformations.Direction, t.Format("Mon, 02 Jan 2006 15:04:05")}) - } - c.cache[request] = cachedResult{ - ts: start, - result: departures, + // TODO test for no json error + for i := 0; i < len(data.Departures); i++ { + t, err := time.Parse("20060102T150405", data.Departures[i].StopDateTime.ArrivalDateTime) + if err != nil { + return nil, newDateParsingError(data.Departures[i].StopDateTime.ArrivalDateTime, err) + } + departures = append(departures, model.Departure{data.Departures[i].DisplayInformations.Direction, t.Format("Mon, 02 Jan 2006 15:04:05")}) + } + c.cache[request] = cachedResult{ + ts: start, + result: departures, + } + } else { + err = newApiError(resp.StatusCode, "GetDepartures "+trainStop) } return } diff --git a/pkg/navitia_api_client/departures_test.go b/pkg/navitia_api_client/departures_test.go index 9b2f52c..3cf9eab 100644 --- a/pkg/navitia_api_client/departures_test.go +++ b/pkg/navitia_api_client/departures_test.go @@ -3,35 +3,72 @@ package navitia_api_client import ( "net/http" "net/http/httptest" + "reflect" "testing" + + "git.adyxax.org/adyxax/trains/pkg/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetDepartures(t *testing.T) { - // invalid characters in token - client := NewClient("}") - _, err := client.GetDepartures("test") - if err == nil { - t.Fatalf("invalid characters in token should raise an error because the url is invalid") + // Simple Test cases + testCases := []struct { + name string + inputNewCLient string + inputGetDepartures string + expected []model.Departure + expectedError interface{} + }{ + {"invalid characters in token should fail", "}", "test", nil, &HttpClientError{}}, + {"unreachable server should fail", "https://", "test", nil, &HttpClientError{}}, } - // unreachable server - client = NewClient("https://") - _, err = client.GetDepartures("test") - if err == nil { - t.Fatalf("unreachable server should raise an error") + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + client := NewClient(tc.inputNewCLient) + valid, err := client.GetDepartures(tc.inputGetDepartures) + if tc.expectedError != nil { + require.Error(t, err) + assert.Equalf(t, reflect.TypeOf(err), reflect.TypeOf(tc.expectedError), "Invalid error type. Got %s but expected %s", reflect.TypeOf(err), reflect.TypeOf(tc.expectedError)) + assert.Equal(t, tc.expected, valid) + } else { + require.NoError(t, err) + assert.Equal(t, tc.expected, valid) + } + }) } - // invalid json - client, ts := newTestClientFromFilename(t, "test_data/invalid.json") - defer ts.Close() - _, err = client.GetDepartures("test") - if err == nil { - t.Fatalf("invalid json should raise an error") + // Test cases with a filename + testCasesFilename := []struct { + name string + inputFilename string + inputGetDepartures string + expected []model.Departure + expectedError interface{} + }{ + {"invalid json should fail", "test_data/invalid.json", "test", nil, &JsonDecodeError{}}, + {"invalid date should fail", "test_data/invalid_date.json", "test", nil, &DateParsingError{}}, + } + for _, tc := range testCasesFilename { + t.Run(tc.name, func(t *testing.T) { + client, ts := newTestClientFromFilename(t, tc.inputFilename) + defer ts.Close() + valid, err := client.GetDepartures(tc.inputGetDepartures) + if tc.expectedError != nil { + require.Error(t, err) + assert.Equalf(t, reflect.TypeOf(err), reflect.TypeOf(tc.expectedError), "Invalid error type. Got %s but expected %s", reflect.TypeOf(err), reflect.TypeOf(tc.expectedError)) + assert.Equal(t, tc.expected, valid) + } else { + require.NoError(t, err) + assert.Equal(t, tc.expected, valid) + } + }) } // http error - ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) })) - client = newTestClient(ts) - _, err = client.GetDepartures("test") + client := newTestClient(ts) + _, err := client.GetDepartures("test") if err == nil { t.Fatalf("404 should raise an error") } diff --git a/pkg/navitia_api_client/error.go b/pkg/navitia_api_client/error.go new file mode 100644 index 0000000..31080de --- /dev/null +++ b/pkg/navitia_api_client/error.go @@ -0,0 +1,70 @@ +package navitia_api_client + +import "fmt" + +// navitia api query error +type ApiError struct { + code int + request string +} + +func (e *ApiError) Error() string { + return fmt.Sprintf("Navitia Api error return code %d - %s", e.code, e.request) +} + +func newApiError(code int, request string) error { + return &ApiError{ + code: code, + request: request, + } +} + +// http client error +type HttpClientError struct { + msg string + err error +} + +func (e *HttpClientError) Error() string { return fmt.Sprintf("Navitia HttpClient error %s", e.msg) } +func (e *HttpClientError) Unwrap() error { return e.err } + +func newHttpClientError(msg string, err error) error { + return &HttpClientError{ + msg: msg, + err: err, + } +} + +// json decoding error +type JsonDecodeError struct { + msg string + err error +} + +func (e *JsonDecodeError) Error() string { return fmt.Sprintf("Navitia JsonDecode error %s", e.msg) } +func (e *JsonDecodeError) Unwrap() error { return e.err } + +func newJsonDecodeError(msg string, err error) error { + return &JsonDecodeError{ + msg: msg, + err: err, + } +} + +// date parsing error +type DateParsingError struct { + date string + err error +} + +func (e *DateParsingError) Error() string { + return fmt.Sprintf("Navitia date parsing error %s", e.date) +} +func (e *DateParsingError) Unwrap() error { return e.err } + +func newDateParsingError(date string, err error) error { + return &DateParsingError{ + date: date, + err: err, + } +} diff --git a/pkg/navitia_api_client/error_test.go b/pkg/navitia_api_client/error_test.go new file mode 100644 index 0000000..5ecf8c2 --- /dev/null +++ b/pkg/navitia_api_client/error_test.go @@ -0,0 +1,17 @@ +package navitia_api_client + +import "testing" + +func TestErrorsCoverage(t *testing.T) { + apiErr := ApiError{} + _ = apiErr.Error() + httpClientErr := HttpClientError{} + _ = httpClientErr.Error() + _ = httpClientErr.Unwrap() + jsonDecodeErr := JsonDecodeError{} + _ = jsonDecodeErr.Error() + _ = jsonDecodeErr.Unwrap() + dateParsingErr := DateParsingError{} + _ = dateParsingErr.Error() + _ = dateParsingErr.Unwrap() +} diff --git a/pkg/navitia_api_client/test_data/invalid_date.json b/pkg/navitia_api_client/test_data/invalid_date.json new file mode 100644 index 0000000..6b9e20d --- /dev/null +++ b/pkg/navitia_api_client/test_data/invalid_date.json @@ -0,0 +1,209 @@ +{ + "departures": [ + { + "display_informations": { + "direction": "Ambérieu-en-Bugey (Ambérieu-en-Bugey)", + "code": "", + "network": "SNCF", + "links": [], + "color": "000000", + "name": "St-Etienne - Lyon - Ambérieu", + "physical_mode": "Train régional / TER", + "headsign": "886823", + "label": "St-Etienne - Lyon - Ambérieu", + "equipments": [], + "text_color": "FFFFFF", + "trip_short_name": "886823", + "commercial_mode": "TER", + "description": "" + }, + "stop_point": { + "commercial_modes": [ + { + "id": "commercial_mode:ter", + "name": "TER" + } + ], + "name": "Crépieux-la-Pape", + "links": [], + "physical_modes": [ + { + "id": "physical_mode:LocalTrain", + "name": "Train régional / TER" + } + ], + "coord": { + "lat": "45.803921", + "lon": "4.892737" + }, + "label": "Crépieux-la-Pape (Rillieux-la-Pape)", + "equipments": [], + "administrative_regions": [ + { + "insee": "69286", + "name": "Rillieux-la-Pape", + "level": 8, + "coord": { + "lat": "45.823514", + "lon": "4.8994366" + }, + "label": "Rillieux-la-Pape (69140)", + "id": "admin:fr:69286", + "zip_code": "69140" + } + ], + "fare_zone": { + "name": "0" + }, + "id": "stop_point:OCE:SP:TrainTER-87723502", + "stop_area": { + "codes": [ + { + "type": "CR-CI-CH", + "value": "0087-723502-00" + }, + { + "type": "UIC8", + "value": "87723502" + }, + { + "type": "external_code", + "value": "OCE87723502" + } + ], + "name": "Crépieux-la-Pape", + "links": [], + "coord": { + "lat": "45.803921", + "lon": "4.892737" + }, + "label": "Crépieux-la-Pape (Rillieux-la-Pape)", + "administrative_regions": [ + { + "insee": "69286", + "name": "Rillieux-la-Pape", + "level": 8, + "coord": { + "lat": "45.823514", + "lon": "4.8994366" + }, + "label": "Rillieux-la-Pape (69140)", + "id": "admin:fr:69286", + "zip_code": "69140" + } + ], + "timezone": "Europe/Paris", + "id": "stop_area:OCE:SA:87723502" + } + }, + "route": { + "direction": { + "embedded_type": "stop_area", + "stop_area": { + "codes": [ + { + "type": "CR-CI-CH", + "value": "0087-743716-BV" + }, + { + "type": "UIC8", + "value": "87743716" + }, + { + "type": "external_code", + "value": "OCE87743716" + } + ], + "name": "Ambérieu-en-Bugey", + "links": [], + "coord": { + "lat": "45.954008", + "lon": "5.342313" + }, + "label": "Ambérieu-en-Bugey (Ambérieu-en-Bugey)", + "timezone": "Europe/Paris", + "id": "stop_area:OCE:SA:87743716" + }, + "quality": 0, + "name": "Ambérieu-en-Bugey (Ambérieu-en-Bugey)", + "id": "stop_area:OCE:SA:87743716" + }, + "name": "St-Etienne-Châteaucreux vers Ambérieu-en-Bugey (Train TER)", + "links": [], + "physical_modes": [ + { + "id": "physical_mode:LocalTrain", + "name": "Train régional / TER" + } + ], + "is_frequence": "False", + "geojson": { + "type": "MultiLineString", + "coordinates": [] + }, + "direction_type": "forward", + "line": { + "code": "", + "name": "St-Etienne - Lyon - Ambérieu", + "links": [], + "color": "000000", + "geojson": { + "type": "MultiLineString", + "coordinates": [] + }, + "text_color": "FFFFFF", + "physical_modes": [ + { + "id": "physical_mode:LocalTrain", + "name": "Train régional / TER" + } + ], + "codes": [], + "closing_time": "221200", + "opening_time": "053500", + "commercial_mode": { + "id": "commercial_mode:ter", + "name": "TER" + }, + "id": "line:OCE:199" + }, + "id": "route:OCE:199-TrainTER-87726000-87743716" + }, + "links": [ + { + "type": "line", + "id": "line:OCE:199" + }, + { + "type": "vehicle_journey", + "id": "vehicle_journey:OCE:SN886823F29029_dst_1" + }, + { + "type": "route", + "id": "route:OCE:199-TrainTER-87726000-87743716" + }, + { + "type": "commercial_mode", + "id": "commercial_mode:ter" + }, + { + "type": "physical_mode", + "id": "physical_mode:LocalTrain" + }, + { + "type": "network", + "id": "network:sncf" + } + ], + "stop_date_time": { + "links": [], + "arrival_date_time": "XXX", + "additional_informations": [], + "departure_date_time": "20210218T131800", + "base_arrival_date_time": "20210218T131800", + "base_departure_date_time": "20210218T131800", + "data_freshness": "base_schedule" + } + } + ] +} -- cgit v1.2.3