Skip to content

[FIX] reraise __cause__ of the CodeEvaluationError if it exists #48

Closed
benwillig wants to merge 1 commit into
16.0from
16.0-fix_root_cause-bwi
Closed

[FIX] reraise __cause__ of the CodeEvaluationError if it exists #48
benwillig wants to merge 1 commit into
16.0from
16.0-fix_root_cause-bwi

Conversation

@benwillig
Copy link
Copy Markdown
Contributor

No description provided.

sbidoul
sbidoul previously approved these changes Jul 5, 2024
Comment thread statechart/models/interpreter.py Outdated
… want the final exception before CodeEvaluationError and not the root one
@benwillig benwillig force-pushed the 16.0-fix_root_cause-bwi branch from fc43145 to bb621c5 Compare July 8, 2024 07:25
@sbidoul
Copy link
Copy Markdown
Member

sbidoul commented Jul 8, 2024

@sbejaoui The previous recursion mechanism was inherited the python 2 era. Maybe worth checking if this works for your project too?

@sbidoul
Copy link
Copy Markdown
Member

sbidoul commented Aug 13, 2025

I'm slightly worried that some projects could depend on the current behaviour of recursively looking for the cause. So I propose to not merge ths in 16 and take it in 18 only.

Comment on lines +12 to +15
cause_exc = getattr(e, "__cause__", None)
if cause_exc:
return cause_exc
return e
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cause_exc = getattr(e, "__cause__", None)
if cause_exc:
return cause_exc
return e
return getattr(e, "__cause__", e)

This could be simplified like this, or even inlined at the two places where this is used, since the _root_cause name does not meany much anymore.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, no. I need more coffee :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather, (e.__cause__ or e), because __cause__ is part of BaseException so getattr should not be necessary anymore.

@sbidoul
Copy link
Copy Markdown
Member

sbidoul commented Aug 13, 2025

So closing. This is included in #49 for 18.0.

@sbidoul sbidoul closed this Aug 13, 2025
@sbidoul sbidoul deleted the 16.0-fix_root_cause-bwi branch August 13, 2025 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants