Skip to content

Commit

Permalink
Add granular errors for Elasticsearch patch
Browse files Browse the repository at this point in the history
With this change, the granular errors normally raised by Elasticsearch
will be preserved. Previously we were discarding the granular error
information raised by the Elasticsearch client and simply raising
an "Error" subclass. Now we raise a Faulty error which is always a
subclass of whichever error was raised by Elasticsearch itself.

To do this, we add support for procs in the new error_mapper option for
circuits. This allows us to individually map the errors returned by
Elasticsearch.

Add Faulty::Deprecation for deprecation notification

DEPRECATIONS:

- The error_module option is deprecated. Patches should use the error_module
  option instead. The option will be removed in 0.9
  • Loading branch information
justinhoward committed Feb 17, 2022
1 parent a5a3631 commit 4df6e27
Show file tree
Hide file tree
Showing 12 changed files with 218 additions and 29 deletions.
1 change: 1 addition & 0 deletions lib/faulty.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require 'forwardable'
require 'concurrent'

require 'faulty/deprecation'
require 'faulty/immutable_options'
require 'faulty/cache'
require 'faulty/circuit'
Expand Down
40 changes: 32 additions & 8 deletions lib/faulty/circuit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,13 @@ class Circuit # rubocop:disable Metrics/ClassLength
# @!attribute [r] cool_down
# @return [Integer] The number of seconds the circuit will
# stay open after it is tripped. Default 300.
# @!attribute [r] error_module
# @return [Module] Used by patches to set the namespace module for
# the faulty errors that will be raised. Default `Faulty`
# @!attribute [r] error_mapper
# @return [Module, #call] Used by patches to set the namespace module for
# the faulty errors that will be raised. Should be a module or a callable.
# If given a module, the circuit assumes the module has error classes
# in that module. If given an object that responds to `#call` (a proc
# or lambda), the return value of the callable will be used. The callable
# is called with (`error_name`, `cause_error`, `circuit`). Default `Faulty`
# @!attribute [r] evaluation_window
# @return [Integer] The number of seconds of history that
# will be evaluated to determine the failure rate for a circuit.
Expand Down Expand Up @@ -93,6 +97,7 @@ class Circuit # rubocop:disable Metrics/ClassLength
:rate_threshold,
:sample_threshold,
:errors,
:error_mapper,
:error_module,
:exclude,
:cache,
Expand Down Expand Up @@ -120,7 +125,7 @@ def defaults
cache_refreshes_after: 900,
cool_down: 300,
errors: [StandardError],
error_module: Faulty,
error_mapper: Faulty,
exclude: [],
evaluation_window: 60,
rate_threshold: 0.5,
Expand All @@ -133,7 +138,7 @@ def required
cache
cool_down
errors
error_module
error_mapper
exclude
evaluation_window
rate_threshold
Expand All @@ -153,6 +158,17 @@ def finalize
unless cache_refreshes_after.nil?
self.cache_refresh_jitter = 0.2 * cache_refreshes_after
end

deprecated_error_module
end

private

def deprecated_error_module
return unless error_module

Deprecation.method(self.class, :error_module, note: 'See :error_mapper', sunset: '0.9.0')
self.error_mapper = error_module
end
end

Expand Down Expand Up @@ -379,7 +395,7 @@ def push_options
# @return The result from cache if available
def run_skipped(cached_value)
skipped!
raise options.error_module::OpenCircuitError.new(nil, self) if cached_value.nil?
raise map_error(:OpenCircuitError) if cached_value.nil?

cached_value
end
Expand All @@ -400,9 +416,9 @@ def run_exec(status, cached_value, cache_key)
opened = failure!(status, e)
if cached_value.nil?
if opened
raise options.error_module::CircuitTrippedError.new(e.message, self)
raise map_error(:CircuitTrippedError, e)
else
raise options.error_module::CircuitFailureError.new(e.message, self)
raise map_error(:CircuitFailureError, e)
end
else
cached_value
Expand Down Expand Up @@ -531,5 +547,13 @@ def storage

@given_options.storage
end

def map_error(error_name, cause = nil)
if options.error_mapper.respond_to?(:call)
options.error_mapper.call(error_name, cause, self)
else
options.error_mapper.const_get(error_name).new(cause&.message, self)
end
end
end
end
37 changes: 37 additions & 0 deletions lib/faulty/deprecation.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# frozen_string_literal: true

class Faulty
# Support deprecating Faulty features
module Deprecation
class << self
# Call to raise errors instead of logging warnings for Faulty deprecations
def raise_errors!(enabled = true)
@raise_errors = (enabled == true)
end

def silenced
@silence = true
yield
ensure
@silence = false
end

