Skip to content

Commit

Permalink
[API] Fixed the "greedy" regex in the Utils.__rescue_from_not_found
Browse files Browse the repository at this point in the history
… 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

(cherry picked from commit 3533d2f)
  • Loading branch information
karmiq committed Jan 28, 2018
1 parent cc5b446 commit 6383bbd
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 11 deletions.
12 changes: 8 additions & 4 deletions elasticsearch-api/lib/elasticsearch/api/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,17 +187,21 @@ 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.
#
# @api private
#
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
Expand Down
11 changes: 4 additions & 7 deletions elasticsearch-api/test/unit/utils_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -233,16 +233,13 @@ class UtilsTest < ::Test::Unit::TestCase
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

Expand Down

0 comments on commit 6383bbd

Please sign in to comment.