Exceptions in Ruby on Rails

Question

simple index action in my app

 def index
  @reports = CustomReport.all
 end

when the application was to ready I was told to add exceptions, I copied it from other application as I did not have any idea,

def index
    begin
      @reports = CustomReport.all
    rescue Exception => e
      Rails.logger.info "Exception in design_custom_reports controller, index action"
      Rails.logger.info e
      flash[:notice] = "Sorry, something went wrong. Please inform administration"
      redirect_to :controller => :user, :action => :dashboard
    end
end

and now it looks clumsy isn't it?

Best way to handle the scenario in RoR?

I don't understand the idea of this exception handler, what do you want to do? — ole, Dec 19 '13 at 12:31
http://stackoverflow.com/questions/10048173/why-is-it-bad-style-to-rescue-exception-e-in-ruby — Marek Lipka, Dec 19 '13 at 12:38
I really doesn't make sense. You try to catch an error while requesting all records from database? Usually the application will stop working before running into this rescue — devanand, Dec 19 '13 at 12:40
This is just a side point, but instead of def...begin...rescue...end the generally accepted style is to def...rescue...end. — kddeisz, Dec 19 '13 at 14:28

score 4 · Accepted Answer · answered Dec 19 '13 at 12:42

Short answer

That rescue clause doesn't really make sense. Remove it.

Long answer

That's a very bad code. First of all, the question is: who told you to "add exceptions" and for which purpose?

The code

CustomReport.all

is known to raise an exception only in very special cases. Generally speaking, when the application can't connect to the database. In fact, there are no external user inputs or conditions that may cause the code to naturally fail.

Conversely, this code can fail more frequently

CustomReport.find(params[:id])

because find raises an error if the object is not found, which is definitely a more common case.

If your method crashes for a database error, it's likely the entire application is affected so the rescue management probably makes sense in your global application, not really in that method. There's not that much you can do there to rescue it.

Last but not least, rescuing an exception of class Exception

rescue Exception => e

is considered a code smell in Ruby. In fact, you should rescue only StandardErrors or greater. If you rescue an Exception class you must be very aware of what it means. System level errors and syntax errors inherits from Exception, so if you rescue an Exception it's likely that you will hide a lot of potential errors in your code.

But again, even rescuing StandardError does not make a lot of sense. In fact, we said before that CustomReport.all could only fail in case of database connection errors. This means that, if you really want to rescue something, you should rescue only database failures there.

The final work is that in the case you mentioned, the exception management you wrote does not make sense. Whenever you are writing code, you should have corresponding unit/functional/integration tests. If you write the tests for the code you wrote, you will notice on your own that the way you wrote it doesn't make a lot of sense. — Simone Carletti, Dec 19 '13 at 13:33
[Don’t rescue Exception. EVER. or I will stab you.](http://www.zenspider.com/Languages/Ruby/QuickRef.html#general-tips) — nicosantangelo, Dec 19 '13 at 14:46

score 0 · Answer 2 · answered Dec 19 '13 at 12:37

0

you can put that code in Application controller . and it will capture all error in your whole application. and you do not need to write any more thing in any other controller as they are already inherited by application controller

rescue_from Exception  do |exception|
  @error = exception
  exception_logger
end

private

def exception_logger
 Rails.logger.error "Exception in design_custom_reports controller, index action"
 Rails.logger.error @error
 flash[:notice] = "Sorry, something went wrong. Please inform administration"
 redirect_to :controller => :user, :action => :dashboard
end

answered Dec 19 '13 at 12:37

Nitin Jain

3,053
2
24
36

2

That's a very bad example. Never rescue an exception of type `Exception` (this is a common Java practice that is considered bad code in Ruby), especially in a complex application such as Rails. – Simone Carletti Dec 19 '13 at 12:47
@SimoneCarletti i know that but i just try to give a solution according to question :( – Nitin Jain Dec 19 '13 at 12:49
2

I understand. However, given the level of expertise of the user, I think it's more effective to provide a solution that would not encourage him to follow bad practices. ;) – Simone Carletti Dec 19 '13 at 12:50
@SimoneCarletti i will keep that in mind in my future answer . thanks for your suggestion – Nitin Jain Dec 19 '13 at 12:50

Exceptions in Ruby on Rails

2 Answers2

Short answer

Long answer