Mailing-List: contact dev-help@deltacloud.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@deltacloud.apache.org
Received-SPF: pass (athena.apache.org: domain of mfojtik@redhat.com designates
 209.132.183.28 as permitted sender)
From: mfojtik@redhat.com
To: dev@deltacloud.apache.org
Subject: [PATCH core 1/2] Core: Various fixed in logging errors
Date: Mon,  8 Oct 2012 16:13:58 +0200
Message-Id: <1349705639-23190-1-git-send-email-mfojtik@redhat.com>

From: Michal Fojtik <mfojtik@redhat.com>

* 404 is not longer logged as 'error'
* Using -L option on deltacloudd should redirec *all* logs into
  specified file
* 404 error page in UI does not longer print super-long message

Signed-off-by: Michal fojtik <mfojtik@redhat.com>
---
 server/lib/deltacloud/drivers/exceptions.rb        |    8 +++--
 server/lib/deltacloud/helpers/deltacloud_helper.rb |   33 ++++++++++++++++----
 server/lib/sinatra/rack_logger.rb                  |    5 +++
 server/views/errors/404.html.haml                  |    5 ++-
 4 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/server/lib/deltacloud/drivers/exceptions.rb b/server/lib/deltacloud/drivers/exceptions.rb
index dba94b6..c855129 100644
--- a/server/lib/deltacloud/drivers/exceptions.rb
+++ b/server/lib/deltacloud/drivers/exceptions.rb
@@ -115,6 +115,7 @@ module Deltacloud
       def initialize(conditions, &block)
         @conditions = conditions
         instance_eval(&block) if block_given?
+        self
       end
 
       def status(code)
@@ -157,6 +158,10 @@ module Deltacloud
 
     end
 
+    def self.exception_from_status(code, message)
+      ExceptionDef.new(nil) { status code}.handler(StandardError.new(message))
+    end
+
     class Exceptions
       attr_reader :exception_definitions
 
@@ -198,17 +203,14 @@ module Deltacloud
       begin
         block.call
       rescue => e
-        log = ExceptionHandler.logger
         self.class.exceptions.each do |definitions|
           next unless definitions.any? e
           if (new_exception = definitions.handler(e)) and new_exception.message
             message = new_exception.message
           end
           message ||= e.message
-          log.error "#{[e.class.to_s, message].join(':')}\n#{e.backtrace[0..10].join("\n")}" unless ENV['RACK_ENV'] == 'test'
           raise definitions.handler(e) unless new_exception.nil?
         end
-        log.error "[NO HANDLED] #{[e.class.to_s, e.message].join(': ')}\n#{e.backtrace.join("\n")}" unless ENV['RACK_ENV'] == 'test'
         raise BackendError.new(e, "Unhandled exception or status code (#{e.message})")
       end
     end
diff --git a/server/lib/deltacloud/helpers/deltacloud_helper.rb b/server/lib/deltacloud/helpers/deltacloud_helper.rb
index ab4a0c0..2dfd01b 100644
--- a/server/lib/deltacloud/helpers/deltacloud_helper.rb
+++ b/server/lib/deltacloud/helpers/deltacloud_helper.rb
@@ -18,6 +18,10 @@ module Deltacloud::Helpers
 
     require 'benchmark'
 
+    def request_headers
+      env.inject({}){|acc, (k,v)| acc[$1.downcase] = v if k =~ /^http_(.*)/i; acc}
+    end
+
     def auth_feature_name
       return 'key' if driver.class.has_feature?(:instances, :authentication_key)
       return 'password' if driver.class.has_feature?(:instances, :authentication_password)
@@ -77,15 +81,32 @@ module Deltacloud::Helpers
       end
     end
 
+    def log
+      Deltacloud::ExceptionHandler.logger(Deltacloud.default_frontend.logger)
+    end
+
     def report_error(code=nil)
-      @error, @code = (request.env['sinatra.error'] || @exception), code
-      @code = 500 if not @code and not @error.class.method_defined? :code
-      response.status = @code || @error.code
-      log = Deltacloud::ExceptionHandler.logger
-      message = @error.respond_to?(:message) ? @error.message : translate_error_code(@code)
+      handler = Deltacloud::ExceptionHandler
+
+      if !code.nil?
+        @error = handler.exception_from_status(code, translate_error_code(code)[:message])
+        @code = code
+        message = @error.message
+      else
+        @error = request.env['sinatra.error'] || @exception
+        @code = @error.respond_to?(:code) ? @error.code : 500
+        message = @error.respond_to?(:message) ? @error.message : translate_error_code(code)[:message]
+      end
+
+      response.status = @code
+
       backtrace = (@error.respond_to?(:backtrace) and !@error.backtrace.nil?) ?
         "\n\n#{@error.backtrace[0..10].join("\n")}\n\n" : ''
-      log.error "[#{@code}] #{[@error.class.to_s, message].join(':')}#{backtrace}"
+
+      if @code.to_s =~ /5(\d+)/
+        log.error(@code.to_s) { "[#{@error.class.to_s}] #{message}#{backtrace}" }
+      end
+
       respond_to do |format|
         format.xml {  haml :"errors/#{@code || @error.code}", :layout => false }
         format.json { xml_to_json("errors/#{@code || @error.code}") }
diff --git a/server/lib/sinatra/rack_logger.rb b/server/lib/sinatra/rack_logger.rb
index d986ac1..677444b 100644
--- a/server/lib/sinatra/rack_logger.rb
+++ b/server/lib/sinatra/rack_logger.rb
@@ -50,6 +50,11 @@ module Rack
       self
     end
 
+    def self.error(code, &block)
+      @logger ||= ::Logger.new(log_path || $stdout)
+      @logger.error(code, &block)
+    end
+
     # Common Log Format: http://httpd.apache.org/docs/1.3/logs.html#common
     # lilith.local - - [07/Aug/2006 23:58:02] "GET / HTTP/1.1" 500 -
     #             %{%s - %s [%s] "%s %s%s %s" %d %s\n} %
diff --git a/server/views/errors/404.html.haml b/server/views/errors/404.html.haml
index a9bbe34..a05c1eb 100644
--- a/server/views/errors/404.html.haml
+++ b/server/views/errors/404.html.haml
@@ -19,11 +19,10 @@
         %li
           %span{:style => 'font-weight:normal;'}=value.inspect
 
-
   %div{ 'data-role' => :collapsible, 'data-collapsed' => "true"}
-    %h3 Request details
+    %h3 Request Headers
     %ul{ :'data-role' => :listview , :'data-inset' => :true, :'data-divider-theme' => 'e'}
-      - request.env.each do |key, value|
+      - request_headers.each do |key, value|
         %li{ :'data-role' => 'list-divider'}=key
         %li
           %span{:style => 'font-weight:normal;'}=value.inspect
-- 
1.7.10.2