-
-
Notifications
You must be signed in to change notification settings - Fork 308
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
Better error handling: Re-raise with function reference on error #510
base: master
Are you sure you want to change the base?
Conversation
d8a39d7
to
06b1470
Compare
06b1470
to
715480b
Compare
@gen-xu I can not accept the PR: with this change, we generally convert any type of exception to the |
I fully agree with your concern. The reason why it is hard to debug on errors like that is mainly because cython doesn't add locals variables to the traceback object corresponding to the exception instance. see also: cython docs. So, even with good package like So I am wondering if adding locals manually to the from dependency_injector.containers import DeclarativeContainer
from dependency_injector.providers import Callable, Configuration, Singleton
class A:
def __init__(self, config) -> None:
pass
class B:
def __init__(self, config) -> None:
pass
class SomeContainer(DeclarativeContainer):
config = Configuration()
a = Singleton(A, config)
b = Singleton(B)
c = Callable(print, a, b) For the code example above, after this change, the new stacktrace would be something like this
and with the help of
|
9abd18a
to
f971109
Compare
f971109
to
9273ab3
Compare
@gen-xu my apologies for the super delayed feedback. Thanks a lot for working on this. I think we provide too much noise if we go that way: Traceback with variables (most recent call last):
File "C:\Users\Gen\Repos\python-dependency-injector\exp.py", line 21, in <module>
s.c()
__name__ = '__main__'
__doc__ = None
__package__ = None
__loader__ = <_frozen_importlib_external.SourceFileLoader object at 0x00000248EF47A100>
__spec__ = None
__annotations__ = {}
__builtins__ = <module 'builtins' (built-in)>
__file__ = 'C:\\Users\\Gen\\Repos\\python-dependency-injector\\exp.py'
__cached__ = None
DeclarativeContainer = <class 'dependency_injector.containers.DeclarativeContainer'>
Callable = <class 'dependency_injector.providers.Callable'>
Configuration = <class 'dependency_injector.providers.Configuration'>
Singleton = <class 'dependency_injector.providers.Singleton'>
print_exc = <function print_exc at 0x00000248F2254EE0>
A = <class '__main__.A'>
B = <class '__main__.B'>
SomeContainer = <class '__main__.SomeContainer'>
s = <dependency_injector.containers.DynamicContainer object at 0x00000248EF9C8FA0> Could you review #528 ? What do you think if we go that way? |
This PR addresses #509