-
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?
Conversation
Hi @kordianbruck, Let me know if any changes are required required. Thanks! |
backends/openweathermap.org.go
Outdated
@@ -55,6 +57,15 @@ type dataBlock struct { | |||
} `json:"rain"` | |||
} | |||
|
|||
type openWeatherErrorReponse struct { | |||
Cod any `json:"cod"` |
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 just cod
.
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
backends/openweathermap.org.go
Outdated
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 comment
The reason will be displayed to describe this comment to others. Learn more.
No need for this else
, because you have a return above.
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 removed the else block and handled the API response outside of the else block
backends/openweathermap.org.go
Outdated
} | ||
return &resp, nil | ||
|
||
switch v := raw.Cod.(type) { |
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.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
String and Integer Cod Value -
so, if you see this
-
This is the API response with a success, cod is of type string
-
This is the API response when I am tampering my API key they are sending a integer
-
This is the API response when I am sending request to a wrong path (removed e from the forecast)
https://api.openweathermap.org/data/2.5/forcast?q=london&appid=xxxxxxxx&units=metric&lang=en
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 comment
The 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 float64
Motivation and Context
This change was requested in this issue #176
Description
I've handled error and success case that is coming from open weather API where I am checking the response code from the API and then printing it to the terminal, The marshalling was not happening because API sends Code as a string value for some of cases but if there is an invalid API key they send Code value is coming as an integer value I've handled this and just checking the type and marshalling it to a new struct called
openWeatherErrorResponse
Steps for Testing
To test we can check different API responses like -
Screenshots
Invalid API key
Invalid API route
Success