Skip to content

Commit

Permalink
Replace white/blacklists with error handler
Browse files Browse the repository at this point in the history
This will fix #91. It still needs some tests.
  • Loading branch information
tfausak committed Apr 26, 2016
1 parent 079330f commit 0ce5cdd
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 246 deletions.
59 changes: 13 additions & 46 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,64 +119,32 @@ a while. (The yellow state corresponds to the half open state for circuit

### Custom errors

#### Whitelisted errors

Some errors shouldn't cause your stoplight to move into the red state. Usually
these are handled elsewhere in your stack and don't represent real failures. A
good example is `ActiveRecord::RecordNotFound`.

``` rb
light = Stoplight('example-not-found') { User.find(123) }
.with_whitelisted_errors([ActiveRecord::RecordNotFound])
# => #<Stoplight::Light:...>
light.run
# ActiveRecord::RecordNotFound: Couldn't find User with ID=123
light.run
# ActiveRecord::RecordNotFound: Couldn't find User with ID=123
light.run
# ActiveRecord::RecordNotFound: Couldn't find User with ID=123
light.color
# => "green"
```

The following errors are always whitelisted: `NoMemoryError`, `ScriptError`,
`SecurityError`, `SignalException`, `SystemExit`, and `SystemStackError`.
To prevent some errors from changing the state of your stoplight, you can
provide a custom `Proc` that will be called with the error and a handler
`Proc`. It can do one of three things:

Whitelisted errors take precedence over [blacklisted errors](#blacklisted-errors).
1. Do nothing.

#### Blacklisted errors
2. Call the handler with the error.

You may want only certain errors to cause your stoplight to move into the red
state.
3. Re-raise the error.

``` rb
light = Stoplight('example-blacklist-zero') { 1 / 0 }
.with_blacklisted_errors([ZeroDivisionError])
# => #<Stoplight::Light:...>
light.run
# ZeroDivisionError: divided by 0
light.run
# ZeroDivisionError: divided by 0
light.run
# Switching example-blacklist-zero from green to red because ZeroDivisionError divided by 0
# ZeroDivisionError: divided by 0
light.color
# => "red"
```

This will cause all other errors to be raised normally. They won't affect the
state of your stoplight.

``` rb
light = Stoplight('example-blacklist-fail') { fail }
.with_blacklisted_errors([ZeroDivisionError])
light = Stoplight('example-not-found') { User.find(123) }
.with_error_handler do |error, handler|
handler.call(error) unless error.is_a?(ActiveRecord::RecordNotFound)
end
# => #<Stoplight::Light:...>
light.run
# RuntimeError:
# ActiveRecord::RecordNotFound: Couldn't find User with ID=123
light.run
# RuntimeError:
# ActiveRecord::RecordNotFound: Couldn't find User with ID=123
light.run
# RuntimeError:
# ActiveRecord::RecordNotFound: Couldn't find User with ID=123
light.color
# => "green"
```
Expand Down Expand Up @@ -268,7 +236,6 @@ class ApplicationController < ActionController::Base

def stoplight(&block)
Stoplight("#{params[:controller]}##{params[:action]}", &block)
.with_whitelisted_errors([ActiveRecord::RecordNotFound])
.with_fallback do |error|
Rails.logger.error(error)
render(nothing: true, status: :service_unavailable)
Expand Down
13 changes: 2 additions & 11 deletions lib/stoplight/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,10 @@

module Stoplight
module Default
WHITELISTED_ERRORS = [
NoMemoryError,
ScriptError,
SecurityError,
SignalException,
SystemExit,
SystemStackError
].freeze

BLACKLISTED_ERRORS = [].freeze

DATA_STORE = DataStore::Memory.new

ERROR_HANDLER = -> (error, handler) { handler.call(error) }

ERROR_NOTIFIER = -> (error) { warn error }

FALLBACK = nil
Expand Down
13 changes: 13 additions & 0 deletions lib/stoplight/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@

module Stoplight
module Error
HANDLER = lambda do |error|
raise error if AVOID_RESCUING.any? { |klass| error.is_a?(klass) }
end

AVOID_RESCUING = [
NoMemoryError,
ScriptError,
SecurityError,
SignalException,
SystemExit,
SystemStackError
].freeze

Base = Class.new(StandardError)
RedLight = Class.new(Base)
end
Expand Down
31 changes: 10 additions & 21 deletions lib/stoplight/light.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@ module Stoplight
class Light # rubocop:disable Style/Documentation
include Runnable

# @return [Array<Exception>]
attr_reader :whitelisted_errors
# @return [Array<Exception>]
attr_reader :blacklisted_errors
# @return [Proc]
attr_reader :code
# @return [DataStore::Base]
attr_reader :data_store
# @return [Proc]
attr_reader :error_handler
# @return [Proc]
attr_reader :error_notifier
# @return [Proc, nil]
attr_reader :fallback
Expand Down Expand Up @@ -44,36 +42,27 @@ def initialize(name, &code)
@name = name
@code = code

@whitelisted_errors = Default::WHITELISTED_ERRORS
@blacklisted_errors = Default::BLACKLISTED_ERRORS
@data_store = self.class.default_data_store
@error_handler = Default::ERROR_HANDLER
@error_notifier = self.class.default_error_notifier
@fallback = Default::FALLBACK
@notifiers = self.class.default_notifiers
@threshold = Default::THRESHOLD
@timeout = Default::TIMEOUT
end

# @param whitelisted_errors [Array<Exception>]
# @return [self]
def with_whitelisted_errors(whitelisted_errors)
@whitelisted_errors = Default::WHITELISTED_ERRORS + whitelisted_errors
self
end

alias with_allowed_errors with_whitelisted_errors

# @param blacklisted_errors [Array<Exception>]
# @param data_store [DataStore::Base]
# @return [self]
def with_blacklisted_errors(blacklisted_errors)
@blacklisted_errors = Default::BLACKLISTED_ERRORS + blacklisted_errors
def with_data_store(data_store)
@data_store = data_store
self
end

# @param data_store [DataStore::Base]
# @yieldparam error [Exception]
# @yieldparam handle [Proc]
# @return [self]
def with_data_store(data_store)
@data_store = data_store
def with_error_handler(&error_handler)
@error_handler = error_handler
self
end

Expand Down
10 changes: 2 additions & 8 deletions lib/stoplight/light/runnable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,12 @@ def run_code(on_success, on_failure)
failures = clear_failures
on_success.call(failures) if on_success
result
rescue *[StandardError].concat(blacklisted_errors) => error
rescue Exception => error # rubocop:disable Lint/RescueException
handle_error(error, on_failure)
end

def not_blacklisted_error?(error)
!blacklisted_errors.empty? &&
blacklisted_errors.none? { |klass| error.is_a?(klass) }
end

def handle_error(error, on_failure)
raise error if whitelisted_errors.any? { |klass| error.is_a?(klass) }
raise error if not_blacklisted_error?(error)
error_handler.call(error, Error::HANDLER)
size = record_failure(error)
on_failure.call(size, error) if on_failure
raise error unless fallback
Expand Down
36 changes: 8 additions & 28 deletions spec/stoplight/default_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,39 +7,19 @@
expect(described_class).to be_a(Module)
end

describe '::WHITELISTED_ERRORS' do
it 'is an array' do
expect(Stoplight::Default::WHITELISTED_ERRORS).to be_an(Array)
end

it 'contains exception classes' do
Stoplight::Default::WHITELISTED_ERRORS.each do |whitelisted_error|
expect(whitelisted_error).to be < Exception
end
end

it 'is frozen' do
expect(Stoplight::Default::WHITELISTED_ERRORS).to be_frozen
describe '::DATA_STORE' do
it 'is a data store' do
expect(Stoplight::Default::DATA_STORE).to be_a(Stoplight::DataStore::Base)
end
end

describe '::BLACKLISTED_ERRORS' do
it 'is an array' do
expect(Stoplight::Default::BLACKLISTED_ERRORS).to be_an(Array)
end

it 'is empty' do
expect(Stoplight::Default::BLACKLISTED_ERRORS).to be_empty
end

it 'is frozen' do
expect(Stoplight::Default::BLACKLISTED_ERRORS).to be_frozen
describe '::ERROR_HANDLER' do
it 'is a proc' do
expect(Stoplight::Default::ERROR_HANDLER).to be_a(Proc)
end
end

describe '::DATA_STORE' do
it 'is a data store' do
expect(Stoplight::Default::DATA_STORE).to be_a(Stoplight::DataStore::Base)
it 'has an arity of 2' do
expect(Stoplight::Default::ERROR_HANDLER.arity).to eql(2)
end
end

Expand Down
107 changes: 18 additions & 89 deletions spec/stoplight/light/runnable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,105 +119,34 @@ def random_string
expect(io.string).to_not eql('')
end

context 'when the error is whitelisted' do
let(:whitelisted_errors) { [error.class] }

before { subject.with_whitelisted_errors(whitelisted_errors) }

it 'does not record the failure' do
expect(subject.data_store.get_failures(subject).size).to eql(0)
begin
subject.run
rescue error.class
nil
end
expect(subject.data_store.get_failures(subject).size).to eql(0)
end
end

context 'when the error is blacklisted' do
let(:blacklisted_errors) { [error.class] }

before { subject.with_blacklisted_errors(blacklisted_errors) }

it 'records the failure' do
expect(subject.data_store.get_failures(subject).size).to eql(0)
begin
subject.run
rescue error.class
nil
end
expect(subject.data_store.get_failures(subject).size).to eql(1)
end
end

context 'when the error is a blacklisted of Exception class' do
let(:error_class) { Class.new(Exception) }
let(:blacklisted_errors) { [error.class] }

before { subject.with_blacklisted_errors(blacklisted_errors) }

it 'records the failure' do
expect(subject.data_store.get_failures(subject).size).to eql(0)
begin
subject.run
rescue error.class
nil
end
expect(subject.data_store.get_failures(subject).size).to eql(1)
end
end

context 'when the error is not blacklisted' do
let(:blacklisted_errors) { [RuntimeError] }

before { subject.with_blacklisted_errors(blacklisted_errors) }

it 'does not record the failure' do
expect(subject.data_store.get_failures(subject).size).to eql(0)
context 'with an error handler' do
let(:result) do
begin
subject.run
expect(false).to be(true)
rescue error.class
nil
expect(true).to be(true)
end
expect(subject.data_store.get_failures(subject).size).to eql(0)
end
end

context 'when the list of blacklisted errors is empty' do
let(:blacklisted_errors) { [] }

before { subject.with_blacklisted_errors(blacklisted_errors) }

it 'records the failure' do
expect(subject.data_store.get_failures(subject).size).to eql(0)
begin
subject.run
rescue error.class
nil
end
expect(subject.data_store.get_failures(subject).size).to eql(1)
it 'records the failure when the handler does nothing' do
subject.with_error_handler { |_error, _handler| }
expect { result }
.to change { subject.data_store.get_failures(subject).size }
.by(1)
end
end

context 'when the error is both whitelisted and blacklisted' do
let(:whitelisted_errors) { [error.class] }
let(:blacklisted_errors) { [error.class] }

before do
subject
.with_whitelisted_errors(whitelisted_errors)
.with_blacklisted_errors(blacklisted_errors)
it 'records the failure when the handler calls handle' do
subject.with_error_handler { |error, handle| handle.call(error) }
expect { result }
.to change { subject.data_store.get_failures(subject).size }
.by(1)
end

it 'does not record the failure' do
expect(subject.data_store.get_failures(subject).size).to eql(0)
begin
subject.run
rescue error.class
nil
end
expect(subject.data_store.get_failures(subject).size).to eql(0)
it 'does not record the failure when the handler raises' do
subject.with_error_handler { |error, _handle| raise error }
expect { result }
.to_not change { subject.data_store.get_failures(subject).size }
end
end

Expand Down

0 comments on commit 0ce5cdd

Please sign in to comment.