Use exceptions for error handling

The previous error handling strategy was very Linux kernel like, showing
influence from what I was working on while I designed this program.

Refactor the program to use exceptions, which significantly reduces the
amount of code needed for error handling. Best practices are now
followed: we only catch exceptions where we need to log a message and
rethrow, or for a complete unit of work.

Signed-off-by: Ira W. Snyder <devel@irasnyder.com>

diff --git a/PAR2Set/Base.py b/PAR2Set/Base.py
index 533ff4f..a5d3dcb 100644 (file)
--- a/PAR2Set/Base.py
+++ b/PAR2Set/Base.py
@@ -109,25 +109,7 @@ class Base (object):
                """Verify and Repair a PAR2Set. This is done using the par2repair command by
                   default"""
 
                """Verify and Repair a PAR2Set. This is done using the par2repair command by
                   default"""
 

-               PAR2_CMD = rsutil.common.config_get_value ('commands', 'par2repair')
-
-               # assemble the command
-               # par2repair -- PAR2 PAR2_EXTRA [JOIN_FILES]
-               command = "%s \"%s\" " % (PAR2_CMD, self.p2file)
-
-               for f in self.all_p2files:
-                       if f != self.p2file:
-                               command += "\"%s\" " % os.path.split (f)[1]
-
-               # run the command
-               ret = rsutil.common.run_command (command, self.dir)
-
-               # check the result
-               if ret != 0:
-                       logging.critical ('PAR2 Check / Repair failed: %s' % self.p2file)
-                       return -rsutil.common.ECHECK
-
-               return rsutil.common.SUCCESS
+               rsutil.common.run_command(['par2repair'] + self.all_p2files, self.dir)

 
        def find_deleteable_files (self):
                """Find all files which are deletable by using the regular expression from the
 
        def find_deleteable_files (self):
                """Find all files which are deletable by using the regular expression from the


@@ -147,32 +129,38 @@ class Base (object):
 
                assert os.path.isdir (dir)
 
 
                assert os.path.isdir (dir)
 

-               done = False
-               valid_y = ['Y', 'YES']
-               valid_n = ['N', 'NO', '']
-
+               # If we are interactive, decide yes or no

                if interactive:
                if interactive:

-                       while not done:
+                       while True:

                                print 'Do you want to delete the following?:'
                                for f in files:
                                        print f
                                print 'Do you want to delete the following?:'
                                for f in files:
                                        print f

-                               s = raw_input ('Delete [y/N]: ').upper()

 
 

-                               if s in valid_y + valid_n:
-                                       done = True
+                               s = raw_input('Delete [y/N]: ').upper()
+
+                               # If we got a valid no answer, leave now
+                               if s in ['N', 'NO', '']:
+                                       return
+
+                               # If we got a valid yes answer, delete them
+                               if s in ['Y', 'YES']:
+                                       break

 
 

-                       if s in valid_n:
-                               return rsutil.common.SUCCESS
+                               # Not a good answer, ask again
+                               print 'Invalid response'

 
 

+               # We got a yes answer (or are non-interactive), delete the files

                for f in files:
                for f in files:

-                       try:
-                               os.remove (os.path.join (dir, f))
-                               logging.debug ('Deleteing: %s' % os.path.join (dir, f))
-                       except:
-                               logging.error ('Failed to delete: %s' % os.path.join (dir, f))
-                               return -rsutil.common.EDELETE

 
 

-               return rsutil.common.SUCCESS
+                       # Get the full filename
+                       fullname = os.path.join(dir, f)
+
+                       # Delete the file
+                       try:
+                               os.remove(fullname)
+                               logging.debug('Deleting: %s' % fullname)
+                       except OSError:
+                               logging.error('Failed to delete: %s' % fullname)

 
        def runDelete (self):
                """Run the delete operation and return the result"""
 
        def runDelete (self):
                """Run the delete operation and return the result"""


@@ -187,78 +175,39 @@ class Base (object):
                """Run all of the major sections in the class: repair, extraction, and deletion."""
 
                # Repair Stage
                """Run all of the major sections in the class: repair, extraction, and deletion."""
 
                # Repair Stage

