[openstack-dev] [Review] Use of exception for non-exceptional cases
Mark Washenberger
mark.washenberger at markwash.net
Sat Jul 13 00:29:12 UTC 2013
I'm curious if folks know about this?
import sys
def foo():
raise Exception('Foo Exception')
def bar():
try:
foo()
except Exception:
exc_info = sys.exc_info()
raise Exception('Bar Exception'), None, exc_info[2]
bar()
"""
Traceback (most recent call last):
File "test.py", line 15, in <module>
bar()
File "test.py", line 9, in bar
foo()
File "test.py", line 4, in foo
raise Exception('Foo Exception')
Exception: Bar Exception
"""
On Fri, Jul 12, 2013 at 3:53 PM, Doug Hellmann
<doug.hellmann at dreamhost.com>wrote:
>
>
>
> On Fri, Jul 12, 2013 at 2:40 PM, Brant Knudson <blk at acm.org> wrote:
>
>>
>> Somewhat offtopic, but others might find it interesting. On another
>> project I used a different technique for exceptions, where the original
>> exception is "enhanced" with new fields as it propagates up the stack to
>> where it's handled. In this way all the information needed to handle the
>> exception is available (even if it's just to log it).
>>
>> Often there's some information that would be useful for the exception
>> handler that isn't available at the place where the exception is raised.
>> The classic example is an error writing to a file where you'd like to know
>> the filename but all you've got is the file descriptor. An example from an
>> OpenStack REST API might be that you get an exception and you'd like to log
>> a message that includes the resource path, but the resource path isn't
>> known at this point.
>>
>> So rather than logging the exception when it happens, enhance the
>> exception, re-raise it, and only once it's got all the information where
>> you can print out a useful log message you log it.
>>
>> Here's a short example to illustrate. An exception is raised by f1(), but
>> at this point we don't know the status code that the server should respond
>> with or the request path which would be useful in a log message. These bits
>> of information are added as the exception propagates and then where it's
>> caught we've got all the information for a useful log message.
>>
>> def f1():
>> raise Exception('something')
>>
>>
>> def f2():
>> try:
>> f1()
>> except Exception as e:
>> e.status_code = 403
>> raise
>>
>>
>> def do_tokens():
>> try:
>> f2()
>> except Exception as e:
>> e.req_url = '/v3/tokens'
>> raise
>>
>>
>> status_code = 200
>> try:
>> do_tokens()
>> except Exception as e:
>> status_code = getattr(e, 'status_code', 500)
>> req_url = getattr(e, 'req_url', 'unknown')
>>
>> if status_code != 200:
>> print 'Got %s from %s' % (status_code, req_url)
>> # build an error document for the response.
>>
>
>
> One problem with this approach is it spreads knowledge of the error
> construction up and down the stack through different layers of the
> application, and that brings with it assumptions about the implementation
> at the different layers. For example, should the application code above
> know that do_tokens() is making web requests to URLs? Why does it need that
> information?
>
> SRP-ly,
> Doug
>
>
>
>>
>>
>>
>> On Fri, Jul 12, 2013 at 12:25 PM, Nachi Ueno <nachi at ntti3.com> wrote:
>>
>>> Hi folks
>>>
>>> > Monty
>>> Thank you for your adding good topic for this.
>>>
>>> I agree, hiding true cause by exception get hard to debug
>>>
>>> For, example, Sqlalchemy gives me this error for general sql errors..
>>> "exceptions must be old-style classes or derived from BaseException, not
>>> str"
>>>
>>> (It should be hidden from REST API users though)
>>>
>>> Also, I agree with you about log policies.
>>> sometimes 404 get warn log..
>>>
>>> IMO, The log more than warn level should help Operators.
>>> also If the exception is truly, the exceptional case which needs help
>>> of operators.
>>> It should be logged as error level.
>>>
>>> Thanks
>>> Nachi
>>>
>>>
>>>
>>> 2013/7/12 Dolph Mathews <dolph.mathews at gmail.com>:
>>> >
>>> > On Fri, Jul 12, 2013 at 10:09 AM, Monty Taylor <mordred at inaugust.com>
>>> wrote:
>>> >>
>>> >>
>>> >>
>>> >> 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.
>>> >
>>> >
>>> > Lies! Write everything to be intuitive for new contributors or we
>>> won't have
>>> > any :(
>>> >
>>> >>
>>> >>
>>> >> > 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
>>> >> >
>>> >>
>>> >> _______________________________________________
>>> >> OpenStack-dev mailing list
>>> >> OpenStack-dev at lists.openstack.org
>>> >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>> >
>>> >
>>> >
>>> >
>>> > --
>>> >
>>> > -Dolph
>>> >
>>> > _______________________________________________
>>> > OpenStack-dev mailing list
>>> > OpenStack-dev at lists.openstack.org
>>> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>> >
>>>
>>> _______________________________________________
>>> OpenStack-dev mailing list
>>> OpenStack-dev at lists.openstack.org
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130712/6ae0e562/attachment.html>
More information about the OpenStack-dev
mailing list