[Ovirt-devel] [PATCH server 2/3] * app/controllers/application.rb (handle_auth_error): pass in block

[Scott Seago]

David Lutterkort wrote:
> This makes dealing with auth errors a little more concise. Eventually, I
> would like to change the signature of this method to be something like
>
>   handle_auth_error(exc_msgs={}, &block)
>
> so that we can pass in a mapping exception class => error message, i.e. a
> typical call might be
>
>   handle_auth_error(
>     ActiveRecord::RecordNotFound => "The object does not exist",
>     ActiveRecord::StaleObjectError => "Midair collision, try again") do
>     .. do stuff ..
>   end
>
>   
With something like the above, it seems like we've no longer got a 
handle_auth_error method but a handle_application_error method, as it 
would become a more general error-handling situation. Do you have any 
examples from the current codebase where we're handling multiple 
exception types that this would simplify? It seems like a good idea, I'm 
just not sure concretely how it would be used.

As it exists now, handle_auth_error is simply a way to encapsulate the 
error handling already present in the authorize_XXX methods, as for the 
service layer we have to separate authorization checking (raises the 
error) from auth error handling (rescues and deals with it). Do you see 
this new handle_<something>_error method as a way to replace all of the 
multiple rescue blocks in the controllers?

Scott
> To be compatible with existing code using it, it doesn't do that right now,
> and behaves just as before if called with a string.
> ---
>  src/app/controllers/application.rb |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/src/app/controllers/application.rb b/src/app/controllers/application.rb
> index e5f4d4b..a36d2fd 100644
> --- a/src/app/controllers/application.rb
> +++ b/src/app/controllers/application.rb
> @@ -115,7 +115,17 @@ class ApplicationController < ActionController::Base
>        true
>      end
>    end
> -  def handle_auth_error(msg)
> +  def handle_auth_error(msg=nil, &block)
> +    if block_given? && !msg.nil?
> +      raise ArgumentError, "You can only provide a msg or a block, not both"
> +    end
> +    if block_given?
> +      begin
> +        return yield
> +      rescue PermissionError => perm_error
> +        msg = perm_error.message
> +      end
> +    end
>      respond_to do |format|
>        format.html do
>          @title = "Access denied"
>