From 1947f26d9b4213a9274e6409a09230cb33660ed2 Mon Sep 17 00:00:00 2001 From: Abe Tomoaki Date: Tue, 15 Oct 2024 16:59:22 +0900 Subject: [PATCH 1/5] response: parse: Include input JSON in Error, on JSON parsing errors For example, when the input is `["header", :{"return_code":-77}}`, a parse error. The error is then as follows. ``` `parse': unexpected token at ':{"return_code":-77}}' (JSON::ParserError) ``` Include the input JSON in the error since the error does not include the full input JSON, making it difficult to debug. --- lib/groonga/client/response/base.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/groonga/client/response/base.rb b/lib/groonga/client/response/base.rb index 2bb8f19..317a2b3 100644 --- a/lib/groonga/client/response/base.rb +++ b/lib/groonga/client/response/base.rb @@ -71,11 +71,16 @@ def parse(command, raw_response) case command.output_type when :json callback = command["callback"] + json_parse = -> (json) do + JSON.parse(json) + rescue JSON::ParserError => err + raise err.exception("source=`#{json}`") + end if callback and /\A#{Regexp.escape(callback)}\((.+)\);\z/ =~ raw_response - response = JSON.parse($1) + response = json_parse.call($1) else - response = JSON.parse(raw_response) + response = json_parse.call(raw_response) end if response.is_a?(::Array) header, body = response From 8bf95bacf3eb490cff11c10414bab81b2c1b32de Mon Sep 17 00:00:00 2001 From: Abe Tomoaki Date: Tue, 15 Oct 2024 17:51:31 +0900 Subject: [PATCH 2/5] Raise InvalidResponse --- lib/groonga/client/response/base.rb | 16 +++++++++------- lib/groonga/client/response/error.rb | 16 ++++++++++++++++ test/response/test-base.rb | 14 ++++++++++++++ 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/lib/groonga/client/response/base.rb b/lib/groonga/client/response/base.rb index 317a2b3..696195f 100644 --- a/lib/groonga/client/response/base.rb +++ b/lib/groonga/client/response/base.rb @@ -71,16 +71,18 @@ def parse(command, raw_response) case command.output_type when :json callback = command["callback"] - json_parse = -> (json) do - JSON.parse(json) - rescue JSON::ParserError => err - raise err.exception("source=`#{json}`") - end if callback and /\A#{Regexp.escape(callback)}\((.+)\);\z/ =~ raw_response - response = json_parse.call($1) + json = $1 else - response = json_parse.call(raw_response) + json = raw_response + end + begin + response = JSON.parse(json) + rescue JSON::ParserError => error + raise InvalidResponse.new(command, + raw_response, + "invalid JSON: #{error}") end if response.is_a?(::Array) header, body = response diff --git a/lib/groonga/client/response/error.rb b/lib/groonga/client/response/error.rb index 731c347..6381b87 100644 --- a/lib/groonga/client/response/error.rb +++ b/lib/groonga/client/response/error.rb @@ -14,6 +14,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +require "groonga/client/error" require "groonga/client/response/base" module Groonga @@ -66,6 +67,21 @@ def line end end end + + class InvalidResponse < Client::Error + attr_reader :command + attr_reader :raw_response + def initialize(command, raw_response, error_message) + @command = command + @raw_response = raw_response + message = +"invalid response: " + message << "#{command.command_name}: " + message << "#{error_message}: " + message << command.to_command_format + message << ": <#{raw_response}>" + super(message) + end + end end end end diff --git a/test/response/test-base.rb b/test/response/test-base.rb index 8d544f4..08d316a 100644 --- a/test/response/test-base.rb +++ b/test/response/test-base.rb @@ -222,5 +222,19 @@ def test_jsonp response = Groonga::Client::Response::Base.parse(command, raw_response) assert_equal(1396012478, response.body["start_time"]) end + + def test_invalid_json + command = Groonga::Command::Base.new("cancel") + raw_response = '["header", :{"return_code":-77}}' + err = assert_raise(Groonga::Client::Response::InvalidResponse) do + Groonga::Client::Response::Base.parse(command, raw_response) + end + error_message = <<~'MESSAGE'.chomp + invalid response: cancel: invalid JSON: unexpected token at ':{"return_code":-77}}': cancel: <["header", :{"return_code":-77}}> + MESSAGE + assert_equal(error_message, err.message) + assert_equal(command, err.command) + assert_equal(raw_response, err.raw_response) + end end end From 983896e6669a5f3f1f1a9a419f26f33fa892786c Mon Sep 17 00:00:00 2001 From: Abe Tomoaki Date: Wed, 16 Oct 2024 08:53:19 +0900 Subject: [PATCH 3/5] Refactoring error related classes --- lib/groonga/client/error.rb | 30 ++++++++++++++++++++++++++++ lib/groonga/client/request/error.rb | 20 +++---------------- lib/groonga/client/response/error.rb | 16 --------------- test/response/test-base.rb | 4 ++-- 4 files changed, 35 insertions(+), 35 deletions(-) diff --git a/lib/groonga/client/error.rb b/lib/groonga/client/error.rb index 6ec96d8..d026cc0 100644 --- a/lib/groonga/client/error.rb +++ b/lib/groonga/client/error.rb @@ -18,5 +18,35 @@ module Groonga class Client class Error < StandardError end + + class ErrorResponse < Error + attr_reader :response + def initialize(response) + @response = response + command = @response.command + status_code = @response.status_code + error_message = @response.error_message + message = "failed to execute: " + message << "#{command.command_name}: #{status_code}: " + message << "<#{error_message}>: " + message << command.to_command_format + super(message) + end + end + + class InvalidResponse < Client::Error + attr_reader :command + attr_reader :raw_response + def initialize(command, raw_response, error_message) + @command = command + @raw_response = raw_response + message = +"invalid response: " + message << "#{command.command_name}: " + message << "#{error_message}: " + message << "<#{command.to_command_format}>: " + message << "<#{raw_response}>" + super(message) + end + end end end diff --git a/lib/groonga/client/request/error.rb b/lib/groonga/client/request/error.rb index e7ffb16..e7678ed 100644 --- a/lib/groonga/client/request/error.rb +++ b/lib/groonga/client/request/error.rb @@ -19,23 +19,9 @@ module Groonga class Client module Request - class Error < Client::Error - end - - class ErrorResponse < Error - attr_reader :response - def initialize(response) - @response = response - command = @response.command - status_code = @response.status_code - error_message = @response.error_message - message = "failed to execute: " - message << "#{command.command_name}: #{status_code}: " - message << "<#{error_message}>: " - message << command.to_command_format - super(message) - end - end + # For backward compatibility + Error = Client::Error + ErrorResponse = Client::ErrorResponse end end end diff --git a/lib/groonga/client/response/error.rb b/lib/groonga/client/response/error.rb index 6381b87..731c347 100644 --- a/lib/groonga/client/response/error.rb +++ b/lib/groonga/client/response/error.rb @@ -14,7 +14,6 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -require "groonga/client/error" require "groonga/client/response/base" module Groonga @@ -67,21 +66,6 @@ def line end end end - - class InvalidResponse < Client::Error - attr_reader :command - attr_reader :raw_response - def initialize(command, raw_response, error_message) - @command = command - @raw_response = raw_response - message = +"invalid response: " - message << "#{command.command_name}: " - message << "#{error_message}: " - message << command.to_command_format - message << ": <#{raw_response}>" - super(message) - end - end end end end diff --git a/test/response/test-base.rb b/test/response/test-base.rb index 08d316a..1506e76 100644 --- a/test/response/test-base.rb +++ b/test/response/test-base.rb @@ -226,11 +226,11 @@ def test_jsonp def test_invalid_json command = Groonga::Command::Base.new("cancel") raw_response = '["header", :{"return_code":-77}}' - err = assert_raise(Groonga::Client::Response::InvalidResponse) do + err = assert_raise(Groonga::Client::InvalidResponse) do Groonga::Client::Response::Base.parse(command, raw_response) end error_message = <<~'MESSAGE'.chomp - invalid response: cancel: invalid JSON: unexpected token at ':{"return_code":-77}}': cancel: <["header", :{"return_code":-77}}> + invalid response: cancel: invalid JSON: unexpected token at ':{"return_code":-77}}': : <["header", :{"return_code":-77}}> MESSAGE assert_equal(error_message, err.message) assert_equal(command, err.command) From 8351ee775793e2389e78964f5ff66b075b74cf36 Mon Sep 17 00:00:00 2001 From: Abe Tomoaki Date: Wed, 16 Oct 2024 09:04:16 +0900 Subject: [PATCH 4/5] Improve the test --- test/response/test-base.rb | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/response/test-base.rb b/test/response/test-base.rb index 1506e76..623a758 100644 --- a/test/response/test-base.rb +++ b/test/response/test-base.rb @@ -226,15 +226,16 @@ def test_jsonp def test_invalid_json command = Groonga::Command::Base.new("cancel") raw_response = '["header", :{"return_code":-77}}' - err = assert_raise(Groonga::Client::InvalidResponse) do + begin + JSON.parse(raw_response) + rescue JSON::ParserError => error + parse_error_message = "invalid JSON: #{error}" + end + error = Groonga::Client::InvalidResponse.new(command, raw_response, parse_error_message) + + assert_raise(error) do Groonga::Client::Response::Base.parse(command, raw_response) end - error_message = <<~'MESSAGE'.chomp - invalid response: cancel: invalid JSON: unexpected token at ':{"return_code":-77}}': : <["header", :{"return_code":-77}}> - MESSAGE - assert_equal(error_message, err.message) - assert_equal(command, err.command) - assert_equal(raw_response, err.raw_response) end end end From 35b90cc7ea7b26c6409a9fd2a293b8bbbd357068 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Wed, 16 Oct 2024 09:07:21 +0900 Subject: [PATCH 5/5] Remove redundant `Client::` --- lib/groonga/client/error.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/groonga/client/error.rb b/lib/groonga/client/error.rb index d026cc0..0f5ebfd 100644 --- a/lib/groonga/client/error.rb +++ b/lib/groonga/client/error.rb @@ -34,7 +34,7 @@ def initialize(response) end end - class InvalidResponse < Client::Error + class InvalidResponse < Error attr_reader :command attr_reader :raw_response def initialize(command, raw_response, error_message)