[openstack-dev] [Review] Use of exception for non-exceptional cases
nachi at ntti3.com
Fri Jul 12 17:25:10 UTC 2013
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
It should be logged as error level.
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
> 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):
>> 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
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
More information about the OpenStack-dev