views:

1304

answers:

7

Hello there everyone, I have searched for other posts, as I felt this is a rather common problem, but all other Python exception questions I have found didn't reflect my problem.

I will try to be as specific here as I can, so I will give a direct example. And pleeeeease do not post any workarounds for this specific problem. I am not specifically interested how you can send an email much nicer with xyz. I want to know how you generally deal with dependent, error prone statements.

My question is, how to handle exceptions nicely, ones that depend on one another, meaning: Only if the first step was successful, try the next, and so on. One more criterion is: All exceptions have to be caught, this code has to be robust.

For your consideration, an example:

try:
    server = smtplib.SMTP(host) #can throw an exception
except smtplib.socket.gaierror:
    #actually it can throw a lot more, this is just an example
    pass
else: #only if no exception was thrown we may continue
    try:
        server.login(username, password)
    except SMTPAuthenticationError:
        pass # do some stuff here
    finally:
        #we can only run this when the first try...except was successful
        #else this throws an exception itself!
        server.quit() 
    else:
        try:
            # this is already the 3rd nested try...except
            # for such a simple procedure! horrible
            server.sendmail(addr, [to], msg.as_string())
            return True
        except Exception:
            return False
        finally:
            server.quit()

return False

This looks extremely unpythonic to me, and the error handling code is triple the real business code, but on the other hand how can I handle several statements that are dependent on one another, meaning statement1 is prerequisite for statement2 and so on?

I am also interested in proper resource cleanup, even Python can manage that for itself.

Thanks, Tom

A: 

Why not one big try: block? This way, if any exception is caught, you'll go all the way to the except. And as long as all the exceptions for the different steps are different, you can always tell which part it was that fired the exception.

Jacob B
because one big try block wont give you the chance to take care of ressources. E.g.: statement1 worked, and opened a file, but statement 2 throws an exception. You cant handle that in one single finally block, because you never know if the resource was actually allocated or not. Also, some steps can fire the same Exceptions, so it cant be determined what went wrong later on, and you cant print errors, because youre not sure which statement actually failed.
Tom
Nested with blocks, then? http://docs.python.org/whatsnew/2.5.html#pep-343
Jacob B
sadly i cant rely on __enter__ and __exit__ methods to be defined for all operations i will use, so with might always not work
Tom
But you can always build your own wrappers--they're lightweight enough, especially if you use decorators, and worth it if they can make your code more readable.
Jacob B
+10  A: 

In general, you want to use as few try blocks as possible, distinguishing failure conditions by the kinds of exceptions they throw. For instance, here's my refactoring of the code you posted:

try:
    server = smtplib.SMTP(host)
    server.login(username, password) # Only runs if the previous line didn't throw
    server.sendmail(addr, [to], msg.as_string())
    return True
except smtplib.socket.gaierror:
    pass # Couldn't contact the host
except SMTPAuthenticationError:
    pass # Login failed
except SomeSendMailError:
    pass # Couldn't send mail
finally:
    if server:
        server.quit()
return False

Here, we use the fact that smtplib.SMTP(), server.login(), and server.sendmail() all throw different exceptions to flatten the tree of try-catch blocks. In the finally block we test server explicitly to avoid invoking quit() on the nil object.

We could also use three sequential try-catch blocks, returning False in the exception conditions, if there are overlapping exception cases that need to be handled separately:

try:
    server = smtplib.SMTP(host)
except smtplib.socket.gaierror:
    return False # Couldn't contact the host

try:
    server.login(username, password)
except SMTPAuthenticationError:
    server.quit()
    return False # Login failed

try:
    server.sendmail(addr, [to], msg.as_string())
except SomeSendMailError:
    server.quit()
    return False # Couldn't send mail

return True

This isn't quite as nice, as you have to kill the server in more than one place, but now we can handle specific exception types different ways in different places without maintaining any extra state.

David Seiler
the point is, as stated above, that they do not throw individual exceptions, so it cant be trivially flattened. In case your connection gets interrupted before you can auth, server.login and server.sendMail could throw the same exception ("connect to server first")But as i also stated above, I am not looking for a solution to this specific problem. I am more interested in a general approach how to solve this.Your second approach is basically my code without "else". its more pretty though i have to admit ;)
Tom
Watch out on the finally block — you'll want to set server to None before the block in order to avoid accidentally referencing a non-existent variable.
Paul Fisher
@Tom +1, that's why I didn't suggest this solution.
Unknown
If each line of your program can throw several different exceptions, and each needs to be handled individually, then the bulk of your code is going to be exception handling. There's nothing bad or unpythonic about that. Just make sure you've picked an approach that scales: if you have ten distinct code groups, each of which can throw 4 different exceptions, then it becomes *really important* that each code group not introduce a new level of indentation ;). That's what the flattening is for, and that's why I'd be wary of using else:, no matter what the docs say.
David Seiler
A: 