# @private
def method(klass, name, note: nil, sunset: nil)
deprecate("#{klass}##{name}", note: note, sunset: sunset)
end

# @private
def deprecate(subject, note: nil, sunset: nil)
return if @silence

message = "#{subject} is deprecated"
message += " and will be removed in #{sunset}" if sunset
message += " (#{note})" if note
raise DeprecationError, message if @raise_errors

Kernel.warn("DEPRECATION: #{message}")
end
end
end
end
3 changes: 3 additions & 0 deletions lib/faulty/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ def initialize(message = nil)
end
end

class DeprecationError < FaultyError
end

# Included in faulty circuit errors to provide common features for
# native and patched errors
module CircuitErrorBase
Expand Down
11 changes: 6 additions & 5 deletions lib/faulty/patch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,15 @@ class << self
# option and these additional options
# @option hash [String] :name The circuit name. Defaults to `default_name`
# @option hash [Boolean] :patch_errors By default, circuit errors will be
# subclasses of `options[:patched_error_module]`. The user can disable
# subclasses of `options[:patched_error_mapper]`. The user can disable
# this by setting this option to false.
# @option hash [Faulty, String, Symbol, Hash{ constant: String }] :instance
# A reference to a faulty instance. See examples.
# @param options [Hash] Additional override options. Supports any circuit
# option and these additional ones.
# @option options [Module] :patched_error_module The namespace module
# for patched errors
# @option options [Module] :patched_error_mapper The namespace module
# for patched errors or a mapping proc. See {Faulty::Circuit::Options}
# `:error_mapper`
# @yield [Circuit::Options] For setting override options in a block
# @return [Circuit, nil] The circuit if one was created
def circuit_from_hash(default_name, hash, **options, &block)
Expand All @@ -80,8 +81,8 @@ def circuit_from_hash(default_name, hash, **options, &block)
hash = symbolize_keys(hash)
name = hash.delete(:name) || default_name
patch_errors = hash.delete(:patch_errors) != false
error_module = options.delete(:patched_error_module)
hash[:error_module] ||= error_module if error_module && patch_errors
error_mapper = options.delete(:patched_error_mapper)
hash[:error_mapper] ||= error_mapper if error_mapper && patch_errors
faulty = resolve_instance(hash.delete(:instance))
faulty.circuit(name, **hash, **options, &block)
end
Expand Down
30 changes: 28 additions & 2 deletions lib/faulty/patch/elasticsearch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,32 @@ module Patch
module Elasticsearch
include Base

Patch.define_circuit_errors(self, ::Elasticsearch::Transport::Transport::Error)
module Error; end
module SnifferTimeoutError; end
module ServerError; end

# We will freeze this after adding the dynamic error classes
MAPPED_ERRORS = { # rubocop:disable Style/MutableConstant
::Elasticsearch::Transport::Transport::Error => Error,
::Elasticsearch::Transport::Transport::SnifferTimeoutError => SnifferTimeoutError,
::Elasticsearch::Transport::Transport::ServerError => ServerError
}

module Errors
::Elasticsearch::Transport::Transport::ERRORS.each do |_code, klass|
MAPPED_ERRORS[klass] = const_set(klass.name.split('::').last, Module.new)
end
end

MAPPED_ERRORS.freeze
MAPPED_ERRORS.each do |klass, mod|
Patch.define_circuit_errors(mod, klass)
end

ERROR_MAPPER = lambda do |error_name, cause, circuit|
MAPPED_ERRORS.fetch(cause&.class, Error).const_get(error_name).new(cause&.message, circuit)
end
private_constant :ERROR_MAPPER, :MAPPED_ERRORS

def initialize(arguments = {}, &block)
super
Expand All @@ -48,7 +73,8 @@ def initialize(arguments = {}, &block)
'elasticsearch',
arguments[:faulty],
errors: errors,
patched_error_module: Faulty::Patch::Elasticsearch
exclude: ::Elasticsearch::Transport::Transport::Errors::NotFound,
patched_error_mapper: ERROR_MAPPER
)
end

Expand Down
2 changes: 1 addition & 1 deletion lib/faulty/patch/mysql2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def initialize(opts = {})
::Mysql2::Error::ConnectionError,
::Mysql2::Error::TimeoutError
],
patched_error_module: Faulty::Patch::Mysql2
patched_error_mapper: Faulty::Patch::Mysql2
)

super
Expand Down
2 changes: 1 addition & 1 deletion lib/faulty/patch/redis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def initialize(options = {})
::Redis::BaseConnectionError,
BusyError
],
patched_error_module: Faulty::Patch::Redis
patched_error_mapper: Faulty::Patch::Redis
)

