[openstack-dev] [Review] Use of exception for non-exceptional cases

Monty Taylor mordred at inaugust.com
Fri Jul 12 15:09:49 UTC 2013



On 07/11/2013 05:20 AM, Mark McLoughlin wrote:
> On Wed, 2013-07-10 at 19:49 -0400, Monty Taylor wrote:
>> I'd like top-post and hijack this thread for another exception related
>> thing:
>>
>> a) Anyone writing code such as:
>>
>> try:
>>   blah()
>> except SomeException:
>>   raise SomeOtherExceptionLeavingOutStackContextFromSomeException
>>
>> should be mocked ruthlessly.
> 
> Ok, mock me ruthlessly then.

Mock. Mock. Mock. Mock.

> Part of designing any API is specifying what predictable exceptions it
> will raise. For any predictable error condition, you don't want callers
> to have to catch random exceptions from the underlying libraries you
> might be calling into.

Absolutely. But you don't want to go so overboard that you remove the
ability for a developer to debug it. More importantly, INSIDE of server
code, we're not designing fun apis for the consumption of people we
don't know. There is clearly an API, but we are the consumers of our own
API.

> Say if I was designing an image downloading API, I'd do something like
> this:
> 
>   https://gist.github.com/markmc/5973868
> 
> Assume there's a tonne more stuff that the API would do. You don't want
> callers to have to catch socket.error exceptions and whatever other
> exceptions might be thrown.
>
> That works out as:
> 
>   Traceback (most recent call last):
>     File "t.py", line 20, in <module>
>       download_image('localhost', 3366, 'foobar')
>     File "t.py", line 18, in download_image
>       raise ImageDownloadFailure(host, port, path, e.strerror)
>   __main__.ImageDownloadFailure: Failed to download foobar from localhost:3366: Connection refused
> 
> Which is a pretty clear exception.

Right, but what if the message from the exception does not give you
enough information to walk down the stack and see it...

Also, I have more than once seen:

class MyIOError(Exception):
    pass

try:
    s = socket.create_connection((host, port))
except socket.error:
    raise MyIOError("something went wrong!")

Which is an extreme example of where my rage comes from, but not a made
up example. I have, actually, seen code exactly like that - in Nova.

BTW - I'd have download_image return None if it can't download the
image, and I'd have it either deal with the socket.error or not locally
at the source. But now we're nitpicking.

> But I think what you're saying is missing is the stack trace from the
> underlying exception.

YES - and I'll let David's response respond to the details of the rest...

But essentially, what I was really mocking earlier is throwing a new
exception in such a way that you lose the exception propagation path. So
if you throw an exception inside of an except block, you should be sure
that you're passing on all of the info, because if a developer gets an
exception, it's quite likely that they want to know how to fix it. :)

> As I understood it, Python doesn't have a way of chaining exceptions
> like this but e.g. Java does. A little bit more poking right now shows
> up this:
> 
>   http://www.python.org/dev/peps/pep-3134/
> 
> i.e. we can't do the right thing until Python 3, where we'd do:
> 
>  def download_image(host, port, path):
>      try:
>          s = socket.create_connection((host, port))
>      except socket.error as e:
>          raise ImageDownloadFailure(host, port, path, e.strerror) from e

This will certainly be cleaner to write and read.

> I haven't read the PEP in detail yet, though.
> 
> Cheers,
> Mark.
> 
> 
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 



More information about the OpenStack-dev mailing list