Rescue Exception

Jim

The idiom in question is:


begin
  # Try something that could fail
rescue Exception => e
  # Handle exception. Sometimes we don't do this properly.
end

I'm guilty of this myself, but I have started to re-think my use of the rescue Exception => e clause.

My current understanding is that it is "bad style"[1] to rescue Exception directly. Rescuing Exception - which is the root class of all Ruby Exceptions - captures all Exceptions including possibly some Ruby system exceptions. I am not sure of the full implications of this, but it seems smarter to be more specific rather than more general when rescuing exceptions.

The real problem might not be rescuing Exception, but rather mishandling or throwing away the Exception once caught.

From [1]

@unknownuser said:

That's a specific case of the rule that you shouldn't catch any exception you don't know how to handle. If you don't know how to handle it, it's always better to let some other part of the system catch and handle it.

and

@unknownuser said:

You should handle only exceptions that you know how to recover from. If you don't anticipate a certain kind of exception, don't handle it, crash loudly (write details to the log), then diagnose logs and fix code.

Swallowing exceptions is bad, don't do this.

Thomthom provided this link to a decent article on using Ruby Exceptions[2].

And here's a book and tiny video[3] (which I haven't watched yet, but looks promising.)

[1] http://stackoverflow.com/questions/10048173/why-is-it-bad-style-to-rescue-exception-e-in-ruby

[2] http://www.skorks.com/2009/09/ruby-exceptions-and-exception-handling/

[3] http://exceptionalruby.com/

Dan Rathbun

My first thought is to remind all, that these "general" Ruby best practices are written from the standpoint of a Ruby script running it's it's own Ruby process, not in an embedded process of some application such as SketchUp or DoubleCAD.

If you "crash loudly" in an embedded process, you can crash the parent application!

In my opinion, for embedded scripting use, the advice should be reversed. Ie.. protect the parent application at all costs, which will protect the user's work (the model.) If your plugin, crashes SketchUp, and corrupts the user's model, ... your plugin (and you as a programmer,) may acquire a negative reputation with the userbase, which can be hard to overcome.

My second thought, is that exception handling should be layered.

If the rescue clause in a specific method, cannot handle the exact exception (ie, based on the message, for instance,) it can re-raise the exception to a higher level, such as the calling method.

Dan Rathbun

@jim said:

The real problem might not be rescuing Exception, but rather mishandling or throwing away the Exception once caught.

I think I'd have to agree (and I should admit I also tend "overcatch" exceptions myself.)

@unknownuser said:

(http://stackoverflow.com/questions/10048173/why-is-it-bad-style-to-rescue-exception-e-in-ruby)":1dv37f3l]... (write details to the log), then diagnose logs and fix code.

Swallowing exceptions is bad, don't do this.

How can anyone argue with that advice?

We cannot fix errors, whether in the development cycle or after deployment, if we don't know about them and their details.

Logging is always a "best practice".

However, how can we ALL (SketchUp plugin authors and non-programmer users,) share a logging system that works well in an embedded Ruby environment?
But...

Does not force anyone to install or adopt a certain author's libraries (that do other things not related to debugging, etc.)
Are easy enough to use, that a non-programming user can be told to push a button, check a list, and copy and paste any error that comes from a specific plugin. An easy feedback workflow.

I have some ideas.. did some tests, but have not had time to go further with this.
Sheesh.. I haven't even had time to try out the OEM Developer Tools yet!

Dan Rathbun

After reading:
@jim said:

[1] why-is-it-bad-style-to-rescue-exception-e-in-ruby

... I think I must agree with Andrew Marshall regarding SignalException, Interrupt and SyntaxError.

I think I will try from now on, to only trap StandardError subclasses (unless I KNOW any of the others could occur, in my own code.).

So in the VisTools updates I am working on, I will need to change all the rescue clauses.
Then I'll just have to hope that if some other plugin uses observers that are triggered by the VisTools commands, that the author of those plugins properly trap whatever possible exceptions that their code causes.

Dan Rathbun

@dan rathbun said:

So in the VisTools updates I am working on, I will need to change all the rescue clauses ...

Well I did make these changes in the v1.3.0 release.

All the rescue clauses look similar to this now:

rescue => e
  #
  unless $VERBOSE.nil? # not in silent mode
    puts("\nVisTools; Error encountered in 'isolate_entities' ; Aborting Model Operation.")
    puts("Error; #<#{e.class.name}; #{e.message}>") if @@debug
    puts(e.backtrace) if @@debug && $VERBOSE
    puts()
  end
  Sketchup.active_model.abort_operation
  raise
  #
end

I realized in this case, what I primarily wanted to do in the rescue clause, was abort the operation.

So after doing that, I just re-raise the exception, so any debugger or development toolkit can see it.

thomthom

@dan rathbun said:

So after doing that, I just re-raise the exception, so any debugger or development toolkit can see it.

I find that pattern useful myself.

Dan Rathbun

@thomthom said:

@dan rathbun said:

So after doing that, I just re-raise the exception, so any debugger or development toolkit can see it.

I find that pattern useful myself.

Yes, when we stop, and think a bit,... we should realize that exceptions may not be coming from OUR code, but from other author's code using observers, that may be triggered by our code.

If we swallow these exceptions at OUR point in the callstack, it could lead to a very frustrating debugging session.

I did test a few of the StandardError subclases (using my Error raising scriptlets for testing,) and it seems that SketchUp protectively wraps API calls.

Jim

@dan rathbun said:

If we swallow these exceptions at OUR point in the callstack, it could lead to a very frustrating debugging session.

I suspect this is currently happening and is what prompted the original topic. This and weird things like:

Dan Rathbun

@jim said:

... and weird things like:

Looks like Fredo could be overriding Enumerable.each_with_index() or the inherited method in the IO class (or it's subclass File) ??

thomthom

Is it not FredoPaint? It uses observers to hook into material events.

fredo6

@dan rathbun said:

@jim said:

... and weird things like:

Looks like Fredo could be overriding Enumerable.each_with_index() or the inherited method in the IO class (or it's subclass File) ??

Dan,

My scripts do not subclass any standard Ruby class.
I think the errors shown in Lib6Traductor.rb comes from the fact that in my utilities to convert strings to length, I use a technique to deliberately provoke an error within a begin / rescue clause in order to find out which locale convention Sketchup is configured for length parsing (basically by testing if "2.3".to_l is valid versus "2,3".to_l).

Fredo

Dan Rathbun

@unknownuser said:

My scripts do not subclass any standard Ruby class.

It is OK to subclass standard Ruby classes, within your own modules, but not allowed to change standard classes for only your use.

@unknownuser said:

I think the errors ... comes from the fact that ... I use a technique to deliberately provoke an error within a begin / rescue clause in order to find out which locale convention Sketchup is configured ...

OK ... yes I had also done that in several plugins.. and the most recent was VisTools < v1.3.0, so up untilv1.2.1 I was doing this:

OLD WAY:

#{ Load localized string hashes from file;
@@lang = Sketchup.get_locale()[0,2] unless defined?(@@lang)
begin
  if @@lang == 'en'
    raise(LoadError,"English")
  else
    load(File.join(BASEPATH,"VisTools_"<<@@lang<<".rb"))
  end
rescue LoadError => e
  if e.message != 'English' && !$VERBOSE.nil?
    puts()
    puts('NOTICE; Could not find a "VisTools_'<<@@lang<<'.rb" file to load.')
    puts('Using English text for VisTools plugin UI.')
    puts()
  end
  @@menutext = Hash[
    ;plugin_name,      "VisTools",
    #
    ;hide_layers,      "Hide Layers",
    ;isolate_layers,   "Isolate Layers",
    ;hide_entities,    "Hide Entities",
    ;isolate_entities, "Isolate Entities",
    ;freeze_entities,  "Freeze Groups/Components",
    ;unfreeze_all,     "Unfreeze All",
    ;show_all,         "Show All",
    #
    ;debug_mode, "Debug Mode"
  ]
  @@tooltips = Hash[
    ;hide_layers,      "Hide selected layers",
    ;isolate_layers,   "Isolate selected layers",
    ;hide_entities,    "Hide selected entities",
    ;isolate_entities, "Isolate selected entities",
    ;freeze_entities,  "Freeze groups and components",
    ;unfreeze_all,     "Unfreeze all",
    ;show_all,         "Show all layers and entities"
  ]
end #}

THEN ... I read the Ruby Style Guide and realized that for various reasons ... using an exception as a conditional / branching construct is VERY BAD CODING STYLE and confusing to anyone reading or debugging the code!

So I revised that code block, so it now looks like this in v1.3.0+:

#{ String hashes;
@@lang = Sketchup.get_locale()[0,2] unless defined?(@@lang)
unless @@lang == 'en'
  # Load localized string hashes from file;
  begin
    load(File.join(BASEPATH,"VisTools_"<<@@lang<<".rb"))
  rescue LoadError => e
    unless $VERBOSE.nil?
      puts()
      puts('NOTICE; Could not find a "VisTools_'<<@@lang<<'.rb" file to load.')
      puts('Using English text for VisTools plugin UI.')
      puts()
    end
    @@lang == 'en' # just use English
  end
end # load attempt
#
if @@lang == 'en'
  @@menutext = Hash[
    ;plugin_name,      "VisTools",
    #
    ;hide_layers,      "Hide Layers",
    ;isolate_layers,   "Isolate Layers",
    ;hide_entities,    "Hide Entities",
    ;isolate_entities, "Isolate Entities",
    ;freeze_entities,  "Freeze Groups/Components",
    ;unfreeze_all,     "Unfreeze All",
    ;show_all,         "Show All",
    #
    ;debug_mode, "Debug Mode"
  ]
  @@tooltips = Hash[
    ;hide_layers,      "Hide selected layers",
    ;isolate_layers,   "Isolate selected layers",
    ;hide_entities,    "Hide selected entities",
    ;isolate_entities, "Isolate selected entities",
    ;freeze_entities,  "Freeze groups and components",
    ;unfreeze_all,     "Unfreeze all",
    ;show_all,         "Show all layers and entities"
  ]
end #}

It should work the same, but not misuse (ie, raise any unneeded) exceptions.

However, a chinese user has just posted an error where it is not working quite correct yet.
English should be used, but the user is getting a LoadError... (see the error post).

fredo6

Dan,

Thanks for your comments

I mean my scripts do not alter in anyway the standard Ruby classes and the Sketchup classes too.
The dot / comma interpretation in Sketchup based on the locale was beyond my understanding and knowledge, so I used this quick way to check whether dot or comma is the decimal separator. I can imagine there will be an API for that in future SU versions.

Fredo

thomthom

@unknownuser said:

The dot / comma interpretation in Sketchup based on the locale was beyond my understanding and knowledge, so I used this quick way to check whether dot or comma is the decimal separator. I can imagine there will be an API for that in future SU versions.

I hope so. I've yet to find another way to "detect" the decimal point within SU's Ruby API.

Dan Rathbun

OK I understand ...

Visual Basic:

CultureInfo.CurrentCulture.NumberFormat.NumberDecimalSeparator