-               ret = self.runVerifyAndRepair ()
-
-               if ret != rsutil.common.SUCCESS:
-                       logging.critical ('Repair stage failed for: %s' % self.p2file)
-                       return -rsutil.common.ECHECK
+               try:
+                       self.runVerifyAndRepair ()
+               except (RuntimeError, OSError):
+                       logging.critical('Repair stage failed for: %s' % self.p2file)
+                       raise

 
 

-               self.update_matches ()
+               self.update_matches()

 
                # Extraction Stage
 
                # Extraction Stage

-               ret = self.runExtract ()
-
-               if ret != rsutil.common.SUCCESS:
-                       logging.critical ('Extraction stage failed for: %s' % self.p2file)
-                       return -rsutil.common.EEXTRACT
+               try:
+                       self.runExtract ()
+               except (RuntimeError, OSError):
+                       logging.critical('Extraction stage failed for: %s' % self.p2file)
+                       raise

 
                self.update_matches ()
 
                # Deletion Stage
 
                self.update_matches ()
 
                # Deletion Stage

-               ret = self.runDelete ()
-
-               if ret != rsutil.common.SUCCESS:
-                       logging.critical ('Deletion stage failed for: %s' % self.p2file)
-                       return -rsutil.common.EDELETE
-
-               logging.info ('Successfully completed: %s' % self.p2file)
-               return rsutil.common.SUCCESS
-
-       def safe_create_directory (self, dir):
-               """Safely create a directory, logging the result.
-
-                  dir -- the directory to create (None is ignored)"""
-
-               if dir == None:
-                       return rsutil.common.SUCCESS
-
-               if os.path.isdir (dir):
-                       return rsutil.common.SUCCESS
-

                try:
                try:

-                       os.makedirs (dir)
-                       logging.info ('Created directory: %s' % dir)
-               except OSError:
-                       logging.critical ('FAILED to create directory: %s' % dir)
-                       return -rsutil.common.ECREATE
+                       self.runDelete ()
+               except (RuntimeError, OSError):
+                       logging.critical('Deletion stage failed for: %s' % self.p2file)
+                       raise

 
 

-               return rsutil.common.SUCCESS
+               logging.info ('Successfully completed: %s' % self.p2file)

 
 

-       def runExtract (self, todir=None):
+       def runExtract (self):

                """Extract all heads of this set and return the result"""
 
                """Extract all heads of this set and return the result"""
 

-               # Extract to the head's dir if we don't care where to extract
-               if todir == None:
-                       todir = self.dir
-
-               # Create the directory $todir if it doesn't exist
-               ret = self.safe_create_directory (todir)
-
-               if ret != rsutil.common.SUCCESS:
-                       return -rsutil.common.EEXTRACT
-

                # Call the extraction function on each head
                for h in self.find_extraction_heads ():
                        full_head = rsutil.common.full_abspath (os.path.join (self.dir, h))
                # Call the extraction function on each head
                for h in self.find_extraction_heads ():
                        full_head = rsutil.common.full_abspath (os.path.join (self.dir, h))

-                       ret = self.extraction_function (full_head, todir)
-                       logging.debug ('Extraction Function returned: %d' % ret)
-
-                       # Check error code
-                       if ret != rsutil.common.SUCCESS:
-                               logging.critical ('Failed extracting: %s' % h)
-                               return -rsutil.common.EEXTRACT
-
-               return rsutil.common.SUCCESS
+                       self.extraction_function (full_head, self.dir)

 
        def find_extraction_heads (self):
                """Find all extraction heads associated with this set. This must be
 
        def find_extraction_heads (self):
                """Find all extraction heads associated with this set. This must be

diff --git a/PAR2Set/ExtractFirstBase.py b/PAR2Set/ExtractFirstBase.py
index 6b8a661..7acfc63 100644 (file)
--- a/PAR2Set/ExtractFirstBase.py
+++ b/PAR2Set/ExtractFirstBase.py
@@ -39,30 +39,29 @@ class ExtractFirstBase (PAR2Set.Base.Base):
                """Run the extraction, repair, and delete stages"""
 
                # Extraction Stage
                """Run the extraction, repair, and delete stages"""
 
                # Extraction Stage

