launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/remove-unnecessary-exception-handling into lp:launchpad.

Commit message:
Remove Python 2.4-style KeyboardInterrupt/SystemExit handling.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/remove-unnecessary-exception-handling/+merge/370616

As of Python 2.5, it's better style to use:

  except Exception:
      ...

... rather than:

  except (KeyboardInterrupt, SystemExit):
      raise
  except:
      ...

Switch to this style across the board.  The only unusual cases are some where we really want very robust last-ditch exception handling, and in those cases I used "except (GeneratorExit, Exception):" instead to make sure we catch any weirdness.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-unnecessary-exception-handling into lp:launchpad.

=== modified file 'lib/lp/archiveuploader/nascentupload.py'
--- lib/lp/archiveuploader/nascentupload.py	2018-12-04 15:01:51 +0000
+++ lib/lp/archiveuploader/nascentupload.py	2019-07-25 15:23:39 +0000
@@ -723,8 +723,6 @@
                 changes_file_object.close()
             return True
 
-        except (SystemExit, KeyboardInterrupt):
-            raise
         except QueueInconsistentStateError as e:
             # A QueueInconsistentStateError is expected if the rejection
             # is a routine rejection due to a bad package upload.

=== modified file 'lib/lp/archiveuploader/nascentuploadfile.py'
--- lib/lp/archiveuploader/nascentuploadfile.py	2019-05-24 17:12:50 +0000
+++ lib/lp/archiveuploader/nascentuploadfile.py	2019-07-25 15:23:39 +0000
@@ -555,9 +555,7 @@
             deb_file = apt_inst.DebFile(self.filepath)
             control_file = deb_file.control.extractdata("control")
             control_lines = apt_pkg.TagSection(control_file, bytes=True)
-        except (SystemExit, KeyboardInterrupt):
-            raise
-        except:
+        except Exception:
             yield UploadError(
                 "%s: extracting control file raised %s, giving up."
                  % (self.filename, sys.exc_type))
@@ -758,8 +756,6 @@
                     "far in the past (e.g. %s [%s])."
                      % (self.filename, len(ancient_files), first_file,
                         timestamp))
-        except (SystemExit, KeyboardInterrupt):
-            raise
         except Exception as error:
             # There is a very large number of places where we
             # might get an exception while checking the timestamps.

=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
--- lib/lp/archiveuploader/uploadprocessor.py	2018-05-08 17:54:36 +0000
+++ lib/lp/archiveuploader/uploadprocessor.py	2019-07-25 15:23:39 +0000
@@ -369,8 +369,6 @@
                               "%s " % e)
                 logger.debug(
                     "UploadPolicyError escaped upload.process", exc_info=True)
-            except (KeyboardInterrupt, SystemExit):
-                raise
             except EarlyReturnUploadError:
                 # An error occurred that prevented further error collection,
                 # add this fact to the list of errors.
@@ -510,9 +508,7 @@
             try:
                 results.add(self.processChangesFile(
                     changes_file, self.processor.log))
-            except (KeyboardInterrupt, SystemExit):
-                raise
-            except:
+            except Exception:
                 info = sys.exc_info()
                 message = (
                     'Exception while processing upload %s' % self.upload_path)
@@ -680,9 +676,7 @@
                 [changes_file] = self.locateChangesFiles()
                 logger.debug("Considering changefile %s" % changes_file)
                 result = self.processChangesFile(changes_file, logger)
-        except (KeyboardInterrupt, SystemExit):
-            raise
-        except:
+        except Exception:
             info = sys.exc_info()
             message = (
                 'Exception while processing upload %s' % self.upload_path)

=== modified file 'lib/lp/bugs/scripts/bugimport.py'
--- lib/lp/bugs/scripts/bugimport.py	2019-06-18 16:52:56 +0000
+++ lib/lp/bugs/scripts/bugimport.py	2019-07-25 15:23:39 +0000
@@ -252,9 +252,7 @@
                 self.loadCache()
                 self.importBug(bugnode)
                 self.saveCache()
-            except (SystemExit, KeyboardInterrupt):
-                raise
-            except:
+            except Exception:
                 self.logger.exception(
                     'Could not import bug #%s', bugnode.get('id'))
                 ztm.abort()

=== modified file 'lib/lp/bugs/scripts/bugnotification.py'
--- lib/lp/bugs/scripts/bugnotification.py	2016-06-25 07:56:09 +0000
+++ lib/lp/bugs/scripts/bugnotification.py	2019-07-25 15:23:39 +0000
@@ -303,9 +303,7 @@
         # being sent, so catch and log all exceptions.
         try:
             yield construct_email_notifications(batch)
-        except (KeyboardInterrupt, SystemExit, GeneratorExit):
-            raise
-        except:
+        except Exception:
             log.exception("Error while building email notifications.")
             transaction.abort()
             transaction.begin()

=== modified file 'lib/lp/bugs/scripts/checkwatches/core.py'
--- lib/lp/bugs/scripts/checkwatches/core.py	2018-02-02 10:06:24 +0000
+++ lib/lp/bugs/scripts/checkwatches/core.py	2019-07-25 15:23:39 +0000
@@ -265,9 +265,6 @@
 
         try:
             self._updateBugTracker(bug_tracker, batch_size)
