1

I fully get the general principle of not catching all exception, as explained in this question, for instance (Why is "except: pass" a bad programming practice?). Yet I have found myself writing this sequence to get the source of a file like object:

    try:
        self.doc.source = source.geturl()
    except:
        try:
            self.doc.source = pathlib.Path(os.path.abspath(source.name)).as_uri()
        except:
            self.doc.source = None

Clearly with some spelunking I could figure out which specific errors to catch with a reasonable degree of confidence. But at is explained in this question (Python: How can I know which exceptions might be thrown from a method call) you can't ever be quite certain.

So is there a better way to do what I am doing here, which is essentially to say: try this and if it doesn't work try this and if that doesn't work do this. Since this is all about setting a single variable, and there is a fallback of setting it to None, it is not obvious to me wherein the peril lies in this construct.

Community
  • 1
  • 1
  • 1
    as long as you save the [logging.exception()](https://docs.python.org/3/library/logging.html#logging.exception) to a log file.. – Aprillion Aug 13 '16 at 12:45

1 Answers1

0

Maybe it'd be better to put it into a dedicated function ?

def func(source):
   try:
       return source.geturl()
   except Exception:
       try:
           return pathlib.Path(os.path.abspath(source.name)).as_uri()
       except Exception:
           pass

Notes on swallowing exceptions

Ignoring any kind of exception is not the best pattern : it'd be better to know what call can raise what kind of exception.

Even if except ...: pass is bad, that's already what you were doing in your example, and here it's clear that we try another way if it fails.

The bad programming practice is basing the error handling on the ability to catch any kind of error. In itself, except: pass is better than except: <lots_of_things_that_wont_raise_again>.

However, the first answer in the post you referred to says something very sensible : pass semantically indicates that you won't do ANYTHING with the exception, and as you do not store in a variable, it also shows that you'll never be able to access it again (well you could, but still..). As /u/Aprillion/, you'll want to at least log these errors, because otherwise, you may be swallowing very useful debugging information, which could make your life (or others) much more difficult at some point.

For example, what happens if there's a bug hidden in geturl, that makes it raises exception in normal cases ? It may stay hidden for long, because this exception will be swallowed and never shown anywhere. Good luck to the debugger to find it !

Ways to improve this

  • replace except Exception with the exceptions that could be raised
  • check before an attempt if it is going to fail, so that you don't have to catch an exception

Additionally, you probably want to use a logger to save/show these exceptions somewhere, at least in DEBUG mode.

Clearly with some spelunking I could figure out which specific errors to catch with a reasonable degree of confidence. But at is explained in this question (Python: How can I know which exceptions might be thrown from a method call) you can't ever be quite certain.

Why would that change anything ? If geturl returns specific exception types, you'd only catch these, and you'd want other unexpected errors to bubble up to the interpreter.

The big problem with the givn approach is that if geturl is undefined, or not callable, or take more arguments, this error would not make the interpreter crash, even though it is a clear programming error. That's because except: or except Exception: will catch a lof of Python errors, should it be AttributeError, NameError, ImportError or even SyntaxError.

To conclude, I really think you'd prefer to replace Exception with the list of exceptions you can catch (and at least send the exceptions to a logger). Note that you can write it as such :

   try:
       return source.geturl()
   except (ExceptionA, ExceptionB, ExceptionC):
       pass
pistache
  • 5,782
  • 1
  • 29
  • 50
  • I get that. I guess what I am asking myself is, what is the lesser of the two evils. If geturl fails for some reasons other than the source not being a URL, it is better for debugging purposes to have the program fail, but it may be better from the user perspective to have it continue with mildly reduced functionality. They will even get a warning down the line if the program is unable to resolve a URL relative to the current source, since that is why we need the self.doc.source variable in the first place. So a bug would not pass entirely unnoticed. So: lesser of two evils? –  Aug 13 '16 at 13:51
  • The bigger of two evils is : sending useful information a blackhole. If if should work with reduced functionality, it should be upon clearly defined conditions, doing it based on "any kind of error" is bad code architecture. – pistache Aug 13 '16 at 13:58
  • Also, you say "source not being a URL", what do you mean by that ? Are you doing all this because `source` can be one of two types, one who has a `name` attribute and the other one a `geturl` function ? – pistache Aug 13 '16 at 13:59
  • A last thing : "better from the user perspective" could also mean "with clearly defined behaviour", which imply "failing on unexpected errors". The thing is to separate expected and unexpected errors, which you can't do if you `except Exception: ...` or `except:`. – pistache Aug 13 '16 at 14:01
  • I meant, not being an object that has a `geturl` function, I suppose. Which by itself would raise an `AttributeError`. So it would be reasonable to catch the `AttributeError` and then have the program recover from unexpected errors where they are not fatal, but report them as unexpected errors and ask for a bug report? For instance, wrap this whole thing in another try, use 'except Exception' to catch unexpected exceptions, report/log them and continue? –  Aug 13 '16 at 14:35
  • You don't want an attribute to have multiple possible types, unless they are compatible and can be used with generic methods, which isn't the case here. You want to make the code **not able** to raise an AttributeError, by **ensuring** that anything you put in `source` provides a generic interface. Even if Python is said "dynamically typed", that doesn't mean you should store anything in a reference. Duck typing (not relying on the type) relies on defining a standard interface ("please quack, thing") instead of multiple behaviours ("quack or woof if you can't"). – pistache Aug 13 '16 at 15:09
  • If you want we can continue the discussion in chat. – pistache Aug 13 '16 at 15:11