progrium / ginkgo

Python service microframework
http://ginkgo.readthedocs.org
MIT License
325 stars 48 forks source link

avoid printing exceptions (traceback.print_exc) #19

Closed progrium closed 12 years ago

progrium commented 12 years ago

In Service.start() and .serve_forever() we catch all exceptions and call stop(). However, we added traceback.print_exc() so that if stop() raises, we can see the original exception. It seems like there is a better way to do this. Perhaps an inner try/except? Even though tests pass, we still get nasty, confusing output:

progrium-twilio:gservice progrium$ python setup.py test
running test
............................Traceback (most recent call last):
  File "/Users/progrium/Projects/gservice/gservice/core.py", line 179, in start
    ready = self.do_start()
  File "/Users/progrium/Projects/gservice/gservice/tests/test_service.py", line 49, in do_start
    raise Exception("Error")
Exception: Error
.Traceback (most recent call last):
  File "/Users/progrium/Projects/gservice/gservice/core.py", line 181, in start
    self._ready_event.wait(self.ready_timeout)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/gevent-0.13.3-py2.7-macosx-10.6-x86_64.egg/gevent/event.py", line 74, in wait
    result = get_hub().switch()
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/gevent-0.13.3-py2.7-macosx-10.6-x86_64.egg/gevent/hub.py", line 163, in switch
    return greenlet.switch(self)
NotImplementedError: Second Error
...............
----------------------------------------------------------------------
Ran 44 tests in 7.403s

OK
ryanlarrabure commented 12 years ago

We should consider logging the exception instead of printing it. It will make debugging and testing easier.

progrium commented 12 years ago

Alan suggests we have two options: swallowing it or aborting (just die). Service.stop() should not raise. If it did, it's the Service implementor's fault.

progrium commented 12 years ago

Okay, we know that Service.stop() should not call Service.do_stop() if the service is not started, but should call stop() on its children.

We also decided to remove the try...except inside start() (and serve_forever()) that will try to call stop() on an exception.

progrium commented 12 years ago

I think the latest master satisfies this. I remember this was a nebulous issue that got into a bunch of other issues... most of which seem to have been resolved after 0.5.0