-
-
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 #509 #528
base: develop
Are you sure you want to change the base?
Conversation
Add providers log on error.
@rmk135 Here - #510 - you write that after reraise exception all exceptions will be just general 'Exception'. and receive type of Exception was TypeError (I used this demo Also, I'm not sure about printing providers, Maybe we can log them somewhere. Addition idea about problem was to add ENV var like DEPENDENCY_INJECTION_DEBUG_MODE I think this issue is very important - for me, it is real hell to find error in dozens providers by unclear error message. |
try: | ||
result = self._provide(args, kwargs) | ||
except Exception as exc: | ||
print(self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite sure about using print()
here. It will send some data to stdout, which might not always be the desired behavior.
The idea, generally, looks interesting. Maybe we can use the logging
module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would agree with @rmk135 to use the logging
module with logger.exception
with some extra useful messages as it allows users to have more control on the output through logging.config
. Meanwhile, I believe the LogRecord may not provide precise info given the cython context (caller line number, stacktrace etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for reply!
Please look at updated pull request.
I hope logging.debug() is good idea because, by default, we have warning log level, and I hope no one use debug level in production) - so nothing will change for current users until logging level set to debug.
If this request will be accepted - I'll add article to documentation about this.
Use logging for providers error.
Add providers log on error.