-        except (KeyboardInterrupt, SystemExit):
-            # We should never catch KeyboardInterrupt or SystemExit.
-            raise
         except Exception as error:
             # If something unexpected goes wrong, we log it and
             # continue: a failure shouldn't break the updating of

=== modified file 'lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-guidelines.txt'
--- lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-guidelines.txt	2019-05-22 14:57:45 +0000
+++ lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-guidelines.txt	2019-07-25 15:23:39 +0000
@@ -208,9 +208,7 @@
     ...     user_browser.open(overview_url)
     ...     try:
     ...         user_browser.getLink(url=edit_url_re)
-    ...     except (SystemExit, KeyboardInterrupt):
-    ...         raise
-    ...     except:
+    ...     except Exception:
     ...         print(sys.exc_info()[0].__name__)
     ...     print('  - Admin:', end=' ')
     ...     admin_browser.open(overview_url)
@@ -236,9 +234,7 @@
     ...     print('* ' + context_name)
     ...     try:
     ...         user_browser.open(edit_url)
-    ...     except (SystemExit, KeyboardInterrupt):
-    ...         raise
-    ...     except:
+    ...     except Exception:
     ...         print(sys.exc_info()[0].__name__)
     * Ubuntu
       Unauthorized

=== modified file 'lib/lp/codehosting/puller/worker.py'
--- lib/lp/codehosting/puller/worker.py	2019-07-09 11:42:39 +0000
+++ lib/lp/codehosting/puller/worker.py	2019-07-25 15:23:39 +0000
@@ -443,10 +443,6 @@
         except InvalidURIError as e:
             self._mirrorFailed(e)
 
-        except (KeyboardInterrupt, SystemExit):
-            # Do not record OOPS for those exceptions.
-            raise
-
         else:
             revid_after = dest_branch.last_revision()
             # XXX: Aaron Bentley 2008-06-13

=== modified file 'lib/lp/hardwaredb/scripts/hwdbsubmissions.py'
--- lib/lp/hardwaredb/scripts/hwdbsubmissions.py	2019-06-18 16:52:56 +0000
+++ lib/lp/hardwaredb/scripts/hwdbsubmissions.py	2019-07-25 15:23:39 +0000
@@ -3038,9 +3038,6 @@
                     self._validateSubmission(submission)
                 else:
                     self._invalidateSubmission(submission)
-            except (KeyboardInterrupt, SystemExit):
-                # We should never catch these exceptions.
-                raise
             except LibrarianServerError:
                 # LibrarianServerError is raised when the server could
                 # not be reaches for 30 minutes.

=== modified file 'lib/lp/registry/scripts/productreleasefinder/finder.py'
--- lib/lp/registry/scripts/productreleasefinder/finder.py	2018-07-13 12:48:19 +0000
+++ lib/lp/registry/scripts/productreleasefinder/finder.py	2019-07-25 15:23:39 +0000
@@ -150,9 +150,7 @@
                 try:
                     self.handleRelease(
                         product_name, series_name, url, file_names)
-                except (KeyboardInterrupt, SystemExit):
-                    raise
-                except:
+                except Exception:
                     self.log.exception("Could not successfully process "
                                        "URL %s for %s/%s",
                                        url, product_name, series_name)

=== modified file 'lib/lp/services/apachelogparser/base.py'
--- lib/lp/services/apachelogparser/base.py	2017-10-05 19:01:06 +0000
+++ lib/lp/services/apachelogparser/base.py	2019-07-25 15:23:39 +0000
@@ -155,8 +155,6 @@
             if country_code not in daily_downloads:
                 daily_downloads[country_code] = 0
             daily_downloads[country_code] += 1
-        except (KeyboardInterrupt, SystemExit):
-            raise
         except Exception as e:
             # Update parsed_bytes to the end of the last line we parsed
             # successfully, log this as an error and break the loop so that

=== modified file 'lib/lp/services/librarian/smoketest.py'
--- lib/lp/services/librarian/smoketest.py	2011-12-30 01:04:24 +0000
+++ lib/lp/services/librarian/smoketest.py	2019-07-25 15:23:39 +0000
@@ -37,10 +37,10 @@
 def read_file(url):
     try:
         data = urllib.urlopen(url).read()
-    except (MemoryError, KeyboardInterrupt, SystemExit):
+    except MemoryError:
         # Re-raise catastrophic errors.
         raise
-    except:
+    except Exception:
         # An error is represented by returning None, which won't match when
         # comapred against FILE_DATA.
         return None

=== modified file 'lib/lp/services/mail/incoming.py'
--- lib/lp/services/mail/incoming.py	2018-04-22 23:30:37 +0000
+++ lib/lp/services/mail/incoming.py	2019-07-25 15:23:39 +0000
@@ -439,14 +439,12 @@
                     signature_timestamp_checker)
                 trans.commit()
                 mailbox.delete(mail_id)
