Merge pull request #768 from spiderpug/raise-response-parse-error

Adds ability to raise error when response parsing fails

lib/geocoder/exceptions.rb

@@ -9,6 +9,14 @@ module Geocoder
   class OverQueryLimitError < Error
   end
 
+  class ResponseParseError < Error
+    attr_reader :response
+
+    def initialize(response)
+      @response = response
+    end
+  end
+
   class RequestDenied < Error
   end
 

lib/geocoder/lookups/base.rb

@@ -182,6 +182,8 @@ module Geocoder
         else
           JSON.parse(data)
         end
+      rescue => err
+        raise_error(ResponseParseError.new(data)) or warn "Geocoding API's response was not valid JSON."
       end
 
       ##
@@ -189,8 +191,6 @@ module Geocoder
       #
       def parse_raw_data(raw_data)
         parse_json(raw_data)
-      rescue
-        warn "Geocoding API's response was not valid JSON."
       end
 
       ##

lib/geocoder/lookups/google.rb

@@ -19,7 +19,8 @@ module Geocoder::Lookup
     private # ---------------------------------------------------------------
 
     def valid_response?(response)
-      status = parse_json(response.body)["status"]
+      json = parse_json(response.body)
+      status = json["status"] if json
       super(response) and ['OK', 'ZERO_RESULTS'].include?(status)
     end
 

lib/geocoder/lookups/okf.rb

@@ -15,7 +15,8 @@ module Geocoder::Lookup
     private # ---------------------------------------------------------------
 
     def valid_response?(response)
-      status = parse_json(response.body)["status"]
+      json = parse_json(response.body)
+      status = json["status"] if json
       super(response) and ['OK', 'ZERO_RESULTS'].include?(status)
     end
 

test/test_helper.rb

@@ -107,6 +107,10 @@ module Geocoder
         raise TimeoutError if query.text == "timeout"
         raise SocketError if query.text == "socket_error"
         raise Errno::ECONNREFUSED if query.text == "connection_refused"
+        if query.text == "invalid_json"
+          return MockHttpResponse.new(:body => 'invalid json', :code => 200)
+        end
+
         read_fixture fixture_for_query(query)
       end
     end
@@ -409,4 +413,8 @@ class MockHttpResponse
     @code = options[:code].to_s
     @body = options[:body]
   end
+
+  def [](key)
+    send key if respond_to?(key)
+  end
 end

test/unit/error_handling_test.rb

@@ -18,6 +18,29 @@ class ErrorHandlingTest < GeocoderTestCase
     end
   end
 
+  def test_always_raise_response_parse_error
+    Geocoder.configure(:always_raise => [Geocoder::ResponseParseError])
+    [:freegeoip, :google, :okf].each do |l|
+      lookup = Geocoder::Lookup.get(l)
+      set_api_key!(l)
+      assert_raises Geocoder::ResponseParseError do
+        lookup.send(:results, Geocoder::Query.new("invalid_json"))
+      end
+    end
+  end
+
+  def test_never_raise_response_parse_error
+    [:freegeoip, :google, :okf].each do |l|
+      lookup = Geocoder::Lookup.get(l)
+      set_api_key!(l)
+      silence_warnings do
+        assert_nothing_raised do
+          lookup.send(:results, Geocoder::Query.new("invalid_json"))
+        end
+      end
+    end
+  end
+
   def test_always_raise_timeout_error
     Geocoder.configure(:always_raise => [TimeoutError])
     Geocoder::Lookup.all_services_except_test.each do |l|