Skip to content

Improve Python Stack retention #213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

zdevito
Copy link
Contributor

@zdevito zdevito commented Jun 10, 2025

Stack from ghstack (oldest at bottom):

This makes SerializedPyError match the exact formatting of python exceptions by using TracebackException to format it. It also ensures that it prepends any current members of the python stack to the Exceptions traceback before formatting. Normally exceptions are only reported after being completely unwound to the root of the python stack. But in most cases when we are capturing a Python exception it is because we are going to move it across the network. We might not have competely unwound the stack at the point we do the serialization, but this unwound stack is still potentially useful. So we also include these frames by adding them to the traceback object.

This also updates two places in PythonActor where exceptions would lose stack traces by going from PyErr -> anyhow::Error. By changing the result type to Result<_, SerialiablePyErr> we force it to capture the stack trace.

Differential Revision: D76316820

NOTE FOR REVIEWERS: This PR has internal Meta-specific changes or comments, please review them on Phabricator!

This makes SerializedPyError match the exact formatting of python exceptions by using TracebackException to format it. It also ensures that it prepends any current members of the python stack to the Exceptions traceback before formatting. Normally exceptions are only reported after being completely unwound to the root of the python stack. But in most cases when we are capturing a Python exception it is because we are going to move it across the network. We might not have competely unwound the stack at the point we do the serialization, but this unwound stack is still potentially useful. So we also include these frames by adding them to the traceback object.

This also updates two places in PythonActor where exceptions would lose stack traces by going from PyErr -> anyhow::Error. By changing the result type to Result<_, SerialiablePyErr> we force it to capture the stack trace.

Differential Revision: [D76316820](https://our.internmc.facebook.com/intern/diff/D76316820/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76316820/)!

[ghstack-poisoned]
zdevito added a commit that referenced this pull request Jun 10, 2025
This makes SerializedPyError match the exact formatting of python exceptions by using TracebackException to format it. It also ensures that it prepends any current members of the python stack to the Exceptions traceback before formatting. Normally exceptions are only reported after being completely unwound to the root of the python stack. But in most cases when we are capturing a Python exception it is because we are going to move it across the network. We might not have competely unwound the stack at the point we do the serialization, but this unwound stack is still potentially useful. So we also include these frames by adding them to the traceback object.

This also updates two places in PythonActor where exceptions would lose stack traces by going from PyErr -> anyhow::Error. By changing the result type to Result<_, SerialiablePyErr> we force it to capture the stack trace.

Differential Revision: [D76316820](https://our.internmc.facebook.com/intern/diff/D76316820/)

**NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D76316820/)!

ghstack-source-id: 289307428
Pull Request resolved: #213
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jun 10, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76316820

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 49be63a.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants