[openstack-dev] [Review] Use of exception for non-exceptional cases
Brant Knudson
blk at acm.org
Fri Jul 12 18:40:28 UTC 2013
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.
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130712/d3f8ea89/attachment.html>
More information about the OpenStack-dev
mailing list