SONAR-2932 log 500 errors + fix error handling in web services

3 files changed, 65 insertions, 43 deletions

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb
index 222409c3af0..02e2a7085df 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/api_controller.rb
@@ -21,22 +21,29 @@ require 'json'
 require 'time'
 class Api::ApiController < ApplicationController
 
-  rescue_from Errors::BadRequest do |error|
-    render_error(400, error.message)
-  end
-
-  rescue_from Errors::NotFound do |error|
-    render_error(404, error.message)
-  end
-
-  rescue_from ActiveRecord::RecordInvalid do |error|
-    render_error(400, error.message)
-  end
+  class ApiException < Exception
+    attr_reader :code, :msg
 
-  rescue_from ActiveRecord::RecordNotFound do |error|
-    render_error(404, error.message)
+    def initialize(code, msg)
+      @code = code
+      @msg = msg
+    end
   end
 
+  #
+  # Override the error handling defined in parent ApplicationController
+  #
+  rescue_from Exception, :with => :render_error
+  rescue_from ApiException, :with => :render_error
+  rescue_from Errors::BadRequest, :with => :render_bad_request
+  rescue_from ActionController::UnknownAction, :with => :render_not_found
+  rescue_from ActionController::RoutingError, :with => :render_not_found
+  rescue_from ActionController::UnknownController, :with => :render_not_found
+  rescue_from ActiveRecord::RecordNotFound, :with => :render_not_found
+  rescue_from Errors::NotFound, :with => :render_not_found
+  rescue_from Errors::AccessDenied, :with => :render_access_denied
+
+  
   protected
 
   def text_not_supported
@@ -63,12 +70,12 @@ class Api::ApiController < ApplicationController
     end
   end
 
-    # deprecated. Use Api::Utils.format_datetime
+  # deprecated. Use Api::Utils.format_datetime
   def format_datetime(datetime)
     Api::Utils.format_datetime(datetime)
   end
 
-    # deprecated. Use Api::Utils.parse_datetime
+  # deprecated. Use Api::Utils.parse_datetime
   def parse_datetime(datetime_string, default_is_now=true)
     Api::Utils.parse_datetime(datetime_string, default_is_now)
   end
@@ -81,14 +88,14 @@ class Api::ApiController < ApplicationController
   end
 
 
-
   #
   #
   # Error handling is different than in ApplicationController
   #
   #
 
-  def render_error(status, message=nil)
+  def render_error(message, status)
+    logger.error("Fail to render: #{request.url}", message) if status==500
     respond_to do |format|
       format.json { render :json => error_to_json(status, message), :status => status }
       format.xml { render :xml => error_to_xml(status, message), :status => status }
@@ -96,6 +103,22 @@ class Api::ApiController < ApplicationController
     end
   end
 
+  def render_not_found(error)
+    render_error(error.message, 404)
+  end
+
+  def render_bad_request(error)
+    render_error(error.message, 400)
+  end
+
+  def render_access_denied
+    render_error('Unauthorized', 401)
+  end
+
+  def render_success(message=nil)
+    render_error(message, 200)
+  end
+
   def error_to_json(status, message=nil)
     hash={:err_code => status}
     hash[:err_msg]=message if message
@@ -110,9 +133,6 @@ class Api::ApiController < ApplicationController
     end
   end
 
-  def render_success(message=nil)
-    render_error(200, message)
-  end
 
 end
 
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
index 6dc627136dc..5deb4851b67 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/application_controller.rb
@@ -24,21 +24,17 @@ class ApplicationController < ActionController::Base
   
   before_filter :check_database_version, :set_locale, :check_authentication
 
-  rescue_from Errors::BadRequest do |error|
-    render :text => error.message, :status => 400
-  end
-
-  rescue_from Errors::NotFound do |error|
-    render :text => error.message, :status => 404
-  end
-
-  rescue_from ActiveRecord::RecordNotFound do |error|
-    render :text => error.message, :status => 404
+  unless ActionController::Base.consider_all_requests_local
+    rescue_from Exception, :with => :render_error
+    rescue_from Errors::BadRequest, :with => :render_bad_request
+    rescue_from ActionController::UnknownAction, :with => :render_not_found
+    rescue_from ActionController::RoutingError, :with => :render_not_found
+    rescue_from ActionController::UnknownController, :with => :render_not_found
+    rescue_from ActiveRecord::RecordNotFound, :with => :render_not_found
+    rescue_from Errors::NotFound, :with => :render_not_found
+    rescue_from Errors::AccessDenied, :with => :render_access_denied # See lib/authenticated_system.rb#access_denied()
   end
 
-  # See lib/authenticated_system.rb#access_denied()
-  rescue_from Errors::AccessDenied, :with => :rescue_from_access_denied
-
   def self.root_context
     ActionController::Base.relative_url_root || ''
   end
@@ -88,14 +84,7 @@ class ApplicationController < ActionController::Base
       redirect_to :controller => 'maintenance', :action => 'index'
     end
   end
-  
-  # Do not log common errors like 404.
-  # See http://maintainable.com/articles/rails_logging_tips
-  EXCEPTIONS_NOT_LOGGED = ['ActionController::UnknownAction','ActionController::RoutingError']
-  def log_error(exc)
-    super unless EXCEPTIONS_NOT_LOGGED.include?(exc.class.name)
-  end
-  
+
   def set_locale
     if params[:locale]
       I18n.locale = request.compatible_language_from(available_locales, [params[:locale]])
@@ -140,4 +129,17 @@ class ApplicationController < ActionController::Base
     raise Errors::AccessDenied
   end
 
+  def render_not_found(error)
+    render :file => "#{Rails.public_path}/404.html", :status => 404
+  end
+
+  def render_bad_request(error)
+    render :text => error.message, :status => 400
+  end
+
+  def render_error(error)
+    logger.error("Fail to render: #{request.url}", error)
+    render :file => "#{Rails.public_path}/500.html", :status => 500
+  end
+
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/lib/authenticated_system.rb b/sonar-server/src/main/webapp/WEB-INF/lib/authenticated_system.rb
index 16d814913e7..10015abc172 100644
--- a/sonar-server/src/main/webapp/WEB-INF/lib/authenticated_system.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/lib/authenticated_system.rb
@@ -50,7 +50,7 @@ module AuthenticatedSystem
     #   skip_before_filter :login_required
     #
     def login_required
-      authorized? || rescue_from_access_denied
+      authorized? || render_access_denied
     end
 
     # Redirect as appropriate when an access request fails.
@@ -61,7 +61,7 @@ module AuthenticatedSystem
     # behavior in case the user is not authorized
     # to access the requested action.  For example, a popup window might
     # simply close itself.
-    def rescue_from_access_denied
+    def render_access_denied
       respond_to do |format|
         format.html do
           store_location