-            except (KeyboardInterrupt, SystemExit):
-                raise
-            except:
-                # This bare except is needed in order to prevent any bug
-                # in the email handling from causing the email interface
-                # to lock up. We simply log the error, then send an oops, and
-                # continue through the mailbox, so that it doesn't stop the
-                # rest of the emails from being processed.
+            except (GeneratorExit, Exception):
+                # This general exception handler is needed in order to
+                # prevent any bug in the email handling from causing the
+                # email interface to lock up. We simply log the error, then
+                # send an oops, and continue through the mailbox, so that it
+                # doesn't stop the rest of the emails from being processed.
                 log.exception(
                     "An exception was raised inside the handler:\n%s"
                     % (file_alias_url,))

=== modified file 'lib/lp/services/profile/profile.py'
--- lib/lp/services/profile/profile.py	2013-04-10 09:12:32 +0000
+++ lib/lp/services/profile/profile.py	2019-07-25 15:23:39 +0000
@@ -489,9 +489,7 @@
         encoding = content_type_params.get('charset', 'utf-8')
         try:
             added_html = template(**template_context).encode(encoding)
-        except (SystemExit, KeyboardInterrupt):
-            raise
-        except:
+        except Exception:
             error = ''.join(format_exception(*sys.exc_info(), as_html=True))
             added_html = (
                 '<div class="profiling_info">' + error + '</div>')

=== modified file 'lib/lp/services/timeout.py'
--- lib/lp/services/timeout.py	2019-06-18 17:30:52 +0000
+++ lib/lp/services/timeout.py	2019-07-25 15:23:39 +0000
@@ -155,9 +155,6 @@
         """See `Thread`."""
         try:
             self.result = self.target(*self.args, **self.kwargs)
-        except (SystemExit, KeyboardInterrupt):
-            # Don't trap those.
-            raise
         except Exception:
             self.exc_info = sys.exc_info()
 

=== modified file 'lib/lp/services/webapp/adapter.py'
--- lib/lp/services/webapp/adapter.py	2018-03-30 20:42:14 +0000
+++ lib/lp/services/webapp/adapter.py	2019-07-25 15:23:39 +0000
@@ -697,9 +697,9 @@
                     log_traceback = conditional(
                         self._normalize_whitespace(
                             statement_to_log.strip()).upper())
-                except (MemoryError, SystemExit, KeyboardInterrupt):
+                except MemoryError:
                     raise
-                except:
+                except (GeneratorExit, Exception):
                     exc_type, exc_value, tb = sys.exc_info()
                     log_sql[-1]['exception'] = (exc_type, exc_value)
                     log_sql[-1]['stack'] = extract_tb(tb)

=== modified file 'lib/lp/testing/yuixhr.py'
--- lib/lp/testing/yuixhr.py	2018-05-06 08:52:34 +0000
+++ lib/lp/testing/yuixhr.py	2019-07-25 15:23:39 +0000
@@ -145,9 +145,7 @@
             DatabaseLayer.testTearDown()
             yield ''
             LibrarianLayer.testSetUp()
-        except (SystemExit, KeyboardInterrupt):
-            raise
-        except:
+        except Exception:
             print "Hm, serious error when trying to clean up the test."
             traceback.print_exc()
         # We're done, so we can yield the body.

=== modified file 'lib/lp/translations/scripts/po_export_queue.py'
--- lib/lp/translations/scripts/po_export_queue.py	2012-09-28 06:01:02 +0000
+++ lib/lp/translations/scripts/po_export_queue.py	2019-07-25 15:23:39 +0000
@@ -388,14 +388,11 @@
         exported_file = translation_exporter.exportTranslationFiles(
             generate_translationfiledata(requested_objects, format),
             target_format=format)
-    except (KeyboardInterrupt, SystemExit):
-        # We should never catch KeyboardInterrupt or SystemExit.
-        raise
     except psycopg2.Error:
         # It's a DB exception, we don't catch it either, the export
         # should be done again in a new transaction.
         raise
-    except:
+    except Exception:
         # The export for the current entry failed with an unexpected
         # error, we add the entry to the list of errors.
         result.addFailure()

=== modified file 'lib/lp/translations/scripts/translations_to_branch.py'
--- lib/lp/translations/scripts/translations_to_branch.py	2013-06-20 05:50:00 +0000
+++ lib/lp/translations/scripts/translations_to_branch.py	2019-07-25 15:23:39 +0000
@@ -254,8 +254,6 @@
 
                 if self.txn:
                     self.txn.commit()
-            except (KeyboardInterrupt, SystemExit):
-                raise
             except NotBranchError:
                 unpushed_branches += 1
                 if self.txn:

=== modified file 'lib/lp/translations/scripts/verify_pofile_stats.py'
--- lib/lp/translations/scripts/verify_pofile_stats.py	2015-07-08 16:05:11 +0000
+++ lib/lp/translations/scripts/verify_pofile_stats.py	2019-07-25 15:23:39 +0000
@@ -68,8 +68,6 @@
             self.start_id = pofile.id + 1
             try:
                 self._verify(pofile)
-            except (KeyboardInterrupt, SystemExit):
-                raise
             except Exception as error:
                 # Verification failed for this POFile.  Don't bail out: if
                 # there's a pattern of failure, we'll want to report that and