diff --git a/.rubocop.yml b/.rubocop.yml index bfce55a..6648879 100644 --- a/.rubocop.yml +++ b/.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' \ No newline at end of file + - 'Rakefile' diff --git a/README.md b/README.md index b1d0247..665070b 100644 --- a/README.md +++ b/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 diff --git a/README.md.erb b/README.md.erb index 625e08d..88300b9 100644 --- a/README.md.erb +++ b/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 diff --git a/lib/serpapi/client.rb b/lib/serpapi/client.rb index ec5fe76..e6944ce 100644 --- a/lib/serpapi/client.rb +++ b/lib/serpapi/client.rb @@ -215,47 +215,81 @@ 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) + # Check for API-level error inside the JSON + if data.is_a?(Hash) && data.key?(:error) + raise_http_error(response, data, endpoint, params, explicit_error: data[:error]) + # Check for HTTP-level 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 ? data[:error] : nil), + search_params: params, + response_status: response.status, + search_id: data&.dig(:search_metadata, :id), + 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 diff --git a/lib/serpapi/error.rb b/lib/serpapi/error.rb index 6334392..2e935e6 100644 --- a/lib/serpapi/error.rb +++ b/lib/serpapi/error.rb @@ -2,13 +2,89 @@ # 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 [String, 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 = freeze_hash(search_params) + @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, + 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.freeze + end + + def freeze_hash(value) + return nil if value.nil? + raise TypeError, "expected search_params to be a Hash, got #{value.class}" unless value.is_a?(Hash) + + # duplicate and freeze to avoid accidental external mutation + value.dup.freeze + end end end diff --git a/lib/serpapi/version.rb b/lib/serpapi/version.rb index 954b930..23f6be0 100644 --- a/lib/serpapi/version.rb +++ b/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 diff --git a/spec/serpapi/client/client_spec.rb b/spec/serpapi/client/client_spec.rb index 52f1c5c..b74b00c 100644 --- a/spec/serpapi/client/client_spec.rb +++ b/spec/serpapi/client/client_spec.rb @@ -73,7 +73,7 @@ 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./) + }.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 @@ -134,6 +134,8 @@ expect { client.search(q: 'Invalid Query') }.to raise_error(SerpApi::SerpApiError) end + + end describe 'SerpApi client with persitency disabled' do