Skip to content
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

Consider using type.__name__ instead of str(type) in marshal_exception() #379

Closed
peterzahemszky opened this issue Jul 7, 2021 · 3 comments · Fixed by #391
Closed

Consider using type.__name__ instead of str(type) in marshal_exception() #379

peterzahemszky opened this issue Jul 7, 2021 · 3 comments · Fixed by #391
Labels
type: enhancement New feature or request
Milestone

Comments

@peterzahemszky
Copy link

Consider using type.__name__ instead of str(type) in marshal_exception() for better use of that string.

In my opinion, 'ZeroDivisionError' is more useful than "<class 'ZeroDivisionError'>".

https://github.com/enthought/traits-futures/blob/main/traits_futures/exception_handling.py#L22

def marshal_exception(e):
"""
Turn exception details into something that can be safely
transmitted across thread / process boundaries.
"""
exc_type = str(type(e))
exc_value = str(e)
formatted_traceback = str(traceback.format_exc())
return exc_type, exc_value, formatted_traceback

@mdickinson mdickinson added this to the Release 0.3.0 milestone Jul 7, 2021
@mdickinson
Copy link
Member

mdickinson commented Jul 7, 2021

Thanks! I think we can squeeze a fix for this into the coming release.

See also #318, which isn't scheduled for this release.

@peterzahemszky : What do you think about using something a bit more qualified than just __name__, for example something that includes the module information? E.g., for a ZeroDivisionError we'd still get just ZeroDivisionError, but for something like TraitError we'd get traits.trait_errors.TraitError?

@mdickinson
Copy link
Member

So we probably want something that uses both __module__ and __qualname__, except in the case where the module is builtins. This is close to what CPython does to construct the repr of a class: https://github.com/python/cpython/blob/2f180ce2cb6e6a7e3c517495e0f4873d6aaf5f2f/Objects/typeobject.c#L1076-L1092

@mdickinson
Copy link
Member

@peterzahemszky See #391 for a possible fix; feedback on whether this fits your use-case would be welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants