From 3533d2f4c184b177300ed5273127302cb6cfa0c4 Mon Sep 17 00:00:00 2001 From: Karel Minarik Date: Sun, 28 Jan 2018 12:29:20 +0100 Subject: [PATCH] [API] Fixed the "greedy" regex in the `Utils.__rescue_from_not_found` method The previous implementation of the `Utils.__rescue_from_not_found` method matched the exception method on a pattern including `404`, which has led to incorrectly capturing eg. the `Elasticsearch::Transport::Transport::Errors::Conflict` exception, when the document version or ID contained the `404` string. This patch narrows the regex pattern to only look for the "Not Found" string, in order with the original motivation to enable this functionality also in custom clients, not based on the `elasticsearch-transport` Rubygem. It also fixes the `rescue` clause itself, to only catch exceptions descending from `StandardError`, not `Exception`. Tests have been updated accordingly. Closes #490 --- elasticsearch-api/lib/elasticsearch/api/utils.rb | 12 ++++++++---- elasticsearch-api/test/unit/utils_test.rb | 11 ++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/elasticsearch-api/lib/elasticsearch/api/utils.rb b/elasticsearch-api/lib/elasticsearch/api/utils.rb index d92967060e..25f19a2fd2 100644 --- a/elasticsearch-api/lib/elasticsearch/api/utils.rb +++ b/elasticsearch-api/lib/elasticsearch/api/utils.rb @@ -187,8 +187,12 @@ def __extract_parts(arguments, valid_parts=[]) return parts end - # Calls given block, rescuing from any exceptions. Returns `false` - # if exception contains NotFound/404 in its class name or message, else re-raises exception. + # Calls the given block, rescuing from `StandardError`. + # + # Primary use case is the `:ignore` parameter for API calls. + # + # Returns `false` if exception contains NotFound in its class name or message, + # else re-raises the exception. # # @yield [block] A block of code to be executed with exception handling. # @@ -196,8 +200,8 @@ def __extract_parts(arguments, valid_parts=[]) # def __rescue_from_not_found(&block) yield - rescue Exception => e - if e.class.to_s =~ /NotFound/ || e.message =~ /Not\s*Found|404/i + rescue StandardError => e + if e.class.to_s =~ /NotFound/ || e.message =~ /Not\s*Found/i false else raise e diff --git a/elasticsearch-api/test/unit/utils_test.rb b/elasticsearch-api/test/unit/utils_test.rb index db198e20da..35fc343a24 100644 --- a/elasticsearch-api/test/unit/utils_test.rb +++ b/elasticsearch-api/test/unit/utils_test.rb @@ -237,16 +237,13 @@ class UtilsTest < UnitTest end should "return false if exception message contains 'Not Found'" do - assert_equal( false, __rescue_from_not_found { raise Exception.new "Not Found" }) - end - - should "return false if exception message contains '404'" do - assert_equal( false, __rescue_from_not_found { raise Exception.new "404" }) + assert_equal( false, __rescue_from_not_found { raise StandardError.new "Not Found" }) + assert_equal( false, __rescue_from_not_found { raise StandardError.new "NotFound" }) end should "raise exception if exception class name and message do not contain NotFound/404" do - assert_raise Exception do - __rescue_from_not_found { raise Exception.new "Any other exception" } + assert_raise StandardError do + __rescue_from_not_found { raise StandardError.new "Any other exception" } end end