msg88965 - (view) |
Author: Patrick Westerhoff (poke) |
Date: 2009-06-05 19:04 |
I'm currently writing a library that executes predefined http requests
to a specified server. In case there is for example a HTTP Error, I want
to raise a user-defined exception that contains additional information
about when the error actually occured (so that the user of that library
gets information what happened without having to look at the actual
library code).
The in Python 3 introduced "feature" of chained exceptions (PEP 3134) is
something which is a bit annoying in that particular situation. While it
is probably good in a lot situation to know where an exception comes
from (especially when they are chained), it makes reading the actual
exception for the library user harder.
Let me show you an example:
def doXY ():
# ...
try:
page = urlopen( someRequest )
except urllib.error.URLError as e:
raise MyError( 'while doing XY', e )
# ...
MyError is an exception class, that uses the second parameter to get
additional information (for HTTPErrors the status code for example) and
compiles a detailed error message.
Before Python 3, this was a good way to prevent users from having to dig
into the code when they encounter an exception. Now however, you get
some error message like this:
-----
Traceback (most recent call last):
File "..", line ., in doXY
page = urlopen( someRequest )
File "..\lib\urllib\request.py", line 122,
in urlopen
return _opener.open(url, data, timeout)
File "..\lib\urllib\request.py", line 364,
in open
response = meth(req, response)
File "..\lib\urllib\request.py", line 476,
in http_response
'http', request, response, code, msg, hdrs)
File "..\lib\urllib\request.py", line 402,
in error
return self._call_chain(*args)
File "..\lib\urllib\request.py", line 336,
in _call_chain
result = func(*args)
File "..\lib\urllib\request.py", line 484,
in http_error_default
raise HTTPError(req.get_full_url(), code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 401: Unauthorized
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "..", line ., in <module>
doXY()
File "..", line ., in doXY
raise MyError( 'while doing XY', e )
MyError: 'HTTP Error 401 while doing XY (Unauthorized)'
-----
While the error message of MyError would be completely sufficient for
the user to know (and to fix by simply giving correct data) he is
flooded with a lot of information about the previously raised exception
instead with far too many unneccessary information.
So what I basically would like to have is some mechanism to prevent
Python from printing out all previous exception. As there is already the
'raise NewException from PreviousException' syntax, something like
'raise NewException from None' would be great, with explicitely stating
None to clear the buffer of previous exceptions.
Thanks you for any comments on this issue.
Patrick W.
|
msg89310 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2009-06-13 01:59 |
The current behaviour also doesn't match the spec in PEP 3134, which
states the __context__ attribute will only be set by the VM if it hasn't
already been set.
This is not currently the case, as setting __context__ does not keep the
VM from setting it (I also tried this setting __context__ to 1 - it was
still overridden):
>>> try:
... 1/0
... finally:
... exc = RuntimeError()
... exc.__context__ = None
... raise exc
...
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
ZeroDivisionError: int division or modulo by zero
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 6, in <module>
RuntimeError
The "raise ... from ..." syntax is not the correct syntax to use
however, since that relates the __cause__ atribute rather than __context__.
A better approach may be to provide a "sys.clear_exc_info()" method in
the sys module to allow exceptions to be raised from exception handlers
without any __context__ information.
A somewhat clumsy workaround that will work with current Python 3 is to
defer raising the exception until after the exception handler has
finished execution:
>>> exc = None
>>> try:
... 1/0
... except:
... exc = RuntimeError()
...
>>> if exc is not None: raise exc
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
RuntimeError
|
msg123312 - (view) |
Author: Steven D'Aprano (steven.daprano) * |
Date: 2010-12-04 01:45 |
It seems to me that an explicit raise inside an except block should *not* chain exceptions by default. The rationale for chaining exceptions is to detect bugs in the exception handler:
try:
something
except SomeError:
y = 1/x # oops, what happens when x == 0?
but an explicit raise isn't a bug. If you want it to chain for some reason, it's easy to do:
try:
something
except SomeError as exp:
raise MyException from exp
or otherwise set __content__, but there's no simple way to avoid chaining except via boilerplate code.
I frequently use the EAFP idiom to detect error conditions, and chained exceptions exposes details to the caller that are irrelevant implementation details. Here's a common example:
def process(iterable):
try:
x = next(iterable)
except StopIteration:
raise ValueError("can't process empty iterable")
continue_processing()
The fact that ValueError was raised in response to a StopIteration is an irrelevant implementation detail that shouldn't be, and wasn't in 2.x, exposed.
Another practical example is avoiding isinstance checks:
def func(x):
try:
x + 0
except (ValueError, TypeError):
raise MyException('x is not a number')
do_something_with(x)
I note that PEP 3134 explicitly listed the issue of *not* chaining exceptions as an open issue. Now that chained exceptions are being seen in the wild, it seems that the inability to easily suppress chaining *is* a real issue for some users:
http://mail.python.org/pipermail/python-list/2010-October/1258583.html
http://mail.python.org/pipermail/python-list/2010-December/1261738.html
|
msg123370 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-12-04 17:00 |
It is not possible to do this using a method, since "raise exc" will add a context anyway. So, if this is desired, it needs some new syntax and should be discussed on python-ideas.
(I don't think this is very important personally. Tracebacks are for developers to look at and I don't think the added information is confusing or detrimental)
|
msg123371 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2010-12-04 17:18 |
To get back to my original objection, it shouldn't be that difficult to differentiate between "__context__ never set" and "__context__ explicitly suppressed".
(e.g. using a property that sets an internal flag when set from Python code or via PyObject_SetAttr, or else a special ExceptionContextSuppressed singleton).
BaseException could then be given a "no_context" class method that did whatever dancing was necessary to suppress the context.
So Steven's examples would become:
def process(iterable):
try:
x = next(iterable)
except StopIteration:
raise ValueError.no_context("can't process empty iterable")
continue_processing()
def func(x):
try:
x + 0
except (ValueError, TypeError):
raise MyException.no_context('x is not a number')
do_something_with(x)
With appropriate changes to the exception display code, no_context could be as simple as the C equivalent of the following:
@classmethod
def no_context(cls, *args, **kwds):
exc = cls(*args, **kwds)
exc.__context__ = ExceptionContextSuppressed
return exc
Another alternative would be an additional internal flag queried by the exception chaining code itself:
@classmethod
def no_context(cls, *args, **kwds):
exc = cls(*args, **kwds)
exc.__context__ = None
exc._add_context = False
return exc
|
msg124735 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2010-12-27 21:37 |
I like MRAB's suggestion best:
MRAB wrote:
> Suggestion: an explicit 'raise' in the exception handler excludes the
> context, but if you want to include it then 'raise with'. For example:
>
> # Exclude the context
> try:
> command_dict[command]()
> except KeyError:
> raise CommandError("Unknown command")
>
> # Include the context
> try:
> command_dict[command]()
> except KeyError:
> raise with CommandError("Unknown command")
I think we can even strike off the verbiage "in the exception handler"... that way, raise always does the same thing -- raise KeyError will raise a KeyError, always, not sometimes a KeyError and sometimes a KeyError nested in a WhatEverError.
|
msg124738 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2010-12-27 22:53 |
I agree with the OP that we need a way to either suppress chaining or have it turned-off by default. A person writing an exception handler should have control over what the user sees.
|
msg124747 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-12-28 01:07 |
> A person writing an exception handler should have control over what
> the user sees.
There is already support for this in the traceback module (see the
"chain" parameter to various funcs).
|
msg124749 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2010-12-28 01:24 |
> There is already support for this in the traceback module (see the
> "chain" parameter to various funcs).
I'm not sure how that's going to help as I don't want my library code to be responsible for printing out exceptions, I just want them to print reasonably -- and it seems very unreasonable to have the exception I'm converting /from/ show up in the traceback.
|
msg124757 - (view) |
Author: Matthew Barnett (mrabarnett) * |
Date: 2010-12-28 03:31 |
Regarding syntax, I'm undecided between:
raise with new_exception
and:
raise new_exception with caught_exception
I think that the second form is clearer:
try:
...
exception SomeException as ex:
raise SomeOtherException() with ex
(I'd prefer 'with' to Steven's 'from') but the first form doesn't force you to provide a name:
try:
...
exception SomeException:
raise with SomeOtherException()
and the syntax also means that you can't chain another exception like this:
try:
...
exception SomeException as ex:
raise SomeOtherException() with YetAnotherException()
although perhaps Python should just rely on the programmer's good judgement. :-)
|
msg124790 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-12-28 12:10 |
> I'm not sure how that's going to help as I don't want my library code
> to be responsible for printing out exceptions, I just want them to
> print reasonably -- and it seems very unreasonable to have the
> exception I'm converting /from/ show up in the traceback.
I don't know if it's unreasonable. In some cases it can definitely help.
Besides, if you are writing library code (as opposed to application
code), you shouldn't care at all how tracebacks are displayed, should
you?
|
msg124820 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2010-12-28 21:42 |
> Besides, if you are writing library code (as opposed to application
> code), you shouldn't care at all how tracebacks are displayed, should
> you?
I care when it lies:
During handling of the above exception, another exception occurred:
This is a blatant falsehood -- another exception did not occur, a different exception was raised.
Now, when another exception does actually occur, I'm all for the nested traceback, but if I'm raising a different one, why is this useful:
--> d.address
Traceback (most recent call last):
File "nested_exceptions.py", line 7, in __getattr__
return self.data[self.names.index(name)]
ValueError: tuple.index(x): x not in tuple
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "nested_exceptions.py", line 9, in __getattr__
raise AttributeError("Attribute %s does not exist" % name)
AttributeError: Attribute address does not exist
?
Furthermore, I use my own library, and have no interest in seeing extra, completely unnecessary, and wrong (verbiage, anyway) traceback info -- not in my own libraries, nor in other's that I may be using.
Keep the nesting for when an exception actually occurs, not for when an exception is, under programmer control, being modified/changed.
|
msg124822 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-12-28 21:58 |
> During handling of the above exception, another exception occurred:
>
> This is a blatant falsehood -- another exception did not occur, a
> different exception was raised.
This doesn't make any difference in any other context, so why would it
here? If only the above sentence is problematic, you can perhaps suggest
another one.
> Now, when another exception does actually occur, I'm all for the
> nested traceback, but if I'm raising a different one, why is this
> useful:
To me that's the same as asking why the full call stack is useful. In
some cases it is useful, in other cases it is distracting. Python
displays comprehensive information by default. As I said, this can be
tweaked using the traceback module.
By the way, this is all described in detail in a PEP:
http://www.python.org/dev/peps/pep-3134/
|
msg124824 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2010-12-28 22:45 |
>> During handling of the above exception, another exception occurred:
>>
>> This is a blatant falsehood -- another exception did not occur, a
>> different exception was raised.
>
> This doesn't make any difference in any other context, so why would it
> here?
I'm not sure I understand what you are saying -- could you rephrase?
> By the way, this is all described in detail in a PEP:
> http://www.python.org/dev/peps/pep-3134/
Yes, I know -- and from the PEP:
Rationale
The Python-Dev discussions revealed interest in exception chaining
for two quite different purposes. To handle the unexpected raising
of a secondary exception, the exception must be retained implicitly.
To support intentional translation of an exception, there must be a
way to chain exceptions explicitly. This PEP addresses both.
Open Issue: Suppressing Context
As written, this PEP makes it impossible to suppress '__context__',
since setting exc.__context__ to None in an 'except' or 'finally'
clause will only result in it being set again when exc is raised.
The two motivations are excellent, and I have no issue with them; what I have issue with is that it is no longer possible to discard previous context. If I want to change the error message, I can use
except ValueError as exc:
raise AttributeError("blah") from exc
and then get
The above exception was the direct cause of the following exception
I would also like to see:
except ValueError as exc:
raise AttributeError("blah") with exc
to get
During handling of the above exception, another exception occurred
which would be the same as:
1/0
to get
During handling of the above exception, another exception occurred
and, finally, if all I have is
except ValueError as exc:
raise AttributeError("blah")
I just get the normal, previous context free, traceback.
|
msg124829 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-12-28 23:54 |
> and, finally, if all I have is
>
> except ValueError as exc:
> raise AttributeError("blah")
>
> I just get the normal, previous context free, traceback.
And what if the exception is raised from a subroutine?
|
msg124830 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2010-12-29 00:06 |
> And what if the exception is raised from a subroutine?
Well, if I have it surrounded by a try/except block, presumably I'm aware that an exception could be raised. ;)
And if I'm aware that an exception could be raised, it may also be possible that I want to change the exception -- leading to the three possibilities I described earlier.
Looking through my dbf module, most of those re-raises I'll be changing to use the raise ... from ... syntax, but there are still a couple places where it makes no sense to have the extra nested information available.
|
msg124832 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-12-29 00:15 |
> > And what if the exception is raised from a subroutine?
>
> Well, if I have it surrounded by a try/except block, presumably I'm
> aware that an exception could be raised. ;)
I'm talking about the exception raised from the except block.
|
msg124836 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2010-12-29 01:15 |
> I'm talking about the exception raised from the except block.
So was I -- why should this:
try:
x = y / z
except ZeroDivisionError as exc:
raise InvalidInput()
be different from this:
try:
x = divide_and_conquer(y, z)
except ZeroDivisionError as exc:
raise InvalidInput()
?
In both cases I want to discard the previous exception, and raise my own in its place (without the nesting, in this example).
|
msg124845 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2010-12-29 08:46 |
No, the context must always be included unless explicitly suppressed. The interpreter can't reliably tell the difference between a raise statement in the current exception handler and one buried somewhere inside a nested function call. The whole point is to give developers a hint as to how to trigger the broken error handling code, which only works if the default behaviour is to provide the additional information.
Being able to suppress the context *is* a valid feature request, but one that will now need to wait until Python 3.3. In the meantime, sys.excepthook + the traceback module + PYTHONSTARTUP allows individual users to modify the interactive prompt to exhibit whatever exception display behaviour they like, and applications can do the same thing in their __main__module (likely via a context manager retrieved from a utility module).
|
msg124846 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2010-12-29 08:53 |
For "can't tell" in my previous message, read "we aren't going to impose the requirement to be able to tell if an exception is being raised directly in the current exception handler as a feature of conforming Python implementations". We probably *could* tell the difference in CPython if we really wanted to.
Even without considering that aspect, it would also be seriously odd if putting the exception raising into a helper function suddenly caused context information to be included that was otherwise implicitly suppressed. Much better to correctly support explicit suppression of the context as discussed earlier in the issue.
|
msg124848 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-12-29 10:40 |
Le mercredi 29 décembre 2010 à 01:15 +0000, Ethan Furman a écrit :
> Ethan Furman <ethan@stoneleaf.us> added the comment:
>
> > I'm talking about the exception raised from the except block.
>
> So was I -- why should this:
>
> try:
> x = y / z
> except ZeroDivisionError as exc:
> raise InvalidInput()
>
> be different from this:
>
> try:
> x = divide_and_conquer(y, z)
I said the *except* block, not the *try* block ;)
|
msg124859 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2010-12-29 17:29 |
> I said the *except* block, not the *try* block ;)
Ah. So you did.
Okay, if I'm understanding correctly, the scenario you are talking about involves the code in the except block calling some other function, and that other function is raising an exception... seems unlikely that this would be a case of transforming one exception into another, therefore the raise should not suppress the context by default... okay, I'll concede the point.
Looks like the best option, then, is Nick's idea of the .no_context() method on exceptions.
|
msg124918 - (view) |
Author: Patrick Westerhoff (poke) |
Date: 2010-12-30 13:30 |
Nick Coghlan (ncoghlan) at 2010-12-29 08:46 (UTC):
> No, the context must always be included unless explicitly suppressed.
Then there should be some actually working way to suppress it, right?
I think the standard behaviour that automatically sets the `__context__` of an exception is fine, especially when thinking about exceptions that get raised inside `except` blocks and are not custom made. However there should be some way to suppress the context in some way.
Personally, I would prefer if `raise .. from ..` would set the exception's cause, but would *not* automatically print that cause. But then again, it would be a problem to get the cause afterwards when the program terminated because of that exception. So obviously, in such situations, both the cause and the context of a raised exception cannot be used (because both prints out the full trackback).
So we obviously need some new mechanism or syntax to ignore the previous exception. As it seems that the cause has a higher precedence than the context (which is fine as the cause is changeable), using `None` as the cause of an exception would be the best solution in my opinion:
try:
x / y
except ZeroDivisionError as e:
raise Exception( 'Invalid value for y' ) from None
While it might be difficult as the cause is `None` before and gets set to `None` afterwards, Python is totally able to detect that the value was explicitely set to `None`. Either the raise statement should set some internal flag, or the setter for `__cause__` should just check when it is first written to.
If that would be too complicated (although it would be possible and very logically to do it like that), maybe a different syntax would work:
try:
x / y
except ZeroDivisionError as e:
raise Exception( 'Invalid value for y' ) instead
Something like that.
|
msg124926 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2010-12-30 18:20 |
> using `None` as the cause of an exception would be the
> best solution in my opinion:
+1
|
msg124927 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-12-30 18:32 |
> > using `None` as the cause of an exception would be the
> > best solution in my opinion:
>
> +1
We are talking about context, not cause.
|
msg124943 - (view) |
Author: Patrick Westerhoff (poke) |
Date: 2010-12-31 00:07 |
Antoine Pitrou (pitrou) at 2010-12-30 18:32 (UTC)
> We are talking about context, not cause.
Yes, but - as said before - obviously the cause takes a higher precedence than context (otherwise it wouldn't show a context message when you explicitely set that). So when *explicitely* setting the cause to `None`, it should use the cause `None` and ignore the context, and as such display nothing.
|
msg124961 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-12-31 10:38 |
Le vendredi 31 décembre 2010 à 00:07 +0000, Patrick W. a écrit :
> Patrick W. <poke@BornToLaugh.de> added the comment:
>
> Antoine Pitrou (pitrou) at 2010-12-30 18:32 (UTC)
> > We are talking about context, not cause.
>
> Yes, but - as said before - obviously the cause takes a higher
> precedence than context (otherwise it wouldn't show a context message
> when you explicitely set that). So when *explicitely* setting the
> cause to `None`, it should use the cause `None` and ignore the
> context, and as such display nothing.
It looks quite unintuitive to me, though.
|
msg124968 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2010-12-31 15:42 |
raise AttributeError from None
makes sense to me, and in a nice, short example like that I prefer it.
In real code, however, I think
raise AttributeError.no_context("Field %s is not in table" % attr)
is going to be easier for the human to parse than
raise AttributeError("Field %s is not in table" % attr) from None
|
msg131162 - (view) |
Author: Matthew Barnett (mrabarnett) * |
Date: 2011-03-16 19:45 |
I've been looking through the list of current keywords and the best syntax I could come up with for suppressing the context is:
try:
x / y
except ZeroDivisionError as e:
raise as Exception( 'Invalid value for y' )
The rationale is that it's saying "forget about the original exception (if any), raise _as though_ this is the original exception".
|
msg146206 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-10-23 00:29 |
I like the class method idea, but not the no_context name. Would without_context be too long?
I don’t dislike raise MyError from None, but a method will be more easy to search and will help people reading code and discovering this unknown idiom. I think it’s also easier to implement than a syntax change.
|
msg151155 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-01-12 22:18 |
Okay, I like Matthew Barnett's idea of
except SomeError [as e]:
raise as SomeOtherError('...')
This would require a change to the grammer as well, yes? From:
raise_stmt: 'raise' [test ['from' test]]
to
raise_stmt: 'raise' [test ['from' test]] | 'raise' 'as' [test ['from' test]]
|
msg151156 - (view) |
Author: Patrick Westerhoff (poke) |
Date: 2012-01-12 22:39 |
Yeah, I would really like that solution, especially as it separates from the `from X` syntax that sets the exception’s cause.
Also I would prefer a syntax solution over a class method simply because the exception context is something that is implicitely set by Python with the `raise` statement. So it would make sense to overwrite that behaviour with a syntax construct as well.
|
msg152042 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-01-26 22:49 |
Okay, here is a patch implementing the 'raise ... from None' method.
All critiques appreciated (especially if they include links to learn from!).
|
msg152045 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-01-26 23:12 |
I went with
raise ... from None
instead of
raise as ...
because there seemed to me more support for 'from None' on python-dev, possible confusion with 'raise as', and 'from None' follows the existing systax of
raise SomeError() from SomeOtherError()
|
msg152058 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-01-27 01:55 |
1. Any syntax change requires a PEP (and, IMO, any such PEP for this issue should get rejected: I don't consider this an important enough feature to deserve dedicated syntax. Others disagree, which is one of the reasons why a PEP is needed. The other, more important, reason is to ensure the new syntax is spec'ed out clearly and incorporated into the language reference for the benefit of other implementations in the event that it *does* get approved)
2. A change that *doesn't* need a PEP is to just adjust the implicit exception chaining such that __context__ doesn't get set automatically if it has already been set explicitly (it turns out handling this situation was an open question in PEP 3134, not a specificied behaviour). That way, this task can be handled using a utility function:
def no_context(new_exc):
new_exc.__context__ = None
return new_exc
def doXY ():
# ...
try:
page = urlopen( someRequest )
except urllib.error.URLError as e:
raise no_context(MyError( 'while doing XY', e ))
# ...
|
msg152061 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-01-27 03:27 |
> 1. Any syntax change requires a PEP (and, IMO, any such PEP for this issue should get rejected: I don't consider this an important enough feature to deserve dedicated syntax. Others disagree, which is one of the reasons why a PEP is needed. The other, more important, reason is to ensure the new syntax is spec'ed out clearly and incorporated into the language reference for the benefit of other implementations in the event that it *does* get approved)
I'm not sure why this would be a syntax change. We can already do
try:
1/0
except ZeroDivisionError:
raise ValueError() from MathError()
to explicitly set the __cause__ and ignore the previous context. The
only difference would allowing 'None' to mean 'no cause, discard
previous context'.
> 2. A change that *doesn't* need a PEP is to just adjust the implicit exception chaining such that __context__ doesn't get set automatically if it has already been set explicitly (it turns out handling this situation was an open question in PEP 3134, not a specificied behaviour). That way, this task can be handled using a utility function:
>
> def no_context(new_exc):
> new_exc.__context__ = None
> return new_exc
>
> def doXY ():
> # ...
> try:
> page = urlopen( someRequest )
> except urllib.error.URLError as e:
> raise no_context(MyError( 'while doing XY', e ))
This seems like a lot more work than just allowing None to mean none.
|
msg152062 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-01-27 03:29 |
Nick Coghlan wrote:
> 1. Any syntax change requires a PEP
PEP is on python-dev.
|
msg152063 - (view) |
Author: Steven D'Aprano (steven.daprano) * |
Date: 2012-01-27 05:26 |
Nick Coghlan wrote:
> Nick Coghlan <ncoghlan@gmail.com> added the comment:
>
> 1. Any syntax change requires a PEP (and, IMO, any such PEP for this issue
> should get rejected: I don't consider this an important enough feature to
> deserve dedicated syntax. Others disagree, which is one of the reasons why
> a PEP is needed. The other, more important, reason is to ensure the new
> syntax is spec'ed out clearly and incorporated into the language reference
> for the benefit of other implementations in the event that it *does* get
> approved)
This already has a PEP. This is an *explicitly* unresolved issue from the
original PEP that introduced exception chaining in the first place.
http://www.python.org/dev/peps/pep-3134/
I quote:
Open Issue: Suppressing Context
As written, this PEP makes it impossible to suppress '__context__',
since setting exc.__context__ to None in an 'except' or 'finally'
clause will only result in it being set again when exc is raised.
With Ethan's patch, no new syntax is required. Since you can already say:
raise exception from another_exception
the syntax remains unchanged. There is an API change: currently
another_exception must inherit from BaseException, with the patch it may also
be None, but that doesn't change the syntax.
|
msg152065 - (view) |
Author: Steven D'Aprano (steven.daprano) * |
Date: 2012-01-27 05:31 |
[...]
My comment has been overtaken by additional comments by Nick on the Python-Dev
list.
|
msg152213 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-01-29 07:37 |
It looks like agreement is forming around the
raise ... from None
method. It has been mentioned more than once that having the context saved on the exception would be a Good Thing, and for further debugging (or logging or what-have-you) I must agree.
The patch attached now sets __cause__ to True, leaving __context__ unclobbered. The exception printing routine checks to see if __cause__ is True, and if so simply skips the display of either cause or __context__, but __context__ can still be queried by later code.
One concern raised was that since it is possible to write (even before this patch)
raise KeyError from NameError
outside of a try block that some would get into the habit of writing
raise KeyError from None
as a way of preemptively suppressing implicit context chaining; I am happy to report that this is not an issue, since when that exception is caught and a new exception raised, it is the new exception that controls the display.
In other words:
>>> try:
... raise ValueError from None
... except:
... raise NameError
...
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
ValueError
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 4, in <module>
NameError
|
msg152216 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-01-29 08:04 |
Ah, nice idea of bringing the boolean constants into the mix so we don't need to invent a new sentinel value.
However, to preserve the current behaviour that "raise X from Y" is essentially just syntactic sugar for: "_var = X; _var.__cause__ = Y; raise Y", I suggest setting the default value for "__cause__" to "False" (indicating "not set"). Then:
"__cause__ is False" means "no cause set, display context"
"__cause__ is None" means "cause explicitly set to None, suppress context"
Any other value means "display cause"
The default value for cause is controlled by get_cause and set_cause in exceptions.c [1]. The basic idea would be to replace the current usage of Py_None and Py_RETURN_NONE in that code with Py_False and Py_RETURN_FALSE, and then change the criteria for valid causes to "arg != Py_None && !PyExceptionInstance_Check(arg)".
In addition to the files already touched by the patch, Lib/traceback.py [2] and its tests will also require updating.
[1] http://hg.python.org/cpython/file/default/Objects/exceptions.c#l293
[2] http://hg.python.org/cpython/file/default/Lib/traceback.py#l117
|
msg152227 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-01-29 14:11 |
Current semantics (before patch):
cause is not None --> cause is set, display it instead of context
cause is None --> no cause, try to display context
context is not None --> no context
context is None --> context set, display it (unless cause already displayed)
---
Proposed semantics (after patch)
cause is True --> context set, but no display
cause is not None --> cause set, display it instead of context
cause is None --> no cause, try to display context
context is None --> no context
context is not None --> context set, display it (unless cause already displayed)
---
I prefer to go with True for cause, instead of False, as a way of saying "Yes, there was an exception before this one, but I'm not going to display it" as opposed to None meaning "No there is no cause, and I'm not going to show you the context either".
Using True instead of False, and leaving the None's as they are now, preserves the behavior of None meaning none, as in "there isn't one".
|
msg152228 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2012-01-29 14:24 |
Hmm, so "from None" sets cause to "True", while all other "from X" sets cause to "X". That does not sound like a good idea to me.
|
msg152242 - (view) |
Author: Patrick Westerhoff (poke) |
Date: 2012-01-29 15:50 |
I have to agree with Georg on that. I think it would make more sense to introduce some internal flag/variable that keeps track of if the cause was explicitely set. So if cause was set (i.e. `from X` syntax is used), then always display it in favor of the context – except that a None-cause causes nothing to display.
Regardless of that I’m actually not sure if just changing the way the cause is displayed is a correct way to handle the context. If I explicitely raise an exception in an except-handler, I usually don’t expect that new exception to get the previous exception attached to. In the original example, I want to completely replace the “context” by a new exception without implicitely keeping over the original exception.
So even if using `from None` will prevent the context from being displayed (as the explicitely set cause will override it), the `__context__` will probably still be set by the `raise` statement, and I think that shouldn’t happen. Hence the `raise X instead` or `raise as X` idea that simply does not set the context but “destroys” it.
|
msg152252 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-01-29 18:10 |
Patrick: The value in this enhancement is in not displaying the chained exception. I do not see any value in throwing it away completely. If you don't care about __context__ you can safely ignore it. On the other hand, if it is completely removed, and you do care about it... well, too bad.
Georg, Nick: On further thought, I agree that having 'from None' set cause from None to True is counter-intuitive, and there is consistency in having __context__ as None and __cause__ as False.
Thanks, Nick, for the pointers on the necessary changes.
|
msg152281 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-01-29 23:14 |
Not sure I have traceback._iter_chain() patched correctly, but all the tests pass.
Here's the latest code.
|
msg152285 - (view) |
Author: Steven D'Aprano (steven.daprano) * |
Date: 2012-01-29 23:30 |
Patrick Westerhoff wrote:
> Patrick Westerhoff <PatrickWesterhoff@gmail.com> added the comment:
>
> I have to agree with Georg on that. I think it would make more sense to
> introduce some internal flag/variable that keeps track of if the cause was
> explicitely set. So if cause was set (i.e. `from X` syntax is used), then
> always display it in favor of the context – except that a None-cause causes
> nothing to display.
>
> Regardless of that I’m actually not sure if just changing the way the cause
> is displayed is a correct way to handle the context. If I explicitely raise
> an exception in an except-handler, I usually don’t expect that new
> exception to get the previous exception attached to.
I'm inclined to agree. I'm not convinced that "raise MyError from None" should
leave any trace of the old exception at all.
Suppose you have a couple of functions like this:
def spam(x): # LBYL
if hasattr(x, "__len__"):
y = len(x)+1
else:
raise MyError("x has no length")
do_stuff_with(y)
def ham(x): # EAFP
try:
y = len(x)+1
except TypeError:
raise MyError("x has no length") from None
do_stuff_with(y)
It is entirely an irrelevant implementation detail whether you happen to write
spam or ham. The exception that the caller gets should, in my opinion, be the
same. I can't see any benefit to exposing the TypeError, even if the caller
has to overtly go looking for it in order to see it.
But having said that, if others have a specific scenario in mind where they
would need to distinguish between spam and ham, I'm happy for the context to
be set. But I am curious to learn what the scenario is. Is it just a matter of
"just in case"?
|
msg152294 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-01-30 00:22 |
This was discussed a little more in the python-dev thread for PEP 409, but both Guido and I have been burned in the past by badly written libraries that replaced detailed exceptions that explained *exactly* what was going wrong with bland, generic "it broke!" exceptions that told us nothing. What should have been a 5 minute fix turns into a long bug hunt because useful information was being thrown away.
With __context__ always being set, all you need to do to cope with inappropriate use of "raise X from None" by libraries is write your own exception handler that always reports the entire exception chain, regardless of the __cause__ setting. If "raise X from None" actually *clobbers* the context, though, you instead have to go in and try to get hold of the detailed exception information *before* it gets clobbered (which is a lot harder to do).
|
msg152295 - (view) |
Author: Patrick Westerhoff (poke) |
Date: 2012-01-30 00:38 |
Oh, where did that PEP come from? ^^ Also thanks for hinting at python-dev, didn’t realize that there was a discussion going on about this!
|
msg152423 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-02-01 06:48 |
Attached patch now includes doc changes.
|
msg152424 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-02-01 07:16 |
Okay, *all* the documentation updates this time. Thanks, Nick!
|
msg152481 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-02-02 22:03 |
Latest version of PEP is on python-dev; here is the latest patch.
Summary:
For __cause__ we are replacing the old special value of None with Ellipsis: Ellipsis means check __context__ for an exception to display; None means ignore __context__ and stop following exception chain; an exception means display this exception and stop following the exception chain.
|
msg152497 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-02-03 07:26 |
PEP 409 has been accepted. :)
Attached is the latest patch implementing it, with the (hopefully ;) final changes.
Because Ellipsis is now the default value for __cause__, 'raise ... from Ellipsis' is treated the same as 'raise ...'
|
msg152543 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-02-03 17:29 |
> Because Ellipsis is now the default value for __cause__,
> 'raise ... from Ellipsis' is treated the same as 'raise ...'
Not exactly true -- if ... is a new exception then they are the same; if ... is a caught exception that is being reraised, __cause__ will be (re)set to Ellipsis.
|
msg152555 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-02-03 20:23 |
I just noticed that no one is assigned to this issue -- anybody on the nosy list able to review?
|
msg152809 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2012-02-07 13:52 |
I started work on integrating this into 3.3 this evening, but ran into too many issues to finish it.
Problems found and fixed:
- traceback.py displayed the wrong exception (test added and impl fixed)
Additional changes:
- eliminated duplicate code paths for __cause__ validation in ceval.c and exceptions.c (latter now exposes a private C API for ceval to use)
- documented that Ellipsis may be used as a sentinel when None is not appropriate
- broke up the long test case in test_exceptions a bit
- started a placeholder section in What's New
Remaining problems:
- default sys.excepthook implementation currently does the wrong thing
- needs a test for the pythonrun display logic (test_cmd_line_script would be the appropriate place)
- testCauseSyntax test should probably be in test_raise, not test_exceptions
Off the top of my head, I'm not even sure where the default sys.excepthook impl even *lives*. Making that behave itself is the major blocker at this point, though (followed by a test for the pythonrun change - given the problem in traceback.py, it may be that it's this code that's buggy and it's affecting the interactive interpreter as well).
(I deliberately haven't added a NEWS entry yet - that's best left until last, since it's a major cause of merge conflicts otherwise)
|
msg153461 - (view) |
Author: Ethan Furman (ethan.furman) * |
Date: 2012-02-16 06:57 |
> Remaining problems:
> - default sys.excepthook implementation currently does the wrong thing
Unable to duplicate on my system (Win XP SP3)
> - needs a test for the pythonrun display logic (test_cmd_line_script
> would be the appropriate place)
Not sure I understand this point, but I added a test in test_raise that starts a subprocess interpreter and checks its output.
> - testCauseSyntax test should probably be in test_raise, not
> test_exceptions
Moved.
|
msg154313 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2012-02-26 08:03 |
New changeset b299c4f31ff2 by Nick Coghlan in branch 'default':
Close issue #6210: Implement PEP 409
http://hg.python.org/cpython/rev/b299c4f31ff2
|
msg154314 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2012-02-26 08:13 |
Thanks, I can’t wait to use that in my code!
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:49 | admin | set | github: 50459 |
2012-02-26 08:13:40 | eric.araujo | set | messages:
+ msg154314 |
2012-02-26 08:04:31 | ncoghlan | set | status: open -> closed resolution: fixed stage: resolved |
2012-02-26 08:03:00 | python-dev | set | nosy:
+ python-dev messages:
+ msg154313
|
2012-02-16 06:58:00 | ethan.furman | set | files:
+ pep409.diff
messages:
+ msg153461 |
2012-02-07 16:48:31 | draghuram | set | nosy:
- draghuram
|
2012-02-07 13:52:45 | ncoghlan | set | files:
+ pep409_interactive_broken.diff
messages:
+ msg152809 |
2012-02-07 11:57:23 | ncoghlan | set | assignee: ncoghlan |
2012-02-03 20:23:21 | ethan.furman | set | messages:
+ msg152555 |
2012-02-03 17:29:36 | ethan.furman | set | messages:
+ msg152543 |
2012-02-03 07:26:27 | ethan.furman | set | files:
+ raise_from_none_v7.diff
messages:
+ msg152497 |
2012-02-02 22:03:15 | ethan.furman | set | files:
+ raise_from_none_v6.diff
messages:
+ msg152481 |
2012-02-01 07:16:08 | ethan.furman | set | files:
+ raise_from_none_v5.diff
messages:
+ msg152424 |
2012-02-01 06:48:39 | ethan.furman | set | files:
+ raise_from_none_v4.diff
messages:
+ msg152423 |
2012-01-30 00:38:08 | poke | set | messages:
+ msg152295 |
2012-01-30 00:22:24 | ncoghlan | set | messages:
+ msg152294 |
2012-01-29 23:30:50 | steven.daprano | set | messages:
+ msg152285 |
2012-01-29 23:14:02 | ethan.furman | set | files:
+ raise_from_none_v3.diff
messages:
+ msg152281 |
2012-01-29 18:10:22 | ethan.furman | set | messages:
+ msg152252 |
2012-01-29 15:50:55 | poke | set | messages:
+ msg152242 |
2012-01-29 14:24:06 | georg.brandl | set | nosy:
+ georg.brandl messages:
+ msg152228
|
2012-01-29 14:11:18 | ethan.furman | set | messages:
+ msg152227 |
2012-01-29 08:04:32 | ncoghlan | set | messages:
+ msg152216 |
2012-01-29 07:37:26 | ethan.furman | set | files:
+ raise_from_none_v2.diff
messages:
+ msg152213 |
2012-01-27 05:31:49 | steven.daprano | set | messages:
+ msg152065 |
2012-01-27 05:26:36 | steven.daprano | set | messages:
+ msg152063 |
2012-01-27 03:29:16 | ethan.furman | set | messages:
+ msg152062 |
2012-01-27 03:27:34 | ethan.furman | set | messages:
+ msg152061 |
2012-01-27 01:55:02 | ncoghlan | set | messages:
+ msg152058 |
2012-01-26 23:12:14 | ethan.furman | set | messages:
+ msg152045 |
2012-01-26 22:49:53 | ethan.furman | set | files:
+ raise_from_none.diff keywords:
+ patch messages:
+ msg152042
|
2012-01-23 08:58:54 | catalin.iacob | set | nosy:
+ catalin.iacob
|
2012-01-12 22:39:54 | poke | set | messages:
+ msg151156 |
2012-01-12 22:18:21 | ethan.furman | set | messages:
+ msg151155 |
2011-10-23 00:29:38 | eric.araujo | set | messages:
+ msg146206 |
2011-10-13 18:51:28 | ezio.melotti | set | nosy:
+ ezio.melotti
|
2011-03-16 19:45:44 | mrabarnett | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, aronacher, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg131162 |
2011-03-16 18:02:47 | aronacher | set | nosy:
+ aronacher
|
2010-12-31 15:42:48 | ethan.furman | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124968 |
2010-12-31 10:38:51 | pitrou | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124961 |
2010-12-31 00:07:43 | poke | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124943 |
2010-12-30 18:32:43 | pitrou | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124927 |
2010-12-30 18:20:50 | rhettinger | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124926 |
2010-12-30 13:30:01 | poke | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124918 |
2010-12-29 17:30:24 | ethan.furman | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman versions:
+ Python 3.3, - Python 3.2 |
2010-12-29 17:29:56 | ethan.furman | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124859 versions:
+ Python 3.2, - Python 3.3 |
2010-12-29 10:40:06 | pitrou | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124848 |
2010-12-29 08:53:24 | ncoghlan | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124846 |
2010-12-29 08:46:55 | ncoghlan | set | keywords:
+ after moratorium versions:
+ Python 3.3, - Python 3.2 messages:
+ msg124845 nosy:
rhettinger, ncoghlan, pitrou, draghuram, eric.araujo, mrabarnett, steven.daprano, poke, ethan.furman |
2010-12-29 01:46:18 | eric.araujo | set | nosy:
+ eric.araujo
|
2010-12-29 01:15:24 | ethan.furman | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124836 |
2010-12-29 00:15:21 | pitrou | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124832 |
2010-12-29 00:06:16 | ethan.furman | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124830 |
2010-12-28 23:54:03 | pitrou | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124829 |
2010-12-28 22:45:54 | ethan.furman | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124824 |
2010-12-28 21:58:14 | pitrou | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124822 |
2010-12-28 21:42:52 | ethan.furman | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124820 |
2010-12-28 12:10:32 | pitrou | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124790 |
2010-12-28 03:31:24 | mrabarnett | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124757 |
2010-12-28 01:24:18 | ethan.furman | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124749 |
2010-12-28 01:07:45 | pitrou | set | nosy:
rhettinger, ncoghlan, pitrou, draghuram, mrabarnett, steven.daprano, poke, ethan.furman messages:
+ msg124747 |
2010-12-27 23:08:23 | mrabarnett | set | nosy:
+ mrabarnett
|
2010-12-27 22:53:49 | rhettinger | set | nosy:
+ rhettinger messages:
+ msg124738
|
2010-12-27 21:37:34 | ethan.furman | set | nosy:
ncoghlan, pitrou, draghuram, steven.daprano, poke, ethan.furman messages:
+ msg124735 |
2010-12-27 19:29:46 | ethan.furman | set | nosy:
+ ethan.furman
|
2010-12-04 17:18:12 | ncoghlan | set | messages:
+ msg123371 |
2010-12-04 17:00:08 | pitrou | set | assignee: pitrou -> (no value) messages:
+ msg123370 |
2010-12-04 08:32:34 | georg.brandl | set | assignee: pitrou
nosy:
+ pitrou |
2010-12-04 01:45:41 | steven.daprano | set | nosy:
+ steven.daprano messages:
+ msg123312
|
2010-08-03 20:01:27 | draghuram | set | nosy:
+ draghuram
|
2009-06-13 01:59:21 | ncoghlan | set | nosy:
+ ncoghlan messages:
+ msg89310
|
2009-06-05 19:14:18 | benjamin.peterson | set | versions:
+ Python 3.2, - Python 3.0 |
2009-06-05 19:04:24 | poke | create | |