I like David's answer but if you are stuck on the server exceptions you can also check for server if is None or states. I flattened out the method a bit bit it is still a but unpythonic looking but more readable in the logic at the bottom.

server = None 

def server_obtained(host):
    try:
        server = smtplib.SMTP(host) #can throw an exception
        return True
    except smtplib.socket.gaierror:
        #actually it can throw a lot more, this is just an example
        return False

def server_login(username, password):
    loggedin = False
    try:
        server.login(username, password)
        loggedin = True
    except SMTPAuthenticationError:
        pass # do some stuff here
    finally:
        #we can only run this when the first try...except was successful
        #else this throws an exception itself!
        if(server is not None):
            server.quit()
    return loggedin

def send_mail(addr, to, msg):
    sent = False
     try:
        server.sendmail(addr, to, msg)
        sent = True
    except Exception:
        return False
    finally:
        server.quit()
    return sent

def do_msg_send():
    if(server_obtained(host)):
        if(server_login(username, password)):
            if(send_mail(addr, [to], msg.as_string())):
                return True
    return False
Ryan Christensen
in both server_login and send_mail you can avoid the local variable, because finally will always execute, even if you use a "return" in a try or except block :) you can simply return True in the try block, and False in the except block instead of saving the state in a local var.
Tom
+1  A: 

Just using one try-block is the way to go. This is exactly what they are designed for: only execute the next statement if the previous statement did not throw an exception. As for the resource clean-ups, maybe you can check the resource if it needs to be cleaned up (e.g. myfile.is_open(), ...) This does add some extra conditions, but they will only be executed in the exceptional case. To handle the case that the same Exception can be raised for different reasons, you should be able to retrieve the reason from the Exception.

I suggest code like this:

server = None
try:
    server = smtplib.SMTP(host) #can throw an exception
    server.login(username, password)
    server.sendmail(addr, [to], msg.as_string())
    server.quit()
    return True
except smtplib.socket.gaierror:
    pass # do some stuff here
except SMTPAuthenticationError:
    pass # do some stuff here
except Exception, msg:
    # Exception can have several reasons
    if msg=='xxx':
        pass # do some stuff here
    elif:
        pass # do some other stuff here

if server:
    server.quit()

return False

It is no uncommon, that error handling code exceeds business code. Correct error handling can be complex. But to increase maintainability it helps to separate the business code from the error handling code.

Ralph
i disagree, because as i stated quite a few times above, the error messages would be ambiguous, as 2 different function calls, for example login and sendmail could throw the same exception. If you want to print to the user, or to your log: "login() failed because of xyz" or "sendmail() failed because of xyz" this isnt possible, because both calls may result in the same exception. I want detailed handling what went wrong for logging purposes.
Tom
The exception should be able to provide its details. E.g. you can use "except gaierror, (code, message):", instead of simple "except gaierror:". Then you have the error code and error message and can use them for detailed error handling, e.g. if code == 11001: print "unknown host name:", message
Ralph
I think you didnt fully get what I was trying to say. Try the following: make yourself an SMTP object, and then try smtp.login() without connecting, and then smtp.sendmail() without connecting, you will see they throw 100% identical exceptions, that you cant distinguish, neither by msg nor by errno
Tom
I see. In this case it is probably best to add a wrapper function, which translates the exception (as others have suggested).
Ralph
+15  A: 

Instead of using the try/except's else block, you could simply return when it errors:

def send_message(addr, to, msg):
    ## Connect to host
    try:
        server = smtplib.SMTP(host) #can throw an exception
    except smtplib.socket.gaierror:
        return False

    ## Login
    try:
        server.login(username, password)
    except SMTPAuthenticationError:
        server.quit()
        return False

    ## Send message
    try:
        server.sendmail(addr, [to], msg.as_string())
        return True
    except Exception: # try to avoid catching Exception unless you have too
        return False
    finally:
        server.quit()

That's perfectly readable and Pythonic..

Another way of doing this is, rather than worry about the specific implementation, decide how you want your code to look, for example..

sender = MyMailer("username", "password") # the except SocketError/AuthError could go here
try:
    sender.message("addr..", ["to.."], "message...")
except SocketError:
    print "Couldn't connect to server"
except AuthError:
    print "Invalid username and/or password!"
else:
    print "Message sent!"

Then write the code for the message() method, catching any errors you expect, and raising your own custom one, and handle that where it's relevant. Your class may look something like..

class ConnectionError(Exception): pass
class AuthError(Exception): pass
class SendError(Exception): pass