-               ret = self.runExtract ()
-
-               if ret != rsutil.common.SUCCESS:
-                       logging.critical ('Extraction stage failed for: %s' % self.p2file)
-                       return -rsutil.common.EEXTRACT
+               try:
+                       self.runExtract()
+               except (RuntimeError, OSError):
+                       logging.critical('Extraction stage failed for: %s' % self.p2file)
+                       raise

 
                self.update_matches ()
 
                # Repair Stage
 
                self.update_matches ()
 
                # Repair Stage

-               ret = self.runVerifyAndRepair ()
-
-               if ret != rsutil.common.SUCCESS:
-                       logging.critical ('Repair stage failed for: %s' % self.p2file)
-                       return -rsutil.common.ECHECK
+               try:
+                       self.runVerifyAndRepair()
+               except (RuntimeError, OSError):
+                       logging.critical('Repair stage failed for: %s' % self.p2file)
+                       raise

 
                self.update_matches ()
 
                # Deletion Stage
 
                self.update_matches ()
 
                # Deletion Stage

-               ret = self.runDelete ()
-
-               if ret != rsutil.common.SUCCESS:
-                       logging.critical ('Deletion stage failed for: %s' % self.p2file)
-                       return -rsutil.common.EDELETE
+               try:
+                       self.runDelete()
+               except (RuntimeError, OSError):
+                       logging.critical('Deletion stage failed for: %s' % self.p2file)
+                       raise

 
                logging.info ('Successfully completed: %s' % self.p2file)
 
                logging.info ('Successfully completed: %s' % self.p2file)

-               return rsutil.common.SUCCESS

 
 

diff --git a/PAR2Set/Join.py b/PAR2Set/Join.py
index 61a1c42..1dd680a 100644 (file)
--- a/PAR2Set/Join.py
+++ b/PAR2Set/Join.py
@@ -75,28 +75,7 @@ class Join (PAR2Set.Base.Base):
 
                   This is done using the par2repair command by default"""
 
 
                   This is done using the par2repair command by default"""
 

-               PAR2_CMD = rsutil.common.config_get_value ('commands', 'par2repair')
-
-               # assemble the command
-               # par2repair -- PAR2 PAR2_EXTRA [JOIN_FILES]
-               command = "%s \"%s\" " % (PAR2_CMD, self.p2file)
-
-               for f in self.all_p2files:
-                       if f != self.p2file:
-                               command += "\"%s\" " % os.path.split (f)[1]
-
-               for f in self.find_joinfiles ():
-                       command += "\"%s\" " % os.path.split (f)[1]
-
-               # run the command
-               ret = rsutil.common.run_command (command, self.dir)
-
-               # check the result
-               if ret != 0:
-                       logging.critical ('PAR2 Check / Repair failed: %s' % self.p2file)
-                       return -rsutil.common.ECHECK
-
-               return rsutil.common.SUCCESS
+               rsutil.common.run_command(['par2repair'] + self.all_p2files + self.find_joinfiles(), self.dir)

 
        def find_deleteable_files (self):
                """Find all files which are deletable by using the regular expression from the
 
        def find_deleteable_files (self):
                """Find all files which are deletable by using the regular expression from the


@@ -123,20 +102,5 @@ class Join (PAR2Set.Base.Base):
                   This command ignores the extraction if file and todir+file are the same
                   file. This keeps things like mv working smoothly."""
 
                   This command ignores the extraction if file and todir+file are the same
                   file. This keeps things like mv working smoothly."""
 

-               NOEXTRACT_CMD = rsutil.common.config_get_value ('commands', 'noextract')
-
-               # Make sure that both files are not the same file. If they are, don't run at all.
-               # NOTE: os.path.samefile() doesn't exist on win32, so we can't use it.
-               if rsutil.common.full_abspath (file) == \
-                               rsutil.common.full_abspath (os.path.join (todir, file)):
-                       return rsutil.common.SUCCESS
-
-               cmd = NOEXTRACT_CMD % (file, todir)
-               ret = rsutil.common.run_command (cmd)
-
-               # Check error code
-               if ret != 0:
-                       return -rsutil.common.EEXTRACT
-
-               return rsutil.common.SUCCESS
-
+               # The Join type doesn't need any extraction
+               pass

