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
Show invalid config message on TraitErrors during init #921
Conversation
implemented via @catch_config decorator Now, the event that was triggered by invalid app config (see `--log-level 5`) is triggered by bad config at any point during initialization. This *will* catch TraitError bugs in IPython itself, but only during initialization. closes ipythongh-908
prevents recursive invocation of the crash handler
base64 encoding functions were called, but had no effect, because the notebook already has everything as b64-encoded bytestrings, which are valid ascii literals on Python 2. However, the encode/decode logic is actually triggered on Python 3, revealing its errors. This fixes the base64 functions that had no effect to have their intended effect, but does not use them. Rather, it is assumed that bytes objects are already b64-encoded (and thus ascii-safe), which assumption was already made in Python 2.
@@ -69,6 +71,26 @@ subcommand 'cmd', do: `{app} cmd -h`. | |||
# Application class | |||
#----------------------------------------------------------------------------- | |||
|
|||
@decorator | |||
def catch_config(method, app, *args, **kwargs): |
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.
This looks excellent, my only suggestion would be a name change: catch_config_error
instead. It seems to me it will make the use of the decorator throughout the code much more self-documenting.
Great work, thanks @minrk. All tests pass here, so once we agree on the final name choice, this one can go in. |
That makes sense. |
renamed to catch_config_error |
Perfect! All looks good, tests pass, thanks for the name change. Merging now. |
Show invalid config message on TraitErrors during initialization. implemented via `@catch_config` decorator Now, the event that was triggered by invalid app config (see `--log-level 5`) is triggered by bad config at any point during initialization. This *will* catch TraitError-raising bugs in IPython itself, but only during initialization. Also, deregister crash handler on use to avoid it being triggered recursively/repeatedly.
Show invalid config message on TraitErrors during initialization. implemented via `@catch_config` decorator Now, the event that was triggered by invalid app config (see `--log-level 5`) is triggered by bad config at any point during initialization. This *will* catch TraitError-raising bugs in IPython itself, but only during initialization. Also, deregister crash handler on use to avoid it being triggered recursively/repeatedly.
implemented via
@catch_config
decoratorNow, the event that was triggered by invalid app config (see
--log-level 5
) is triggered by bad config at any point during initialization.This will catch TraitError-raising bugs in IPython itself, but only during initialization.
A less aggressive approach would be to look for all possible TraitError-raising points, and protect only those.
Also includes a tiny commit which unregisters crash_handler in
__call__
, because it should never be called twice. The fact that it callsexit()
demonstrates that if it ever does get called twice, we are in a great big mess, and we should at least avoid printing pages and pages of extra crash messages.