Make sure experiment doesn't fail when plotting raises an exception

[aalmah]

This would catch the exception and would not lead to the experiment failing when, e.g., the bokeh server goes down..

[dmitriy-serdyuk]

I see the problem, but it is a bad practice to catch all possible exceptions. For example your code is going to catch Ctrl-C and just print a message. Can you investigate which kind of exception can bokeh throw and catch only that ones?

[dwf]

@dmitriy-serdyuk Actually things like SystemExit and KeyboardInterrupt inherits from BaseException and not Exception, so except Exception: will let those pass, however I agree that this is still too broad.

[aalmah]

Good point. I will try to investigate what kind of exceptions bokeh throws.

[rizar]

Actually, I think that catching Exception is fine, because as David said, it is a subclass of BaseException which indicates that an actual error has happened. And I don't think we really care how exactly Bokeh broke. On the other hand, sometimes I really want to run training with Bokeh and find it convenient that training does not start unless Bokeh works well. So I would make this functionality optional.

On 28 September 2015 at 17:07, Amjad Almahairi notifications@github.com wrote:

Good point. I will try to investigate what kind of exceptions bokeh throws.

— Reply to this email directly or view it on GitHub https://github.com/mila-udem/blocks-extras/pull/23#issuecomment-143874577 .

[dwf]

Okay, fair enough. I still think we should make sure that the only thing in the try block is the external library call (I forget if this is the case) and we should print the full traceback, at least the first time it happens; otherwise it becomes hard to debug.

[rizar]

Good point. blocks.util.reraise_as might be very helpful in this context.

[aalmah]

This prints stack trace, but every time do is called. Should that be only first time only? Should I limit more the try block. There are two places cursession() and push(). What do you think?

[rizar]

@dwf, I think it should be fine to put the whole do method in try-except, because well, do we really care? What we are doing now is required mostly because is not fully stable, but its instabilities could even cause an error in our code.

That said, I would also wrap __init__ with try-except. We could even introduce a class UnstableExtension:

class UnstableExtension(SimpleExtension):
    def __init__(self, on_error='raise', *args, **kwargs):
          self.on_error = on_error
          try:
               super(UnstableExtension, self).__init__(self, *args, **kwargs) 
          except Extension:
               if self.on_error == 'raise':  
                     reraise_as(...) 

     def do(self, *args, **kwargs):
           #the same try-except

It would be good to have such thing in general, because when e.g. one has two weeks before deadline and suddenly has occasional problems with something like ProgressBar or Plot, the simplest thing would be just ignore their failures.