Change inappropriate TypeError raises

[maphew]

I've not been using Raise or Exceptions properly. Some of that is inheritance from the original cyg-apt script, which just used a bare raise 'string message' syntax, and some of it is me, in ignorance, using TypeError all over the place, even when the error is not a type error. I've since learned enough to recognize the issue, but don't think I know enough about exception handling to solve it properly. We'll see I guess ;-)

[maphew]

Starting this with aacc767, and asked on SO, Hide traceback unless a debug flag is set

[maphew]

Required reading on properly using Exceptions: http://stackoverflow.com/questions/2052390/how-do-i-manually-throw-raise-an-exception-in-python.

Not only does @aaronchall cleanly describe what to do, he's doing so in a very old question. It's a perfect example of what the Stack Exchange network is capable of (as well illustrating a significant SE wart: most votes doesn't automatically mean best answer). Aaron has given me a model to emulate and aspire to.

[amr66]

I like to suggest an Error Handling in do_download. URL-Downloads are critical if servers are down etc.
Just Yesterday apt crashed while downloading pkg shell due to bad connection to the server.
I think a failed download should not lead to sys.exit() either, because apt can't react to this error. My suggested error-handling looks like this:

    try:
        a = urllib.urlopen(srcFile)
    except IOErrror as e:
        print "I/O error({0}): {1}".format(e.errno, e.strerror)
        return -1

    if not a.getcode() is 200:
        msg = 'Problem getting %s\nServer returned "%s"' % (srcFile, a.getcode())
        print msg
        return -2

later on in do_download we have urlretrieve:

    try:
        status = urllib.urlretrieve(srcFile, dstFile, down_stat)
    except IOError as e:
        print "I/O error({0}): {1}".format(e.errno, e.strerror)
        return -3

I also suggest to evaluate a return value of do_download in download()

for p in packages:
        if do_download(p) > 0:
            ball(p)
            md5(p)
        else:
            print "Download for Package", p, "failed!"

Of course apt could exit() on certain errors, but because i try to use it as a module, return helps me a lot.

[maphew]

Those changes look good to me. Thank you Andreas! I encourage you to fork and issue a pull request with the changes applied. I've not handled pull requests from the receiving side and this would be a good opportunity to learn how to use that aspect of git/github.

Using sys.exit() is something inherited from the original version, I'm not wedded to using them.

What is the significance of the numbers used in the returns? (-2, -3, ...)

[amr66]

Sorry, late response:
I will try to do a pull request, but i have to figure it out first.
Concerning the return values: You don't need them, but they can be used by the caller function, to see, if it ended correctly or with errors.

if myfunction() < 0:
    do_something_usefull

[amr66]

A possibly better way for error handling would not use return values but raise a specific error wich can be catched outside the function. One would do it like here:

errors = []
try:
...
except Error, err:
    errors.extend(err.args[0])

the idea comes from the shutils modul. Here the function copytree uses the technique of collecting errors and ends up like this:

if errors:
    raise Error, errors

i don't understand the last line and it seems, it's the python way < 2.6 -> http://stackoverflow.com/questions/6470428/catch-multiple-exceptions-in-one-line-except-block
So i did my own copytree with a new Exception class. At the end i throw my quick and dirty CopyError(errors) - (Haven't tested heavily):

import os
import shutil as s

class CopyError(Exception):
    def __init__(self, errors):
        self.message = "".join(errors)
    def __str__(self):
        return repr(self.message)

def copytree(src, dst, symlinks=False, ignore=None):
    """Recursively copy a directory tree using copy2().

    ...

    """
    names = os.listdir(src)
    if ignore is not None:
        ignored_names = ignore(src, names)
    else:
        ignored_names = set()

    errors = []

    try:
        os.makedirs(dst)
    except OSError as e:
        if WindowsError is not None and isinstance(e, WindowsError):
            # Makedirs fails on existing dirs
            pass
        else:
            errors.append((src, dst, str(why)))

    for name in names:
        if name in ignored_names:
            continue
        srcname = os.path.join(src, name)
        dstname = os.path.join(dst, name)
        try:
            if symlinks and os.path.islink(srcname):
                linkto = os.readlink(srcname)
                os.symlink(linkto, dstname)
            elif os.path.isdir(srcname):
                copytree(srcname, dstname, symlinks, ignore)
            else:
                # Will raise a SpecialFileError for unsupported file types
                s.copy2(srcname, dstname)
        # catch the Error from the recursive copytree so that we can
        # continue with other files
        except Error, err:
            errors.extend(err.args[0])
        except EnvironmentError, why:
            errors.append((srcname, dstname, str(why)))
    try:
        s.copystat(src, dst)
    except OSError as e:
        if WindowsError is not None and isinstance(why, WindowsError):
            # Copying file access times may fail on Windows
            pass
        else:
            errors.append((src, dst, str(why)))
    if errors:
        print "error: ", errors
        raise CopyError(errors)

[maphew]

@amr66, I think this #12 (comment) might be why you're not getting enough usable exceptions when things go awry. Try setting apt.debug = True in your module.

[maphew]

I'm going to close this as fixed, good enough for now.

That said, I wont be surprised if there are related follow up issues, at which time we can re-open this ticket, or create new ones focussed on the specific error/situations encountered.