Enhance error reporting #16

ron-at-swgy · 2025-12-08T22:12:08Z

This pull request enhances the SerpApi::SerpApiError class to include additional details:

serpapi_error - The contents of the error attribute of the parsed JSON response object, if available
search_params - The original query params used for the search
response_status - HTTP response status
search_id - The search id, from the search metadata, from the parsed JSON response object, if available
decoder - The decoded used in the request (json or html)

With this additional information, client code can print out the search ID for failed searches with something like:

  rescue SerpApi::SerpApiError => e
    puts "#" * 40
    puts "No results found for query: #{query}"
    puts "Search ID: #{e.search_id}"
    puts "Search Response: #{e.response_status}"
    puts "SerpApi Error: #{e.serpapi_error}"
    puts "#" * 40
  end

This pull request fixes #15

ron-at-swgy · 2025-12-08T23:38:41Z

lib/serpapi/client.rb

+      handle_response(response, decoder, endpoint, params)
+    end
+
+    def execute_request(endpoint, params)


Extracted to satisfy Rubocop

Copilot

Pull request overview

This pull request enhances error reporting in the SerpApi Ruby client by enriching the SerpApiError exception class with contextual attributes (API error message, search parameters, HTTP status, search ID, and decoder type) and refactoring the error handling logic in the client for better maintainability.

Enhanced SerpApiError class with five new optional attributes for detailed error context
Refactored client error handling into focused helper methods with centralized error raising
Updated version to 1.0.3 and adjusted RuboCop configuration to accommodate the changes

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
lib/serpapi/error.rb	Added new error attributes with validation methods and a `to_h` serialization method
lib/serpapi/client.rb	Refactored error handling into separate methods (`execute_request`, `handle_response`, `process_json_response`, `process_html_response`, `raise_http_error`, `raise_parser_error`) and updated all error raising to include new context attributes
spec/serpapi/client/client_spec.rb	Updated test expectations to match new error message format
lib/serpapi/version.rb	Bumped version from 1.0.2 to 1.0.3
README.md.erb	Updated gem version reference to 1.0.3
README.md	Updated gem version reference to 1.0.3
.rubocop.yml	Relaxed `Metrics/ClassLength` and `Metrics/ParameterLists` constraints to accommodate the enhanced error class

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-22T12:54:18Z

lib/serpapi/client.rb

+        serpapi_error: explicit_error || (data ? data[:error] : nil),
+        search_params: params,
+        response_status: response.status,
+        search_id: data&.dig(:search_metadata, :id),


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 · 2025-12-22T12:54:19Z

lib/serpapi/error.rb

+    # @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")


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 · 2025-12-22T12:54:19Z

spec/serpapi/client/client_spec.rb

  end

+
+


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 · 2025-12-22T12:54:19Z

spec/serpapi/client/client_spec.rb

    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./)


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 · 2025-12-22T12:54:19Z

lib/serpapi/error.rb

+      return nil if value.nil?
+      raise TypeError, "expected #{name || 'value'} to be a Symbol, got #{value.class}" unless value.is_a?(Symbol)
+
+      value.freeze


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 · 2025-12-22T12:54:20Z

lib/serpapi/client.rb

+      # 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


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 · 2025-12-22T12:54:20Z

lib/serpapi/client.rb

+
+      raise SerpApiError.new(
+        "#{msg} from url: https://#{BACKEND}#{endpoint}",
+        serpapi_error: explicit_error || (data ? data[:error] : nil),


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),

Ron Dahlgren added 2 commits December 7, 2025 12:50

Enhance SerpApiError, update usage

6c04a55

Decode is a symbol, not a string

8f80716

ron-at-swgy requested a review from andypple83 December 8, 2025 22:12

ron-at-swgy marked this pull request as draft December 8, 2025 22:38

Ron Dahlgren added 2 commits December 8, 2025 18:01

linting fixes

3073549

Update test expectations to match reformated messages

8b798fa

ron-at-swgy marked this pull request as ready for review December 8, 2025 23:38

ron-at-swgy commented Dec 8, 2025

View reviewed changes

Ron Dahlgren added 2 commits December 8, 2025 18:42

Pass decoder through to error object

8986a96

Bump version to 1.0.3

4dfc15b

ron-at-swgy requested a review from Copilot December 22, 2025 12:49

Copilot started reviewing on behalf of ron-at-swgy December 22, 2025 12:49 View session

Copilot AI reviewed Dec 22, 2025

View reviewed changes

andypple83 requested a review from jvmvik December 23, 2025 08:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enhance error reporting #16

Enhance error reporting #16

Uh oh!

ron-at-swgy commented Dec 8, 2025 •

edited

Loading

Uh oh!

ron-at-swgy Dec 8, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Copilot AI Dec 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

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

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

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

Enhance error reporting #16

Are you sure you want to change the base?

Enhance error reporting #16

Uh oh!

Conversation

ron-at-swgy commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ron-at-swgy Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ron-at-swgy commented Dec 8, 2025 •

edited

Loading