diff --git a/PAR2Set/NewRAR.py b/PAR2Set/NewRAR.py
index caefb29..7c8f25b 100644 (file)
--- a/PAR2Set/NewRAR.py
+++ b/PAR2Set/NewRAR.py
@@ -72,14 +72,5 @@ class NewRAR (PAR2Set.Base.Base):
                assert os.path.isfile (file)
                assert os.path.isdir (todir)
 
                assert os.path.isfile (file)
                assert os.path.isdir (todir)
 

-               RAR_CMD = rsutil.common.config_get_value ('commands', 'unrar')
-
-               cmd = '%s \"%s\"' % (RAR_CMD, file)
-               ret = rsutil.common.run_command (cmd, todir)
-
-               # Check error code
-               if ret != 0:
-                       return -rsutil.common.EEXTRACT
-
-               return rsutil.common.SUCCESS
+               rsutil.common.run_command(['unrar', 'x', '-o+', file], todir)

 
 

diff --git a/PAR2Set/NoExtract.py b/PAR2Set/NoExtract.py
index 51ee235..9b4ea46 100644 (file)
--- a/PAR2Set/NoExtract.py
+++ b/PAR2Set/NoExtract.py
@@ -94,21 +94,20 @@ class NoExtract (PAR2Set.Base.Base):
                """Run the Repair and Deletion stages, omitting the Extraction stage"""
 
                # Repair Stage
                """Run the Repair and Deletion stages, omitting the Extraction stage"""
 
                # Repair Stage

-               ret = self.runVerifyAndRepair ()
-
-               if ret != rsutil.common.SUCCESS:
-                       logging.critical ('Repair stage failed for: %s' % self.p2file)
-                       return -rsutil.common.ECHECK
+               try:
+                       self.runVerifyAndRepair()
+               except (RuntimeError, OSError):
+                       logging.critical('Repair stage failed for: %s' % self.p2file)
+                       raise

 
                self.update_matches ()
 
                # Deletion Stage
 
                self.update_matches ()
 
                # Deletion Stage

-               ret = self.runDelete ()
-
-               if ret != rsutil.common.SUCCESS:
-                       logging.critical ('Deletion stage failed for: %s' % self.p2file)
-                       return -rsutil.common.EDELETE
+               try:
+                       self.runDelete()
+               except (RuntimeError, OSError):
+                       logging.critical('Delete stage failed for: %s' % self.p2file)
+                       raise

 
                logging.info ('Successfully completed: %s' % self.p2file)
 
                logging.info ('Successfully completed: %s' % self.p2file)

-               return rsutil.common.SUCCESS

 
 

diff --git a/PAR2Set/OldRAR.py b/PAR2Set/OldRAR.py
index b872ccf..b139570 100644 (file)
--- a/PAR2Set/OldRAR.py
+++ b/PAR2Set/OldRAR.py
@@ -74,14 +74,5 @@ class OldRAR (PAR2Set.Base.Base):
                assert os.path.isfile (file)
                assert os.path.isdir (todir)
 
                assert os.path.isfile (file)
                assert os.path.isdir (todir)
 

-               RAR_CMD = rsutil.common.config_get_value ('commands', 'unrar')
-
-               cmd = '%s \"%s\"' % (RAR_CMD, file)
-               ret = rsutil.common.run_command (cmd, todir)
-
-               # Check error code
-               if ret != 0:
-                       return -rsutil.common.EEXTRACT
-
-               return rsutil.common.SUCCESS
+               rsutil.common.run_command(['unrar', 'x', '-o+', file], todir)

 
 

diff --git a/PAR2Set/ZIP.py b/PAR2Set/ZIP.py
index 36acb74..39a83ff 100644 (file)
--- a/PAR2Set/ZIP.py
+++ b/PAR2Set/ZIP.py
@@ -69,14 +69,5 @@ class ZIP (PAR2Set.Base.Base):
                   file -- the file to extract
                   todir -- the directory to extract into"""
 
                   file -- the file to extract
                   todir -- the directory to extract into"""
 

