-
Notifications
You must be signed in to change notification settings - Fork 477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enhanced error handling for open weather API errors #192
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,13 +4,15 @@ import ( | |
"encoding/json" | ||
"flag" | ||
"fmt" | ||
"github.com/schachmat/wego/iface" | ||
"io" | ||
"log" | ||
"net/http" | ||
"regexp" | ||
"strconv" | ||
"strings" | ||
"time" | ||
|
||
"github.com/schachmat/wego/iface" | ||
) | ||
|
||
type openWeatherConfig struct { | ||
|
@@ -22,12 +24,12 @@ type openWeatherConfig struct { | |
type openWeatherResponse struct { | ||
Cod string `json:"cod"` | ||
City struct { | ||
Name string `json:"name"` | ||
Country string `json:"country"` | ||
TimeZone int64 `json: "timezone"` | ||
Name string `json:"name"` | ||
Country string `json:"country"` | ||
TimeZone int64 `json: "timezone"` | ||
// sunrise/sunset are once per call | ||
SunRise int64 `json: "sunrise"` | ||
SunSet int64 `json: "sunset"` | ||
SunSet int64 `json: "sunset"` | ||
} `json:"city"` | ||
List []dataBlock `json:"list"` | ||
} | ||
|
@@ -55,6 +57,15 @@ type dataBlock struct { | |
} `json:"rain"` | ||
} | ||
|
||
type openWeatherErrorReponse struct { | ||
Cod any `json:"cod"` | ||
Message string `json:"message"` | ||
} | ||
|
||
func (e openWeatherErrorReponse) Error() string { | ||
return fmt.Sprintf("Error Response from openweathermap.org (%v): %s", e.Cod, e.Message) | ||
} | ||
|
||
const ( | ||
openweatherURI = "http://api.openweathermap.org/data/2.5/forecast?%s&appid=%s&units=metric&lang=%s" | ||
) | ||
|
@@ -82,15 +93,52 @@ func (c *openWeatherConfig) fetch(url string) (*openWeatherResponse, error) { | |
if c.debug { | ||
fmt.Printf("Response (%s):\n%s\n", url, string(body)) | ||
} | ||
if res.StatusCode != 200 { | ||
err = openWeatherErrorReponseHandler(body, url) | ||
return nil, err | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have removed the else block and handled the API response outside of the else block |
||
var resp openWeatherResponse | ||
if err = json.Unmarshal(body, &resp); err != nil { | ||
return nil, fmt.Errorf("Unable to unmarshal response (%s): %v\nThe json body is: %s", url, err, string(body)) | ||
} | ||
if resp.Cod != "200" { | ||
return nil, fmt.Errorf("Erroneous response body: %s", string(body)) | ||
} | ||
return &resp, nil | ||
} | ||
} | ||
|
||
func openWeatherErrorReponseHandler(body []byte, url string) error { | ||
var resp openWeatherErrorReponse | ||
|
||
if err := resp.UnmarshalJSON(body); err != nil { | ||
return fmt.Errorf("Unable to unmarshal error response (%s): %v\nThe json body is: %s", url, err, string(body)) | ||
} | ||
|
||
return resp | ||
} | ||
|
||
var resp openWeatherResponse | ||
if err = json.Unmarshal(body, &resp); err != nil { | ||
return nil, fmt.Errorf("Unable to unmarshal response (%s): %v\nThe json body is: %s", url, err, string(body)) | ||
func (r *openWeatherErrorReponse) UnmarshalJSON(data []byte) error { | ||
var raw struct { | ||
Cod interface{} `json:"cod"` | ||
Message string `json:"message"` | ||
} | ||
if resp.Cod != "200" { | ||
return nil, fmt.Errorf("Erroneous response body: %s", string(body)) | ||
|
||
if err := json.Unmarshal(data, &raw); err != nil { | ||
return err | ||
} | ||
return &resp, nil | ||
|
||
switch v := raw.Cod.(type) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is weird: can the type really be a string? Codes are usually only integers, so using a float64 here makes no sense to me. According to the docs this should always be an integer: https://openweathermap.org/api/one-call-3#errorstructure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. String and Integer Cod Value -so, if you see this
They are sending a string in few cases like this 404 and 200 but for 400 it is an integer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why Float64?The reason why I am comparing type with float64 is because I am taking Code as a type of interface and when go unmarshals an interface value for numbers it returns the number with the type of |
||
case string: | ||
r.Cod = v | ||
case float64: | ||
r.Cod = strconv.Itoa(int(v)) | ||
default: | ||
return fmt.Errorf("unexpected cod type") | ||
} | ||
|
||
r.Message = raw.Message | ||
return nil | ||
} | ||
|
||
func (c *openWeatherConfig) parseDaily(dataInfo []dataBlock, numdays int) []iface.Day { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets call this
Code
internally, even if the API spec we parse is justcod
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed Cod to Code everywhere for
openWeatherErrorResponse