coderhs / ruby_open_weather_map

A ruby wrapper for open weather map
MIT License
79 stars 52 forks source link

response code is string on 404, but an Integer on 200 #35

Closed vheuken closed 7 years ago

vheuken commented 8 years ago

On a 404, the response code is a string. On a 200, the response code is an Integer.

2.3.0 :042 > hayward = OpenWeather::Current.city("Hayward, CA", options)
 => {"coord"=>{"lon"=>-122.08, "lat"=>37.67}, "weather"=>[{"id"=>800, "main"=>"Clear", "description"=>"clear sky", "icon"=>"01n"}], "base"=>"stations", "main"=>{"temp"=>22.64, "pressure"=>1017, "humidity"=>52, "temp_min"=>17, "temp_max"=>28}, "visibility"=>16093, "wind"=>{"speed"=>7.7, "deg"=>290}, "clouds"=>{"all"=>1}, "dt"=>1473905437, "sys"=>{"type"=>1, "id"=>397, "message"=>0.0403, "country"=>"US", "sunrise"=>1473947449, "sunset"=>1473992102}, "id"=>5355933, "name"=>"Hayward", "cod"=>200} 
2.3.0 :043 > hayward['cod']
 => 200 
2.3.0 :044 > hayward['cod'].is_a? Integer
 => true 
2.3.0 :045 > bad_data = OpenWeather::Current.city("99999999999", options)
 => {"cod"=>"404", "message"=>"Error: Not found city"} 
2.3.0 :046 > bad_data['cod']
 => "404" 
2.3.0 :047 > bad_data['cod'].is_a? String
 => true 
coderhs commented 8 years ago

@vheuken the Open Weather API returns the response as "404" string. So I don't think we should fix it at our end, as thats not the expected behavior. We can let the Open Weather Map, people know about this issue and ask them for their feedback.

coderhs commented 8 years ago

@vheuken any reason why you feel this is an edge case which should be handled?

vheuken commented 8 years ago

any reason why you feel this is an edge case which should be handled?

Consistency. It's what the end user expects.

So I don't think we should fix it at our end, as thats not the expected behavior.

Are you sure about this? If it is explicitly documented saying that a "404" will be a string and a 200 will be an integer, I can see where you're coming from. I don't think it's beyond the scope of an API wrapper to work around API bugs and "gotchyas", though. After all, if I wanted to deal with API quirks, I'd just interact with the API directly.

We can let the Open Weather Map, people know about this issue and ask them for their feedback.

This would be a good idea. I will do that this weekend.

coderhs commented 8 years ago

@vheuken I mailed them yesterday and I got this reply from them.

Dear Mr. Harisankar,

thank you for using our services and reporting the problem! I will add this to our internal bug tracker. Sorry for temporary inconvenience. Best regards, Ivan Mashchenko

Regarding this:

Consistency. It's what the end user expects.

I am kinda biased here, if a guy who has already been working with the API, uses our wrapper and expects a string 404 and gets an integer 404, then it would be an issue.

As a wrapper we just need to let developers communicate with an API through Ruby syntax, and leave the rest to the API. Unless its a huge usability issues with the API.

So I feel it would be better for the API to fix it, since they have added it to their Internal Bug Tracker. Hopefully the developers there will fix it, or they will let us know why it can not be changed.

vheuken commented 8 years ago

if a guy who has already been working with the API, uses our wrapper and expects a string 404 and gets an integer 404, then it would be an issue.

While I doubt that that would be a real issue in practice, I think your stance on the responsibilities of an API is reasonable. Hopefully they end up fixing this on their end.