super
Expand Down
36 changes: 36 additions & 0 deletions spec/circuit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,22 @@
expect(circuit.options.cool_down).to eq(300)
end

context 'with error_mapper module' do
let(:options) do
{
cache: cache,
error_mapper: custom_error_module,
storage: storage
}
end

it 'raises custom errors' do
expect do
circuit.run { raise 'fail' }
end.to raise_error(custom_error_module::CircuitFailureError)
end
end

context 'with error_module' do
let(:options) do
{
Expand All @@ -288,12 +304,32 @@
}
end

around { |example| Faulty::Deprecation.silenced(&example) }

it 'raises custom errors' do
expect do
circuit.run { raise 'fail' }
end.to raise_error(custom_error_module::CircuitFailureError)
end
end

context 'with error_mapper lambda' do
let(:options) do
{
cache: cache,
error_mapper: lambda do |error_name, cause, circuit|
custom_error_module.const_get(error_name).new("mapped #{cause.message}", circuit)
end,
storage: storage
}
end

it 'raises mapped error' do
expect do
circuit.run { raise 'fail' }
end.to raise_error(custom_error_module::CircuitFailureError, /.*mapped fail/)
end
end
end

context 'with memory storage' do
Expand Down
41 changes: 41 additions & 0 deletions spec/deprecation_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# frozen_string_literal: true

RSpec.describe Faulty::Deprecation do
it 'prints note and sunset version' do
expect(Kernel).to receive(:warn)
.with('DEPRECATION: foo is deprecated and will be removed in 1.0 (Use bar)')
described_class.deprecate(:foo, note: 'Use bar', sunset: '1.0')
end

it 'prints only subject' do
expect(Kernel).to receive(:warn)
.with('DEPRECATION: blah is deprecated')
described_class.deprecate('blah')
end

it 'prints method deprecation' do
expect(Kernel).to receive(:warn)
.with('DEPRECATION: Faulty::Circuit#foo is deprecated and will be removed in 1.0 (Use bar)')
described_class.method(Faulty::Circuit, :foo, note: 'Use bar', sunset: '1.0')
end

context 'with raise_errors!' do
before { described_class.raise_errors! }

after { described_class.raise_errors!(false) }

it 'raises DeprecationError' do
expect { described_class.deprecate('blah') }
.to raise_error(Faulty::DeprecationError, 'blah is deprecated')
end
end

context 'when silenced' do
it 'does not surface deprecations' do
expect(Kernel).not_to receive(:warn)
described_class.silenced do
described_class.deprecate('blah')
end
end
end
end
22 changes: 21 additions & 1 deletion spec/patch/elasticsearch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def build_client(**options)
expect { patched_bad_client.perform_request('GET', '_cluster/state') }
.to raise_error do |error|
expect(error).to be_a(Elasticsearch::Transport::Transport::Error)
expect(error.class).to eq(Faulty::Patch::Elasticsearch::Error::CircuitFailureError)
expect(error).to be_a(Faulty::CircuitErrorBase)
expect(error.cause).to be_a(Faraday::ConnectionFailed)
end
Expand Down Expand Up @@ -48,9 +49,28 @@ def build_client(**options)
it 'raises unpatched errors if configured to' do
expect { bad_client_unpatched_errors.perform_request('GET', '_cluster/state') }
.to raise_error do |error|
expect(error).to be_a(Faulty::CircuitError)
expect(error.class).to eq(Faulty::CircuitFailureError)
expect(error.cause).to be_a(Faraday::ConnectionFailed)
end
expect(faulty.circuit('elasticsearch').status.failure_rate).to eq(1)
end

it 'raises case-specific Elasticsearch errors' do
expect { patched_good_client.perform_request('PUT', '') }
.to raise_error do |error|
expect(error).to be_a(Elasticsearch::Transport::Transport::Errors::MethodNotAllowed)
expect(error.class).to eq(Faulty::Patch::Elasticsearch::Errors::MethodNotAllowed::CircuitFailureError)
expect(error).to be_a(Faulty::CircuitErrorBase)
expect(error.cause.class).to eq(Elasticsearch::Transport::Transport::Errors::MethodNotAllowed)
end
expect(faulty.circuit('elasticsearch').status.failure_rate).to eq(1)
end

it 'ignores 404 errors' do
expect { patched_good_client.perform_request('GET', 'not_an_index') }
.to raise_error do |error|
expect(error.class).to eq(Elasticsearch::Transport::Transport::Errors::NotFound)
end
expect(faulty.circuit('elasticsearch').status.failure_rate).to eq(0)
end
end
Loading

0 comments on commit 4df6e27

Please sign in to comment.