[[[
> Preserve exception state in the Python binding of the abort_edit editor
> interface, so that when finish_report returns NULL the exception is still
> there, rather than a 'SystemError: NULL result without error in
> PyObject_Call' error.
>
> * subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
> (abort_edit): This needs to be different from all the other editor
> callback wrappers that use close_baton, so copy a simplified version
> of that in here. But, take care to preserve any exception that may
> have already been raised (likely the reason abort_edit is being called
> in the first place).
>
> * subversion/bindings/swig/python/tests/ra.py
> (test_do_diff2_abort_edit): Add test that raising an exception in an
> editor function causes abort_edit to be called, and that abort_edit
> can handle exceptions of its own without clobbering the original.
> ]]]
Patch looks mostly good. I have one question: What happens if the
abort_edit callback raises an exception which is not caught anywhere?
In this case, I think the abort_edit exception should win and clobber
the old exception.
Here's some examples to show my point:
-- Case 1: Exception 2 is thrown and caught while exception 1 is being handled
-- Result: Exception 2 is suppressed, and Exception 1 propagates
try:
try:
raise Exception("1")
finally:
try:
raise Exception("2")
except:
pass
except:
traceback.print_exc()
-- Case 2: Exception 2 is thrown but not caught while exception 1 is
being handled
-- Result: Exception 1 is suppressed, and Exception 2 propagates
import traceback
try:
try:
raise Exception("1")
finally:
raise Exception("2")
except:
traceback.print_exc()
It looks like your code handles case 1 correctly but handles case 2 wrong.
Thoughts? Either way I think a test case for this case would be a good idea.
Cheers,
David
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-04-03 23:40:04 CEST