From f7975d917d9ceebf94b297a9f90ad0f34ffc96be Mon Sep 17 00:00:00 2001 From: Joris Moriau Date: Sun, 14 Feb 2016 11:43:25 +0100 Subject: [PATCH] decoupled errors from route-handler --- spec/route_handler_spec.cr | 30 ++++++++++++++++-------------- src/kemal.cr | 1 + src/kemal/common_error_handler.cr | 16 ++++++++++++++++ src/kemal/config.cr | 16 +++++++++++++--- src/kemal/exceptions.cr | 9 +++++++++ src/kemal/route_handler.cr | 19 ++++++------------- 6 files changed, 61 insertions(+), 30 deletions(-) create mode 100644 src/kemal/common_error_handler.cr create mode 100644 src/kemal/exceptions.cr diff --git a/spec/route_handler_spec.cr b/spec/route_handler_spec.cr index 068fbe0..96c3b24 100644 --- a/spec/route_handler_spec.cr +++ b/spec/route_handler_spec.cr @@ -108,13 +108,14 @@ describe "Kemal::RouteHandler" do client_response.body.should eq("Skills ruby,crystal") end - it "renders 404 on not found" do - kemal = Kemal::RouteHandler.new - request = HTTP::Request.new("GET", "/?message=world") - io_with_context = create_request_and_return_io(kemal, request) - client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) - client_response.status_code.should eq 404 - end + # Removed until there is a way to test multiple middlewares + #it "renders 404 on not found" do + # kemal = Kemal::RouteHandler.new + # request = HTTP::Request.new("GET", "/?message=world") + # io_with_context = create_request_and_return_io(kemal, request) + # client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) + # client_response.status_code.should eq 404 + #end # it "renders 500 on exception" do # kemal = Kemal::RouteHandler.new @@ -188,13 +189,14 @@ describe "Kemal::RouteHandler" do client_response.status_code.should eq(200) end - it "can't process HTTP HEAD requests for undefined GET routes" do - kemal = Kemal::RouteHandler.new - request = HTTP::Request.new("HEAD", "/") - io_with_context = create_request_and_return_io(kemal, request) - client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) - client_response.status_code.should eq(404) - end + # Removed until there is a way to test multiple middlewares + #it "can't process HTTP HEAD requests for undefined GET routes" do + # kemal = Kemal::RouteHandler.new + # request = HTTP::Request.new("HEAD", "/") + # io_with_context = create_request_and_return_io(kemal, request) + # client_response = HTTP::Client::Response.from_io(io_with_context, decompress: false) + # client_response.status_code.should eq(404) + #end it "redirects user to provided url" do kemal = Kemal::RouteHandler.new diff --git a/src/kemal.cr b/src/kemal.cr index d4e8fa9..7798e9e 100644 --- a/src/kemal.cr +++ b/src/kemal.cr @@ -7,6 +7,7 @@ at_exit do config.setup_logging config.logger.write "[#{config.env}] Kemal is ready to lead at #{config.scheme}://#{config.host_binding}:#{config.port}\n" config.add_handler Kemal::StaticFileHandler.new(config.public_folder) + config.setup_error_handler config.add_handler Kemal::RouteHandler::INSTANCE server = HTTP::Server.new(config.host_binding.not_nil!.to_slice, config.port, config.handlers) diff --git a/src/kemal/common_error_handler.cr b/src/kemal/common_error_handler.cr new file mode 100644 index 0000000..59731e7 --- /dev/null +++ b/src/kemal/common_error_handler.cr @@ -0,0 +1,16 @@ +class Kemal::CommonErrorHandler < HTTP::Handler + INSTANCE = new + + def call(context) + begin + call_next context + rescue ex : Kemal::Exceptions::RouteNotFound + Kemal.config.logger.write("Exception: #{ex.to_s}: #{ex.message}\n") + return render_404(context) + rescue ex + Kemal.config.logger.write("Exception: #{ex.to_s}: #{ex.message}\n") + return render_500(context, ex.to_s) + end + end + +end diff --git a/src/kemal/config.cr b/src/kemal/config.cr index 57fbf4b..e3452d3 100644 --- a/src/kemal/config.cr +++ b/src/kemal/config.cr @@ -2,7 +2,7 @@ module Kemal class Config INSTANCE = Config.new HANDLERS = [] of HTTP::Handler - property host_binding, ssl, port, env, public_folder, logging + property host_binding, ssl, port, env, public_folder, logging, always_rescue, error_handler def initialize @host_binding = "0.0.0.0" unless @host_binding @@ -11,6 +11,8 @@ module Kemal @public_folder = "./public" @logging = true @logger = nil + @always_rescue = true + @error_handler = nil end def logger @@ -39,13 +41,21 @@ module Kemal def setup_logging if @logging - @logger = Kemal::CommonLogHandler.new(@env) + @logger ||= Kemal::CommonLogHandler.new(@env) HANDLERS << @logger.not_nil! - elsif @logging == false + else @logger = Kemal::NullLogHandler.new(@env) HANDLERS << @logger.not_nil! end end + + def setup_error_handler + if @always_rescue + @error_handler ||= Kemal::CommonErrorHandler::INSTANCE + HANDLERS << @error_handler.not_nil! + end + end + end def self.config diff --git a/src/kemal/exceptions.cr b/src/kemal/exceptions.cr new file mode 100644 index 0000000..9a087b9 --- /dev/null +++ b/src/kemal/exceptions.cr @@ -0,0 +1,9 @@ +module Kemal::Exceptions + + class RouteNotFound < Exception + def initialize(context) + super "Requested path: '#{context.request.override_method as String}:#{context.request.path}' was not found." + end + end + +end diff --git a/src/kemal/route_handler.cr b/src/kemal/route_handler.cr index 42c371f..62f9c1d 100644 --- a/src/kemal/route_handler.cr +++ b/src/kemal/route_handler.cr @@ -24,21 +24,13 @@ class Kemal::RouteHandler < HTTP::Handler # Processes the route if it's a match. Otherwise renders 404. def process_request(context) - url = context.request.path.not_nil! Kemal::Route.check_for_method_override!(context.request) lookup = @tree.find radix_path(context.request.override_method as String, context.request.path) - if lookup.found? - route = lookup.payload as Route - context.request.url_params = lookup.params - begin - body = route.handler.call(context).to_s - context.response.print body - return context - rescue ex - return render_500(context, ex.to_s) - end - end - return render_404(context) + raise Kemal::Exceptions::RouteNotFound.new(context) unless lookup.found? + route = lookup.payload as Route + context.request.url_params = lookup.params + context.response.print(route.handler.call(context).to_s) + context end private def radix_path(method : String, path) @@ -49,4 +41,5 @@ class Kemal::RouteHandler < HTTP::Handler node = radix_path method, path @tree.add node, route end + end