-               ZIP_CMD = rsutil.common.config_get_value ('commands', 'unzip')
-
-               cmd = ZIP_CMD % (file, todir)
-               ret = rsutil.common.run_command (cmd)
-
-               # Check error code
-               if ret != 0:
-                       return -rsutil.common.EEXTRACT
-
-               return rsutil.common.SUCCESS
+               rsutil.common.run_command(['unzip', file], todir)

 
 

diff --git a/RarslaveDetector.py b/RarslaveDetector.py
index 73a442b..dfb2768 100644 (file)
--- a/RarslaveDetector.py
+++ b/RarslaveDetector.py
@@ -82,6 +82,7 @@ class RarslaveDetector (object):
                   the class is valid."""
 
                detected = False
                   the class is valid."""
 
                detected = False

+               success = False

 
                for (detector, classname) in self.TYPES:
                        if detector (self.name_matched_files, self.prot_matched_files):
 
                for (detector, classname) in self.TYPES:
                        if detector (self.name_matched_files, self.prot_matched_files):


@@ -93,15 +94,18 @@ class RarslaveDetector (object):
                                logging.debug ('Detected type: %s' % p2set)
 
                                # Try to have rarslave do it's thing
                                logging.debug ('Detected type: %s' % p2set)
 
                                # Try to have rarslave do it's thing

-                               ret = p2set.runAll ()
+                               try:
+                                       # Have rarslave do it's thing
+                                       p2set.runAll ()

 
 

-                               # If something already worked, there is no need to continue,
-                               # since we've already finished!
-                               if ret == rsutil.common.SUCCESS:
+                                       # It succeeded, exit the loop early
+                                       success = True

                                        break
                                        break

-                               else:
-                                       logging.error ('Detected type (%s) failed for: %s' % (p2set,
-                                                       self.p2file))
+                               except (OSError, RuntimeError):
+                                       logging.error('Detected type (%s) failed for: %s' % (p2set, self.p2file))
+                               except:
+                                       logging.error('Unknown exception occurred')
+                                       raise

 
                # Make sure we detected at least one valid type
                if not detected:
 
                # Make sure we detected at least one valid type
                if not detected:


@@ -109,13 +113,10 @@ class RarslaveDetector (object):
                        logging.debug ('The following information will help in writing a detector:')
                        logging.debug ('name_matches: %s' % self.name_matched_files)
                        logging.debug ('prot_matches: %s' % self.prot_matched_files)
                        logging.debug ('The following information will help in writing a detector:')
                        logging.debug ('name_matches: %s' % self.name_matched_files)
                        logging.debug ('prot_matches: %s' % self.prot_matched_files)

-                       return -rsutil.common.EDETECT

 
                # Make sure that something worked
 
                # Make sure that something worked

-               if ret != rsutil.common.SUCCESS:
-                       logging.critical ('All types failed for: %s' % self.p2file)
-
-               return ret
+               if success == False:
+                       logging.critical('All types failed for: %s' % self.p2file)

 
 def main ():
        pass
 
 def main ():
        pass

diff --git a/rarslave.py b/rarslave.py
index 7c1b4c8..18bb540 100755 (executable)
--- a/rarslave.py
+++ b/rarslave.py
@@ -122,17 +122,28 @@ def generate_all_parsets (dir):
 def check_required_progs():
        """Check if the required programs are installed"""
 
 def check_required_progs():
        """Check if the required programs are installed"""
 

-       shell_not_found = 32512

        needed = []
 
        needed = []
 

-       if rsutil.common.run_command ('par2repair --help > /dev/null 2>&1') == shell_not_found:
-               needed.append ('par2repair')
+       try:
+               rsutil.common.run_command(['par2repair', '--help'])
+       except OSError:
+               needed.append('par2repair')
+       except RuntimeError:
+               pass

 
 

-       if rsutil.common.run_command ('unrar --help > /dev/null 2>&1') == shell_not_found:
-               needed.append ('unrar')
+       try:
+               rsutil.common.run_command(['unrar', '--help'])
+       except OSError:
+               needed.append('unrar')
+       except RuntimeError:
+               pass

 
 

