Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -59,4 +67,4 @@ AllCops:
- 'spec/**/*_spec.rb'
- 'spec/spec_helper.rb'
- 'Gemfile'
- 'Rakefile'
- 'Rakefile'
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
104 changes: 69 additions & 35 deletions lib/serpapi/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted to satisfy Rubocop

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
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline comment on line 265 between the if and elsif clauses reduces readability. Consider moving it above the elsif statement on line 266, or removing it since the elsif condition is self-explanatory.

Suggested change
# Check for HTTP-level error

Copilot uses AI. Check for mistakes.
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),
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When data is not a Hash (e.g., if the JSON response is an array or primitive value), the fallback data[:error] on line 278 may not behave as expected. While this is unlikely given typical API responses, consider either checking if data is a Hash before accessing data[:error], or simplifying this to just use explicit_error since the fallback case (data[:error]) would be nil anyway when called from line 267 (where we know data doesn't have an :error key or isn't a Hash).

Suggested change
serpapi_error: explicit_error || (data ? data[:error] : nil),
serpapi_error: explicit_error || (data.is_a?(Hash) ? data[:error] : nil),

Copilot uses AI. Check for mistakes.
search_params: params,
response_status: response.status,
search_id: data&.dig(:search_metadata, :id),
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The safe navigation operator &. on line 281 only guards against nil, not against data being a type that doesn't respond to dig (like String or Integer). If the JSON response is unexpectedly a primitive value, this would raise a NoMethodError. Consider wrapping this in a check: data.is_a?(Hash) ? data.dig(:search_metadata, :id) : nil or handling this more defensively.

Suggested change
search_id: data&.dig(:search_metadata, :id),
search_id: data.is_a?(Hash) ? data.dig(:search_metadata, :id) : nil,

Copilot uses AI. Check for mistakes.
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
90 changes: 83 additions & 7 deletions lib/serpapi/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation states that the decoder parameter should be a String, but the actual implementation expects a Symbol. The validate_optional_symbol method on line 37 validates it as a Symbol, and line 13 of the class documentation also describes it as a Symbol. Update the parameter documentation to indicate Symbol instead of String.

Suggested change
# @param decoder [String, nil] optional decoder/format used (e.g. "json")
# @param decoder [Symbol, nil] optional decoder/format used (e.g. :json)

Copilot uses AI. Check for mistakes.
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
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling freeze on a Symbol is unnecessary since symbols are already immutable in Ruby. While this doesn't cause any issues, you can remove the freeze call on line 79 for consistency with Ruby idioms.

Suggested change
value.freeze
value

Copilot uses AI. Check for mistakes.
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
2 changes: 1 addition & 1 deletion lib/serpapi/version.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module SerpApi
# Current version of the gem
VERSION = '1.0.2'.freeze
VERSION = '1.0.3'.freeze
end
6 changes: 4 additions & 2 deletions spec/serpapi/client/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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./)
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new error attributes (serpapi_error, search_params, response_status, search_id, decoder) are not being tested in the updated test expectations. Given the comprehensive testing approach in this test suite and that these new attributes are a core part of the PR's purpose, consider adding assertions to verify that these attributes are properly populated when errors occur. For example, in the 'missing query' test, you could verify that e.response_status is 400 and e.search_params contains the expected parameters.

Copilot uses AI. Check for mistakes.
end

it 'get bad endpoint' do
Expand All @@ -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
Expand Down Expand Up @@ -134,6 +134,8 @@
expect { client.search(q: 'Invalid Query') }.to raise_error(SerpApi::SerpApiError)
end



Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two blank lines were added at the end of this test block. Per Ruby style conventions, there should be only one blank line between test blocks. Remove one of the extra blank lines.

Suggested change

Copilot uses AI. Check for mistakes.
end

describe 'SerpApi client with persitency disabled' do
Expand Down
Loading