On 01/04/2011 12:06 AM, Eduardo Habkost wrote:
> On Mon, Jan 03, 2011 at 07:22:17PM -0200, Eduardo Habkost wrote:
>> On Mon, Jan 03, 2011 at 11:07:53PM +0200, Michael Goldish wrote:
>>>
>>> If I understand your suggestion correctly, then:
>>>
>>> - The purpose of contexts is to add information to exceptions. If an
>>> exception is raised and you call clear_context() in a finally clause,
>>> you remove the context of the current function. If the function itself
>>> calls get_context() it'll see the current context, but as soon as an
>>> exception is raised the context disappears all the way up to the point
>>> where the exception is caught.
>>
>> I need to add the code to save the context stack when an exception is
>> raised (I focused on the code to save the context information and forgot
>> about the initial purpose :).
>>
>>>
>>> - A possible solution would be not to use finally and only call
>>> clear_context() if the function terminates successfully. Then if a
>>> function raises an exception, the context remains for some handler to
>>> catch and print. However, if we catch the exception sometime later and
>>> decide not to fail the test (for example: login failed and we don't
>>> care) then we have to pop some context items, or the context stack will
>>> be corrupted.
>>
>> When an exception is raised, you don't need to keep the context stack
>> untouched, you can pop it anyway. You just need to save a copy of the
>> current stack in case the exception is never handled.
>>
>>>
>>> - Additionally, if some function raises an exception and we perform some
>>> cleanup in a finally clause, and during that cleanup we call another
>>> function that calls context(), then our context stack will be modified
>>> by the other function, and the context handler (which calls
>>> get_context()) will not see the original context but rather a modified one.
>>
>> We can just save the context when an exception is raised. If upper in
>> the stack some other @context_aware function gets the same exception, we
>> don't need to save it again. If the exception is ignored, we can just
>> leave the saved context there (it would be overwritten if a new
>> exception is raised later, anyway).
>>
>> I will try to make it work and send a new version. I believe we won't
>> need stack trace tricks to make that work.
>
> What about the following code?
>
> I used 'cur_exception is last_exception' to check if an upper function
> is handling the same Exception that a lower function already saved. We
> could store/compare the traceback object from sys.exc_info() if we
> wanted to keep track of a traceback->context mapping instead of
> exception->context mapping, but I think the traceback information is
> more likely to be lost in a "except ...,e: raise e" construct somewhere,
> than the Exception object identity.
>
> If you want to be extra careful, you can keep an index of contexts for
> each exception ever seen, and clear it in _call_test_function() code,
> instead of keeping only the last exception. But I think we can keep it
> simple and just store info for the last exception.
I think that's risky:
try:
vm.create() # context aware, raises an exception
except:
try:
vm.destroy() # context aware, raises an exception
except:
pass
raise
The exception raised in VM.destroy() will override last_exception, and
then whoever calls context_for_exception() will get the wrong context.
But I think this will work and it looks pretty clean:
def _context_aware(fn, *args, **kwargs):
new_context("")
try:
try:
return fn(*args, **kwargs)
except Exception, e:
if not hasattr(e, "context"):
e.context = get_context()
raise sys.exc_info()[0], e, sys.exc_info()[2]
finally:
clear_context()
This way we don't need to keep track of the last exception, and we even
take care of the injection of .context here (instead of in test.py), so
we don't need context_for_exception() at all.
What do you think?
If this makes sense, I'd like to rewrite my patch using this decorator,
unless you feel like doing so yourself.
Thanks,
Michael
> ----------------------------------
> import threading, decorator
>
> CTX = threading.local()
>
> def _ctx(f, default):
> """Get a field from CTX and set a default if not set"""
> if not hasattr(CTX, f):
> setattr(CTX, f, default)
> return getattr(CTX, f)
>
> def _context_str(ctx):
> """Return string representation of a context"""
> return " --> ".join(ctx)
>
>
> def context(s):
> """Change current context"""
> CTX.context[-1] = s
>
> def new_context(s):
> """Push new context in the stack"""
> _ctx('context', []).append(s)
>
> def clear_context():
> """Remove current context from the stack"""
> CTX.context.pop()
>
> def get_context():
> """Get a description of the current context"""
> return _context_str(_ctx('context', []))
>
> def _last_exception():
> return _ctx('last_exception', None)
>
> def _context_for_exception(e):
> """Get information for the context where an exception was raised
>
> It requires getting the exception as argument for safety, to make sure
> information for the right exception is returned.
> """
> if e is _last_exception():
> return CTX.last_exception_context
>
> def context_for_exception(e):
> return _context_str(_context_for_exception(e))
>
> def _context_aware(fn, *args, **kwargs):
> new_context("[%s function begin]" % (fn.__name__))
> try:
> try:
> return fn(*args, **kwargs)
> except Exception,e:
> if e is not _last_exception():
> CTX.last_exception = e
> CTX.last_exception_context = CTX.context[:] # make a copy
> raise
> finally:
> clear_context()
>
> context_aware = decorator.decorator(_context_aware)
>
>
> ## test code:
> #############
>
> import unittest
> class TestContext(unittest.TestCase):
> def testSimpleCtx(self):
> @context_aware
> def a():
> context("hello")
> b()
> context("world")
> self.assertEquals(get_context(), 'world')
>
> @context_aware
> def b():
> context("foo")
> c()
>
> @context_aware
> def c():
> context("bar")
> self.assertEquals(get_context(), 'hello --> foo --> bar')
>
> a() # test it
> self.assertEquals(get_context(), '') # no context after everything returned
>
>
> def testException(self):
> @context_aware
> def try_something():
> context("trying something")
> raise Exception("something failed")
>
> @context_aware
> def do_something():
> context("do this")
> context("do that")
> context("try something")
> try_something()
> context("do something else")
>
> @context_aware
> def main():
> context("main code")
> context("do something")
> do_something()
>
> def test_exception():
> try:
> main()
> except Exception,e:
> ctx = context_for_exception(e)
> self.assertEquals(ctx, 'do something --> try something --> trying something')
> self.assertEquals(str(e), "something failed")
> else:
> self.fail('exception not raised')
>
> test_exception()
> self.assertEquals(get_context(), '') # no context after everything returned
>
> def testDroppedException(self):
> @context_aware
> def try_something():
> context("trying something")
> raise Exception("something failed")
>
> @context_aware
> def do_something():
> context("do this")
> context("do that")
> context("try something")
> try:
> try_something()
> except Exception,e:
> # function may do something here after handling the exception
> pass
> context("do something else")
>
> @context_aware
> def main():
> context("main code")
> context("do something")
> do_something()
> context("after do something")
> raise Exception("another failure")
>
> def test_exception():
> try:
> main()
> except Exception,e:
> ctx = context_for_exception(e)
> self.assertEquals(ctx, 'after do something')
> self.assertEquals(str(e), "another failure")
> else:
> self.fail('exception not raised')
>
> test_exception()
> self.assertEquals(get_context(), '') # no context after everything returned
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