-       if rsutil.common.run_command ('unzip --help > /dev/null 2>&1') == shell_not_found:
-               needed.append ('unzip')
+       try:
+               rsutil.common.run_command(['unzip', '--help'])
+       except OSError:
+               needed.append('unzip')
+       except RuntimeError:
+               pass

 
        if needed:
                for n in needed:
 
        if needed:
                for n in needed:


@@ -169,6 +180,7 @@ def run_options (options):
 
        if options.check_progs:
                check_required_progs ()
 
        if options.check_progs:
                check_required_progs ()

+               sys.exit (0)

 
        if options.write_def_config:
                config.write_config (default=True)
 
        if options.write_def_config:
                config.write_config (default=True)


@@ -264,14 +276,14 @@ def main ():
                        parsets = generate_all_parsets (dir)
                        for (p2dir, p2file) in parsets:
                                detector = RarslaveDetector.RarslaveDetector (p2dir, p2file)
                        parsets = generate_all_parsets (dir)
                        for (p2dir, p2file) in parsets:
                                detector = RarslaveDetector.RarslaveDetector (p2dir, p2file)

-                               ret = detector.runMatchingTypes ()
+                               detector.runMatchingTypes ()

 
        # Non-recursive
        else:
                parsets = generate_all_parsets (options.work_dir)
                for (p2dir, p2file) in parsets:
                        detector = RarslaveDetector.RarslaveDetector (p2dir, p2file)
 
        # Non-recursive
        else:
                parsets = generate_all_parsets (options.work_dir)
                for (p2dir, p2file) in parsets:
                        detector = RarslaveDetector.RarslaveDetector (p2dir, p2file)

-                       ret = detector.runMatchingTypes ()
+                       detector.runMatchingTypes ()

 
        # Print the results
        if logger.size () > 0:
 
        # Print the results
        if logger.size () > 0:

diff --git a/rsutil/common.py b/rsutil/common.py
index 66b7d02..370d7d6 100644 (file)
--- a/rsutil/common.py
+++ b/rsutil/common.py
@@ -29,13 +29,12 @@ __license__   = "GNU GPL v2 (or, at your option, any later version)"
 import os
 import re
 import logging
 import os
 import re
 import logging

+import subprocess
+import exceptions

 
 import rsutil.globals
 import rsutil.par2parser
 
 
 import rsutil.globals
 import rsutil.par2parser
 

-# Global constants
-(SUCCESS, ECHECK, EEXTRACT, EDELETE, ECREATE, EDETECT, EPARSE) = range(7)
-

 def find_matches (regex, li, ignorecase=True):
 
        if ignorecase:
 def find_matches (regex, li, ignorecase=True):
 
        if ignorecase:


@@ -66,14 +65,22 @@ def run_command (cmd, indir=None):
        # Runs the specified command-line in the directory given (or, in the current directory
        # if none is given). It returns the status code given by the application.
 
        # Runs the specified command-line in the directory given (or, in the current directory
        # if none is given). It returns the status code given by the application.
 

-       pwd = os.getcwd ()
-
-       if indir != None:
+       if indir == None:
+               indir = os.getcwd()
+       else:

                assert os.path.isdir (indir) # MUST be a directory!
                assert os.path.isdir (indir) # MUST be a directory!

-               os.chdir (indir)

 
 

-       ret = os.system (cmd)
-       os.chdir (pwd)
+       print 'RUNNING COMMAND'
+       print 'Directory: %s' % indir
+       print 'Command: %s' % cmd[0]
+       for f in cmd[1:]:
+               print '-> %s' % f
+
+       ret = subprocess.Popen(cmd, cwd=indir).wait()
+
+       if ret != 0:
+               raise RuntimeError
+

        return ret
 
 def full_abspath (p):
        return ret
 
 def full_abspath (p):


@@ -143,11 +150,7 @@ def list_eq (l1, l2):
        if len(l1) != len(l2):
                return False
 
        if len(l1) != len(l2):
                return False
 

-       for e in l1:
-               if e not in l2:
-                       return False
-
-       return True
+       return set(l1) == set(l2)

 
 # Convience functions for the config
 
 
 # Convience functions for the config