Merge pull request #16 from serpapi/enhance-error-reporting

Enhance error reporting

Author: ron-at-swgy

.rubocop.yml

@@ -44,6 +44,14 @@ Style/FetchEnvVar:
 Metrics/CyclomaticComplexity:
   Max: 12
 
+# There is a tradeoff between line length and line count.
+Metrics/ClassLength:
+  Max: 140
+
+# Keyword args are readable.
+Metrics/ParameterLists:
+  CountKeywordArgs: false
+
 # this rule doesn't always work well with Ruby
 Layout/FirstHashElementIndentation:
   Enabled: false
@@ -59,4 +67,4 @@ AllCops:
     - 'spec/**/*_spec.rb'
     - 'spec/spec_helper.rb'
     - 'Gemfile'
-    - 'Rakefile'
+    - 'Rakefile'

README.md

@@ -22,7 +22,7 @@ Other versions, such as Ruby 1.9, Ruby 2.x, and JRuby, are compatible with [lega
 
 ### Bundler
 ```ruby
-gem 'serpapi', '~> 1.0', '>= 1.0.2'
+gem 'serpapi', '~> 1.0', '>= 1.0.3'
 ```
 
 ### Gem 

README.md.erb

@@ -42,7 +42,7 @@ Other versions, such as Ruby 1.9, Ruby 2.x, and JRuby, are compatible with [lega
 
 ### Bundler
 ```ruby
-gem 'serpapi', '~> 1.0', '>= 1.0.2'
+gem 'serpapi', '~> 1.0', '>= 1.0.3'
 ```
 
 ### Gem 

lib/serpapi/client.rb

@@ -215,47 +215,79 @@ def persistent?
     # @param [Hash] params custom search inputs
     # @return [String|Hash] raw HTML or decoded response as JSON / Hash
     def get(endpoint, decoder = :json, params = {})
-      # execute get via open socket
-      response = if persistent?
-                   @socket.get(endpoint, params: query(params))
-                 else
-                   HTTP.timeout(timeout).get("https://#{BACKEND}#{endpoint}", params: query(params))
-                 end
-
-      # decode response using JSON native parser
+      response = execute_request(endpoint, params)
+      handle_response(response, decoder, endpoint, params)
+    end
+
+    def execute_request(endpoint, params)
+      if persistent?
+        @socket.get(endpoint, params: query(params))
+      else
+        url = "https://#{BACKEND}#{endpoint}"
+        HTTP.timeout(timeout).get(url, params: query(params))
+      end
+    end
+
+    def handle_response(response, decoder, endpoint, params)
       case decoder
       when :json
-        # read http response
-        begin
-          # user can turn on/off JSON keys to symbols
-          # this is more memory efficient, but not always needed
-          symbolize_names = params.key?(:symbolize_names) ? params[:symbolize_names] : true
-
-          # parse JSON response with Ruby standard library
-          data = JSON.parse(response.body, symbolize_names: symbolize_names)
-          if data.instance_of?(Hash) && data.key?(:error)
-            raise SerpApiError, "HTTP request failed with error: #{data[:error]} from url: https://#{BACKEND}#{endpoint}, params: #{params}, decoder: #{decoder}, response status: #{response.status} "
-          elsif response.status != 200
-            raise SerpApiError, "HTTP request failed with response status: #{response.status} reponse: #{data} on get url: https://#{BACKEND}#{endpoint}, params: #{params}, decoder: #{decoder}"
-          end
-        rescue JSON::ParserError
-          raise SerpApiError, "JSON parse error: #{response.body} on get url: https://#{BACKEND}#{endpoint}, params: #{params}, decoder: #{decoder}, response status: #{response.status}"
-        end
-
-        # discard response body
-        response.flush if persistent?
-
-        data
+        process_json_response(response, endpoint, params)
       when :html
-        # html decoder
-        if response.status != 200
-          raise SerpApiError, "HTTP request failed with response status: #{response.status} reponse: #{data} on get url: https://#{BACKEND}#{endpoint}, params: #{params}, decoder: #{decoder}"
-        end
-
-        response.body
+        process_html_response(response, endpoint, params)
       else
         raise SerpApiError, "not supported decoder: #{decoder}, available: :json, :html"
       end
     end
+
+    def process_json_response(response, endpoint, params)
+      symbolize = params.fetch(:symbolize_names, true)
+
+      begin
+        data = JSON.parse(response.body, symbolize_names: symbolize)
+        validate_json_content!(data, response, endpoint, params)
+      rescue JSON::ParserError
+        raise_parser_error(response, endpoint, params)
+      end
+
+      response.flush if persistent?
+      data
+    end
+
+    def process_html_response(response, endpoint, params)
+      raise_http_error(response, nil, endpoint, params, decoder: :html) if response.status != 200
+      response.body
+    end
+
+    def validate_json_content!(data, response, endpoint, params)
+      if data.is_a?(Hash) && data.key?(:error)
+        raise_http_error(response, data, endpoint, params, explicit_error: data[:error])
+      elsif response.status != 200
+        raise_http_error(response, data, endpoint, params)
+      end
+    end
+
+    # Centralized error raising to clean up the logic methods
+    def raise_http_error(response, data, endpoint, params, explicit_error: nil, decoder: :json)
+      msg = "HTTP request failed with status: #{response.status}"
+      msg += " error: #{explicit_error}" if explicit_error
+
+      raise SerpApiError.new(
+        "#{msg} from url: https://#{BACKEND}#{endpoint}",
+        serpapi_error: explicit_error || (data.is_a?(Hash) ? data[:error] : nil),
+        search_params: params,
+        response_status: response.status,
+        search_id: data.is_a?(Hash) ? data&.dig(:search_metadata, :id) : nil,
+        decoder: decoder
+      )
+    end
+
+    def raise_parser_error(response, endpoint, params)
+      raise SerpApiError.new(
+        "JSON parse error: #{response.body} on get url: https://#{BACKEND}#{endpoint}",
+        search_params: params,
+        response_status: response.status,
+        decoder: :json
+      )
+    end
   end
 end

lib/serpapi/error.rb

@@ -2,13 +2,81 @@
 # frozen_string_literal: true
 
 module SerpApi
-  # SerpApiError wraps any errors related to the SerpApi client.
+  # Custom error class for SerpApi-related errors.
+  # Inherits from StandardError.
+  # Includes optional attributes for detailed error context.
+  # Attributes:
+  # - serpapi_error: String error message from SerpApi (optional)
+  # - search_params: Hash of search parameters used (optional)
+  # - response_status: Integer HTTP or response status code (optional)
+  # - search_id: String id returned by the service for the search (optional)
+  # - decoder: Symbol representing the decoder/format used (optional) (e.g. :json)
   class SerpApiError < StandardError
-    # List the specific types of errors handled by the Error class.
-    #  - HTTP response errors from SerpApi.com
-    #  - Missing API key
-    #  - Credit limit
-    #  - Incorrect query
-    #  - more ...
+    attr_reader :serpapi_error, :search_params, :response_status, :search_id, :decoder
+
+    # All attributes are optional keyword arguments.
+    #
+    # @param message [String, nil] an optional human message passed to StandardError
+    # @param serpapi_error [String, nil] optional error string coming from SerpAPI
+    # @param search_params [Hash, nil] optional hash of the search parameters used
+    # @param response_status [Integer, nil] optional HTTP or response status code
+    # @param search_id [String, nil] optional id returned by the service for the search
+    # @param decoder [Symbol, nil] optional decoder/format used (e.g. :json)
+    def initialize(message = nil,
+                   serpapi_error: nil,
+                   search_params: nil,
+                   response_status: nil,
+                   search_id: nil,
+                   decoder: nil)
+      super(message)
+
+      @serpapi_error = validate_optional_string(serpapi_error, :serpapi_error)
+      @search_params = search_params.dup
+      @response_status = validate_optional_integer(response_status, :response_status)
+      @search_id = validate_optional_string(search_id, :search_id)
+      @decoder = validate_optional_symbol(decoder, :decoder)
+    end
+
+    # Return a compact hash representation (omits nil values).
+    #
+    # @return [Hash]
+    def to_h
+      {
+        message: message,
+        serpapi_error: serpapi_error,
+        search_params: search_params.dup,
+        response_status: response_status,
+        search_id: search_id,
+        decoder: decoder
+      }.compact
+    end
+
+    private
+
+    def validate_optional_string(value, name = nil)
+      return nil if value.nil?
+      raise TypeError, "expected #{name || 'value'} to be a String, got #{value.class}" unless value.is_a?(String)
+
+      value.freeze
+    end
+
+    def validate_optional_integer(value, name = nil)
+      return nil if value.nil?
+      return value if value.is_a?(Integer)
+
+      # Accept numeric-like strings (e.g. "200") by converting; fail otherwise.
+      begin
+        Integer(value)
+      rescue ArgumentError, TypeError
+        raise TypeError, "expected #{name || 'value'} to be an Integer (or integer-like), got #{value.inspect}"
+      end
+    end
+
+    def validate_optional_symbol(value, name = nil)
+      return nil if value.nil?
+      raise TypeError, "expected #{name || 'value'} to be a Symbol, got #{value.class}" unless value.is_a?(Symbol)
+
+      value
+    end
   end
 end

lib/serpapi/version.rb

@@ -1,4 +1,4 @@
 module SerpApi
   # Current version of the gem
-  VERSION = '1.0.2'.freeze
+  VERSION = '1.0.3'.freeze
 end

spec/serpapi/client/client_spec.rb

@@ -71,9 +71,9 @@
   end
 
   it 'get endpoint error' do
-    expect { 
-      client.send(:get, '/search', :json, {}) 
-    }.to raise_error(SerpApi::SerpApiError).with_message(/HTTP request failed with error: Missing query `q` parameter./)
+    expect {
+      client.send(:get, '/search', :json, {})
+    }.to raise_error(SerpApi::SerpApiError).with_message(/HTTP request failed with status: 400.* error: Missing query `q` parameter./)
   end
 
   it 'get bad endpoint' do
@@ -90,7 +90,7 @@
     begin
       client.send(:get, '/invalid', :html, {})
     rescue SerpApi::SerpApiError => e
-      expect(e.message).to include(/HTTP request failed with response status: 404 Not Found reponse/), "got #{e.message}"
+      expect(e.message).to include("HTTP request failed with status: 404")
     rescue => e
       raise("wrong exception: #{e}")
     end
@@ -133,7 +133,6 @@
     allow(client).to receive(:search).and_raise(SerpApi::SerpApiError)
     expect { client.search(q: 'Invalid Query') }.to raise_error(SerpApi::SerpApiError)
   end
-
 end
 
 describe 'SerpApi client with persitency disabled' do