Skip to content

Commit

Permalink
Add dont_report for errors
Browse files Browse the repository at this point in the history
  • Loading branch information
paulcsmith committed Sep 16, 2019
1 parent 3c6495e commit a60c804
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 22 deletions.
17 changes: 0 additions & 17 deletions spec/lucky/error_action_spec.cr

This file was deleted.

35 changes: 34 additions & 1 deletion spec/lucky/error_handling_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,20 @@ include ContextHelper
private class CustomError < Exception
end

private class StealthError < Exception
end

private class UnhandledError < Exception
end

private class FakeErrorAction < Lucky::ErrorAction
# Can't be private because if it is Crystal won't let us use temp_config
class FakeErrorAction < Lucky::ErrorAction
default_format :html
dont_report [StealthError]

Habitat.create do
setting output : IO = IO::Memory.new
end

def render(error : CustomError) : Lucky::Response
head status: 404
Expand All @@ -18,9 +27,33 @@ private class FakeErrorAction < Lucky::ErrorAction
def render(error : Exception) : Lucky::Response
plain_text "This is not a debug page", status: 500
end

def report(error : Exception)
settings.output.print("Reported: #{error.class.name}")
end
end

describe "Error handling" do
describe "reporting" do
it "calls the report method on the error action" do
io = IO::Memory.new
FakeErrorAction.temp_config(output: io) do
handle_error(error: CustomError.new) do |_context, _output|
io.to_s.should eq("Reported: CustomError")
end
end
end

it "can skip reporting some errors" do
io = IO::Memory.new
FakeErrorAction.temp_config(output: io) do
handle_error(error: StealthError.new) do |_context, _output|
io.to_s.should eq("")
end
end
end
end

describe "ErrorAction" do
describe "show_debug_output setting is true" do
it "renders debug output if request accepts HTML" do
Expand Down
6 changes: 6 additions & 0 deletions src/lucky/action.cr
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,10 @@ abstract class Lucky::Action
include Lucky::ActionCallbacks
include Lucky::Redirectable
include Lucky::VerifyAcceptsFormat

# Must be defined here instead of in Renderable
# Otherwise it clashes with ErrorAction#render
private def render(page_class, **named_args)
{% raise "'render' in actions has been renamed to 'html'" %}
end
end
19 changes: 19 additions & 0 deletions src/lucky/error_action.cr
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,20 @@ abstract class Lucky::ErrorAction

getter context

def _dont_report
[] of Exception.class
end

macro dont_report(exception_classes)
{% if exception_classes.is_a?(ArrayLiteral) %}
def _dont_report
{{ exception_classes }} of Exception.class
end
{% else %}
{% exception_classes.raise "dont_report expects an array of Exception classes." %}
{% end %}
end

def initialize(@context : HTTP::Server::Context)
end

Expand All @@ -20,6 +34,7 @@ abstract class Lucky::ErrorAction
class_getter _accepted_formats = [] of Symbol

abstract def render(error : Exception) : Lucky::Response
abstract def report(error : Exception) : Nil

def perform_action(error : Exception)
# Always get the rendered error because it also includes the HTTP status.
Expand All @@ -32,6 +47,10 @@ abstract class Lucky::ErrorAction
end

response.print

if !_dont_report.includes?(error.class)
report(error)
end
end

private def ensure_response_is_returned(response : Lucky::Response) : Lucky::Response
Expand Down
4 changes: 0 additions & 4 deletions src/lucky/renderable.cr
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@ module Lucky::Renderable
)
end

private def render(*args, **named_args)
{% raise "'render' in actions has been renamed to 'html'" %}
end

macro validate_page_class!(page_class)
{% if page_class && page_class.resolve? %}
{% ancestors = page_class.resolve.ancestors %}
Expand Down

0 comments on commit a60c804

Please sign in to comment.