Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-14310

Fix catching of generic Exceptions

    XMLWordPrintable

Details

    • generic Exceptions

    Description

      There are a lot of except Exception blocks in our code. These blocks often just swallow the error, either logging a message or just continue-ing without saying anything. In almost every case (I can't prove it's every case without further exploration), we should only catch a very specific and short list of exceptions. The same applies to except lsst.pex.exceptions.Exception and except pexExcept.Exception.

      I'm making this an epic as I started filling in the components list and ended up with just about every LSST owned package, so I figure this is best done on a package-by-package basis, or at least on groups of packages.

      Filing as critical, as we already know of cases where this bug has hidden other problems (e.g. DM-13803).

      Attachments

        Issue Links

          Activity

            ktl Kian-Tat Lim added a comment -

            While the general principle is correct, I'm not sure we should ban cases like this:

            try:
                do_something_specific
            except Exception as e:
                log.debug("Couldn't do something: %s", e)
                do_something_in_another_way
            

            where overall failure in the case of an unexpected exception is undesirable.

            ktl Kian-Tat Lim added a comment - While the general principle is correct, I'm not sure we should ban cases like this: try : do_something_specific except Exception as e: log.debug( "Couldn't do something: %s" , e) do_something_in_another_way where overall failure in the case of an unexpected exception is undesirable.
            Parejkoj John Parejko added a comment -

            do_something_specific has a handful of known exceptions it can raise. If something else gets raised, something strange is probably going wrong. If an exception outside those defined for do_something_specific is raised, how do you have any idea if do_something_in_another_way could possibly work?

            Parejkoj John Parejko added a comment - do_something_specific has a handful of known exceptions it can raise. If something else gets raised, something strange is probably going wrong. If an exception outside those defined for do_something_specific is raised, how do you have any idea if do_something_in_another_way could possibly work?
            rowen Russell Owen added a comment -

            We have a number of cases where we want to make a "best effort" to do something, and continue if not. Plotting code is typical. In such cases I think it is appropriate to log and move on.

            We also have a lot of code that does not document what exceptions it may raise. And even if all our code did document the exceptions it could raise, that documentation is likely to go out of date as our code changes. It can be very tricky to maintain such documentation because code calls other code that calls other code...

            I agree that logging exceptions is much better than silently ignoring them. I am much less sanguine that we can always catch just a few specific exceptions. Sometimes we certainly can. But for complex code it is often safest to cast a broader net, which in Python usually means catching Exception.

            rowen Russell Owen added a comment - We have a number of cases where we want to make a "best effort" to do something, and continue if not. Plotting code is typical. In such cases I think it is appropriate to log and move on. We also have a lot of code that does not document what exceptions it may raise. And even if all our code did document the exceptions it could raise, that documentation is likely to go out of date as our code changes. It can be very tricky to maintain such documentation because code calls other code that calls other code... I agree that logging exceptions is much better than silently ignoring them. I am much less sanguine that we can always catch just a few specific exceptions. Sometimes we certainly can. But for complex code it is often safest to cast a broader net, which in Python usually means catching Exception .
            Parejkoj John Parejko added a comment -

            Having just been burned by this again just now in makeCoaddTempExp (whose run contains a for loop with 4 "catch and mask all exceptions" blocks!), I'm going to try to make a list of these cases. At absolutely bare minimum, they should be modified to respect doRaise.

            Parejkoj John Parejko added a comment - Having just been burned by this again just now in makeCoaddTempExp (whose run contains a for loop with 4 "catch and mask all exceptions" blocks!), I'm going to try to make a list of these cases. At absolutely bare minimum, they should be modified to respect doRaise .

            Just voting here that I support doing something about this

            mfisherlevine Merlin Fisher-Levine added a comment - Just voting here that I support doing something about this

            People

              Unassigned Unassigned
              Parejkoj John Parejko
              Hsin-Fang Chiang, Jim Bosch, John Parejko, John Swinbank, Kian-Tat Lim, Krzysztof Findeisen, Merlin Fisher-Levine, Michael Wood-Vasey, Russell Owen, Tim Jenness, Yusra AlSayyad
              Votes:
              1 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

                Created:
                Updated:

                Jenkins

                  No builds found.