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

Fix catching of generic Exceptions

    XMLWordPrintable

    Details

    • Epic Name:
      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

            Hide
            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.

            Show
            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.
            Hide
            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?

            Show
            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?
            Hide
            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.

            Show
            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 .
            Hide
            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.

            Show
            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 .
            Hide
            mfisherlevine Merlin Fisher-Levine added a comment -

            Just voting here that I support doing something about this

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

              People

              Assignee:
              Unassigned Unassigned
              Reporter:
              Parejkoj John Parejko
              Watchers:
              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.