Merge pull request #92 from jmoriau/decouple-error-hanlder

decoupled errors from route-handler

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

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)

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

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

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

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?
+    raise Kemal::Exceptions::RouteNotFound.new(context) unless 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)
+    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