class MyMailer:
    def __init__(self, host, username, password):
        self.host = host
        self.username = username
        self.password = password

    def connect(self):
        try:
            self.server = smtp.SMTP(self.host)
        except smtplib.socket.gaierror:
            raise ConnectionError("Error connecting to %s" % (self.host))

    def auth(self):
        try:
            self.server.login(self.username, self.password)
        except SMTPAuthenticationError:
            raise AuthError("Invalid username (%s) and/or password" % (self.username))

    def message(self, addr, to, msg):
        try:
            server.sendmail(addr, [to], msg.as_string())
        except smtplib.something.senderror, errormsg:
            raise SendError("Couldn't send message: %s" % (errormsg))
        except smtp.socket.timeout:
            raise ConnectionError("Socket error while sending message")
dbr
+1 I really like how you have solved the "library only uses one exception for everything" problem
Tom Leys
In your first example, send_message() will always return after the server.login() and never send the message. I don't think that there should be a finally for this statement.
mhawke
then now it boils down to a question of principle. your first code is basically the same as mine, except that you simply dont nest the exceptions, as I did in an "else"-tree, which was suggested in the python docs. which one is better practise? the docs state that else should always be preferred rather than additional statements in the try block. its basically the same question with if's: is it better to return before another if, or is it better to nest the ifs conditionally.
Tom
Generally I don't think you should nestle try/except blocks, especially not 3 levels, or with as much code as in the question - maybe for sequentially trying to import a module (like try to `import cElementTree`, then `ElementTree`, finally try `lxml.et.ElementTree` for example)
dbr
I chose this as the accepted answer because I think my nested exception handling is not very readable although I thought it would be the proposed way using the else-statement of try...except block. For your second example: It's a nice way of handling this problem, but generally repackaging is considered bad practise. Thanks for your valuable input dbr :)
Tom
No problem! "but generally repackaging is considered bad practise" I don't consider my second code repackaging, it's just taking advantage of classes (which in Python are just ways of grouping together a bunch of functions/variables) - I'm not suggesting you redistribute that class alone, just treat it like any another function
dbr
+3  A: 

If it was me I would probably do something like the following:

try:
    server = smtplib.SMTP(host)
    try:
        server.login(username, password)
        server.sendmail(addr, [to], str(msg))
    finally:
        server.quit()
except:
    debug("sendmail", traceback.format_exc().splitlines()[-1])
    return True

All errors are caught and debugged, the return value == True on success, and the server connection is properly cleaned up if the initial connection is made.

Peter Ericson
this looks intuitive to me, and this is probably also what it would look like in java, because you dont have the luxury of an "else" statement there. You also didnt have this before python2.5 iirc. It was introduced in the docs to avoid large try blocks but still keep the grouping nice and obvious, so all code that belongs together will still be in the same try...except...else block, without blowing the try part out of proprtions. as the pythonians are so attached to their style guides and peps, i figured this would be the correct way to go.
Tom
+1  A: 

I would try something like this:

class Mailer():

    def send_message(self):
        exception = None
        for method in [self.connect, 
                       self.authenticate, 
                       self.send, 
                       self.quit]:
            try:
                if not method(): break
            except Exception, ex:
                exception = ex
                break

        if method == quit and exception == None:
            return True

        if exception:
            self.handle_exception(method, exception)
        else:
            self.handle_failure(method)

    def connect(self):
        return True

    def authenticate(self):
        return True

    def send(self):
        return True

    def quit(self):
        return True

    def handle_exception(self, method, exception):
        print "{name} ({msg}) in {method}.".format(
           name=exception.__class__.__name__, 
           msg=exception,
           method=method.__name__)

    def handle_failure(self, method):
        print "Failure in {0}.".format(method.__name__)

All of the methods (including send_message, really) follow the same protocol: they return True if they succeeded, and unless they actually handle an exception, they don't trap it. This protocol also makes it possible to handle the case where a method needs to indicate that it failed without raising an exception. (If the only way your methods fail is by raising an exception, that simplifies the protocol. If you're having to deal with a lot of non-exception failure states outside of the method that failed, you probably have a design problem that you haven't worked out yet.)

The downside of this approach is that all of the methods have to use the same arguments. I've opted for none, with the expectation that the methods I've stubbed out will end up manipulating class members.

The upside of this approach is considerable, though. First, you can add dozens of methods to the process without send_message getting any more complex.

You can also go crazy and do something like this:

def handle_exception(self, method, exception):
    custom_handler_name = "handle_{0}_in_{1}".format(\
                                             exception.__class__.__name__,
                                             method.__name__)
    try:
        custom_handler = self.__dict__[custom_handler_name]
    except KeyError:
        print "{name} ({msg}) in {method}.".format(
           name=exception.__class__.__name__, 
           msg=exception,
           method=method.__name__)
        return
    custom_handler()

def handle_AuthenticationError_in_authenticate(self):
   print "Your login credentials are questionable."

...though at that point, I might say to myself, "self, you're working the Command pattern pretty hard without creating a Command class. Maybe now is the time."

Robert Rossney