Use the currently handled exception as UndefinedError.__context__ #2104
Conversation
Add ``Undefined._undefined_cause`` attribute, which is used as the default ``__cause__`` for exceptions raised by that ``Undefined``. When an ``Undefined`` is created, automatically set this attribute to the currently handled exception (or ``None``). This mirrors Python's own exception chaining. Also, restructure `Environment.getitem` and `Environment.getattr` uses exception chaning to preserve the exceptions they encounter.
docs/api.rst
Outdated
| The closest to regular Python behavior is the :class:`StrictUndefined` which | ||
| disallows all operations beside testing if it's an undefined object. | ||
|
|
||
| When :class:`Undefined` is created in an ``except`` or ``finally`` clause, |
There was a problem hiding this comment.
Did you mean to finish this line, or did you move it further down and forget to remove it here?
There was a problem hiding this comment.
The latter. Thanks for noticing!
| - When ``Undefined`` is created in an ``except`` block, the handled | ||
| exception is stored in a new attribute, ``_undefined_context``, | ||
| and used as the default ``__context__`` for errors raised by | ||
| ``_fail_with_undefined_error``. |
There was a problem hiding this comment.
Link to the issue you opened using :issue:`...`.
| """ | ||
| raise self._undefined_exception(self._undefined_message) | ||
| exception = self._undefined_exception(self._undefined_message) | ||
| if exception.__context__ is None: |
There was a problem hiding this comment.
Do we need to consider __cause__ as well?
There was a problem hiding this comment.
No.
Setting __cause__ will hide __context__ in the default (“linear”) traceback display, but both are available for more in-depth debugging. (See docs).
|
This all makes sense to me. Were you planning to propose another idea as well? |
|
You might need to modify |
encukou
left a comment
encukou
left a comment
There was a problem hiding this comment.
This all makes sense to me. Were you planning to propose another idea as well?
Thanks!
If the issue looks fix-worthy, there are a few questions best answered by the project maintainer.
Should this be an explicit keyword argument for Undefined? (Or should there be an additional argument for setting __cause__ -- that one is set using explicit syntax in Python?)
Some projects treat tested behaviour as promises. Should there be some disclaimer around raises_cause_chain that jijnja doesn't promise these exact chains -- we only want to ensure the tracebacks get shown to the user?
docs/api.rst
Outdated
| The closest to regular Python behavior is the :class:`StrictUndefined` which | ||
| disallows all operations beside testing if it's an undefined object. | ||
|
|
||
| When :class:`Undefined` is created in an ``except`` or ``finally`` clause, |
There was a problem hiding this comment.
The latter. Thanks for noticing!
| """ | ||
| raise self._undefined_exception(self._undefined_message) | ||
| exception = self._undefined_exception(self._undefined_message) | ||
| if exception.__context__ is None: |
There was a problem hiding this comment.
No.
Setting __cause__ will hide __context__ in the default (“linear”) traceback display, but both are available for more in-depth debugging. (See docs).
encukou
changed the title
2 participants
At this point, please treat this PR as a “feasibility study” for issue #2103. There are other ways to fix this.
Add
Undefined._undefined_causeattribute, which is used as the default__context__for exceptions raised by thatUndefined.When an
Undefinedis created, automatically set this attribute to the currently handled exception (orNone). This mirrors Python's own exception chaining.Also, restructure
Environment.getitemandEnvironment.getattrto use exception chaning to preserve the exceptions they encounter.Fixes: #2103
.. versionadded::