Skip to content

Conversation

@oliverb123
Copy link
Contributor

This goes with the reversed-full-stack approach. It is naive to async considerations. Options below:

Original
image

Naive full stack:
image

Reversed full stack:
image

@oliverb123 oliverb123 requested review from a team, daibhin and hpouillot June 19, 2025 12:17
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Enhances exception handling in posthog/exception_utils.py by adding artificial traceback construction for exceptions that lack one, improving debugging capabilities with stack trace visualization.

  • Added functionality to construct artificial tracebacks by walking through stack frames from current execution point
  • Implemented reversed full-stack approach that shows more intuitive stack trace order for debugging
  • Current implementation is explicitly noted as being naive to async considerations, which may need future enhancement
  • Modified exception handling to attempt artificial traceback construction before falling back to system exception info

1 file reviewed, no comments
Edit PR Review Bot Settings | Greptile

if getattr(e, "__traceback__", None) is not None:
return

depth = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we start the depth at 1 to remove the capture_exception frame?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The one you see in the screenshot is actually the wrapper you added in posthog (there are 5 or 6 frames beneath this one, but they're correctly marked not-in-app, and I think it's fine to leave them)

Copy link
Contributor

Choose a reason for hiding this comment

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

Would actually love to get rid of that wrapper soon. Think it's less necessary now that we have all the context stuff

@oliverb123 oliverb123 merged commit e13c428 into master Jun 19, 2025
7 checks passed
@oliverb123 oliverb123 deleted the err/artificial-traces branch June 19, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants