Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Show invalid config message on TraitErrors during init #921

Merged
merged 4 commits into from

2 participants

@minrk
Owner

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.

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 calls exit() 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.

minrk added some commits
@minrk minrk 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 bugs in IPython itself, but only during initialization.

closes gh-908
7ed219c
@minrk minrk unregister crash handler on call
prevents recursive invocation of the crash handler
074918c
@minrk minrk fix base64 code in nbformat.v2
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.
6607706
IPython/config/application.py
@@ -69,6 +71,26 @@ subcommand 'cmd', do: `{app} cmd -h`.
# Application class
#-----------------------------------------------------------------------------
+@decorator
+def catch_config(method, app, *args, **kwargs):
@fperez Owner
fperez added a note

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@fperez
Owner

Great work, thanks @minrk. All tests pass here, so once we agree on the final name choice, this one can go in.

@minrk
Owner

That makes sense.

@minrk
Owner

renamed to catch_config_error

@fperez
Owner

Perfect! All looks good, tests pass, thanks for the name change. Merging now.

@fperez fperez merged commit 6fb30ae into ipython:master
This was referenced
@fperez fperez referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 22, 2011
  1. @minrk

    Show invalid config message on TraitErrors during initialization

    minrk authored
    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 gh-908
  2. @minrk

    unregister crash handler on call

    minrk authored
    prevents recursive invocation of the crash handler
  3. @minrk

    fix base64 code in nbformat.v2

    minrk authored
    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.
Commits on Oct 28, 2011
  1. @minrk
This page is out of date. Refresh to see the latest.
View
37 IPython/config/application.py
@@ -26,6 +26,8 @@
from copy import deepcopy
from collections import defaultdict
+from IPython.external.decorator import decorator
+
from IPython.config.configurable import SingletonConfigurable
from IPython.config.loader import (
KVArgParseConfigLoader, PyFileConfigLoader, Config, ArgumentError, ConfigFileNotFound,
@@ -69,6 +71,26 @@
# Application class
#-----------------------------------------------------------------------------
+@decorator
+def catch_config_error(method, app, *args, **kwargs):
+ """Method decorator for catching invalid config (Trait/ArgumentErrors) during init.
+
+ On a TraitError (generally caused by bad config), this will print the trait's
+ message, and exit the app.
+
+ For use on init methods, to prevent invoking excepthook on invalid input.
+ """
+ try:
+ return method(app, *args, **kwargs)
+ except (TraitError, ArgumentError) as e:
+ app.print_description()
+ app.print_help()
+ app.print_examples()
+ app.log.fatal("Bad config encountered during initialization:")
+ app.log.fatal(str(e))
+ app.log.debug("Config at the time: %s", app.config)
+ app.exit(1)
+
class ApplicationError(Exception):
pass
@@ -173,6 +195,7 @@ def init_logging(self):
self._log_handler.setFormatter(self._log_formatter)
self.log.addHandler(self._log_handler)
+ @catch_config_error
def initialize(self, argv=None):
"""Do the basic steps to configure me.
@@ -317,6 +340,7 @@ def update_config(self, config):
# events.
self.config = newconfig
+ @catch_config_error
def initialize_subcommand(self, subc, argv=None):
"""Initialize a subcommand with argv."""
subapp,help = self.subcommands.get(subc)
@@ -376,6 +400,7 @@ def flatten_flags(self):
flags[key] = (newflag, help)
return flags, aliases
+ @catch_config_error
def parse_command_line(self, argv=None):
"""Parse the command line arguments."""
argv = sys.argv[1:] if argv is None else argv
@@ -402,18 +427,12 @@ def parse_command_line(self, argv=None):
loader = KVArgParseConfigLoader(argv=argv, aliases=aliases,
flags=flags)
- try:
- config = loader.load_config()
- self.update_config(config)
- except (TraitError, ArgumentError) as e:
- self.print_description()
- self.print_help()
- self.print_examples()
- self.log.fatal(str(e))
- self.exit(1)
+ config = loader.load_config()
+ self.update_config(config)
# store unparsed args in extra_args
self.extra_args = loader.extra_args
+ @catch_config_error
def load_config_file(self, filename, path=None):
"""Load a .py based config file by filename and path."""
loader = PyFileConfigLoader(filename, path=path)
View
4 IPython/core/application.py
@@ -34,7 +34,7 @@
import shutil
import sys
-from IPython.config.application import Application
+from IPython.config.application import Application, catch_config_error
from IPython.config.configurable import Configurable
from IPython.config.loader import Config, ConfigFileNotFound
from IPython.core import release, crashhandler
@@ -304,7 +304,7 @@ def stage_default_config_file(self):
with open(fname, 'w') as f:
f.write(s)
-
+ @catch_config_error
def initialize(self, argv=None):
# don't hook up crash handler before parsing command-line
self.parse_command_line(argv)
View
7 IPython/core/crashhandler.py
@@ -110,7 +110,12 @@ def __init__(self, app, contact_name=None, contact_email=None,
def __call__(self, etype, evalue, etb):
"""Handle an exception, call for compatible with sys.excepthook"""
-
+
+ # do not allow the crash handler to be called twice without reinstalling it
+ # this prevents unlikely errors in the crash handling from entering an
+ # infinite loop.
+ sys.excepthook = sys.__excepthook__
+
# Report tracebacks shouldn't use color in general (safer for users)
color_scheme = 'NoColor'
View
2  IPython/frontend/html/notebook/notebookapp.py
@@ -43,6 +43,7 @@
)
from .notebookmanager import NotebookManager
+from IPython.config.application import catch_config_error
from IPython.core.application import BaseIPythonApplication
from IPython.core.profiledir import ProfileDir
from IPython.zmq.session import Session, default_secure
@@ -260,6 +261,7 @@ def init_logging(self):
# and all of its ancenstors until propagate is set to False.
self.log.propagate = False
+ @catch_config_error
def initialize(self, argv=None):
super(NotebookApp, self).initialize(argv)
self.init_configurables()
View
3  IPython/frontend/qt/console/qtconsoleapp.py
@@ -29,7 +29,7 @@
from IPython.external.qt import QtGui
# Local imports
-from IPython.config.application import boolean_flag
+from IPython.config.application import boolean_flag, catch_config_error
from IPython.core.application import BaseIPythonApplication
from IPython.core.profiledir import ProfileDir
from IPython.lib.kernel import tunnel_to_kernel, find_connection_file
@@ -516,6 +516,7 @@ def init_colors(self):
else:
raise IOError("Stylesheet %r not found."%self.stylesheet)
+ @catch_config_error
def initialize(self, argv=None):
super(IPythonQtConsoleApp, self).initialize(argv)
self.init_connection_file()
View
5 IPython/frontend/terminal/ipapp.py
@@ -32,7 +32,7 @@
from IPython.config.loader import (
Config, PyFileConfigLoader, ConfigFileNotFound
)
-from IPython.config.application import boolean_flag
+from IPython.config.application import boolean_flag, catch_config_error
from IPython.core import release
from IPython.core import usage
from IPython.core.completer import Completer
@@ -285,7 +285,8 @@ def parse_command_line(self, argv=None):
argv[idx] = sub
return super(TerminalIPythonApp, self).parse_command_line(argv)
-
+
+ @catch_config_error
def initialize(self, argv=None):
"""Do actions after construct, but before starting the app."""
super(TerminalIPythonApp, self).initialize(argv)
View
8 IPython/nbformat/v2/nbjson.py
@@ -16,9 +16,8 @@
# Imports
#-----------------------------------------------------------------------------
-from base64 import encodestring
from .nbbase import from_dict
-from .rwbase import NotebookReader, NotebookWriter, base64_decode
+from .rwbase import NotebookReader, NotebookWriter, restore_bytes
import json
#-----------------------------------------------------------------------------
@@ -26,9 +25,10 @@
#-----------------------------------------------------------------------------
class BytesEncoder(json.JSONEncoder):
+ """A JSON encoder that accepts b64 (and other *ascii*) bytestrings."""
def default(self, obj):
if isinstance(obj, bytes):
- return unicode(encodestring(bytes))
+ return obj.decode('ascii')
return json.JSONEncoder.default(self, obj)
@@ -40,7 +40,7 @@ def reads(self, s, **kwargs):
return nb
def to_notebook(self, d, **kwargs):
- return base64_decode(from_dict(d))
+ return restore_bytes(from_dict(d))
class JSONWriter(NotebookWriter):
View
56 IPython/nbformat/v2/rwbase.py
@@ -19,31 +19,67 @@
from base64 import encodestring, decodestring
import pprint
+from IPython.utils.py3compat import str_to_bytes
+
#-----------------------------------------------------------------------------
# Code
#-----------------------------------------------------------------------------
+def restore_bytes(nb):
+ """Restore bytes of image data from unicode-only formats.
+
+ Base64 encoding is handled elsewhere. Bytes objects in the notebook are
+ always b64-encoded. We DO NOT encode/decode around file formats.
+ """
+ for ws in nb.worksheets:
+ for cell in ws.cells:
+ if cell.cell_type == 'code':
+ for output in cell.outputs:
+ if 'png' in output:
+ output.png = str_to_bytes(output.png, 'ascii')
+ if 'jpeg' in output:
+ output.jpeg = str_to_bytes(output.jpeg, 'ascii')
+ return nb
+
+
+# b64 encode/decode are never actually used, because all bytes objects in
+# the notebook are already b64-encoded, and we don't need/want to double-encode
+
def base64_decode(nb):
- """Base64 encode all bytes objects in the notebook."""
+ """Restore all bytes objects in the notebook from base64-encoded strings.
+
+ Note: This is never used
+ """
for ws in nb.worksheets:
for cell in ws.cells:
if cell.cell_type == 'code':
- if 'png' in cell:
- cell.png = bytes(decodestring(cell.png))
- if 'jpeg' in cell:
- cell.jpeg = bytes(decodestring(cell.jpeg))
+ for output in cell.outputs:
+ if 'png' in output:
+ if isinstance(output.png, unicode):
+ output.png = output.png.encode('ascii')
+ output.png = decodestring(output.png)
+ if 'jpeg' in output:
+ if isinstance(output.jpeg, unicode):
+ output.jpeg = output.jpeg.encode('ascii')
+ output.jpeg = decodestring(output.jpeg)
return nb
def base64_encode(nb):
- """Base64 decode all binary objects in the notebook."""
+ """Base64 encode all bytes objects in the notebook.
+
+ These will be b64-encoded unicode strings
+
+ Note: This is never used
+ """
for ws in nb.worksheets:
for cell in ws.cells:
if cell.cell_type == 'code':
- if 'png' in cell:
- cell.png = unicode(encodestring(cell.png))
- if 'jpeg' in cell:
- cell.jpeg = unicode(encodestring(cell.jpeg))
+ for output in cell.outputs:
+ if 'png' in output:
+ output.png = encodestring(output.png).decode('ascii')
+ if 'jpeg' in output:
+ output.jpeg = encodestring(output.jpeg).decode('ascii')
return nb
View
15 IPython/nbformat/v2/tests/nbexamples.py
@@ -1,10 +1,15 @@
+import os
+from base64 import encodestring
+
from ..nbbase import (
NotebookNode,
new_code_cell, new_text_cell, new_worksheet, new_notebook, new_output,
new_metadata, new_author
)
-
+# some random base64-encoded *bytes*
+png = encodestring(os.urandom(5))
+jpeg = encodestring(os.urandom(6))
ws = new_worksheet(name='worksheet1')
@@ -42,8 +47,8 @@
output_text=u'<array a>',
output_html=u'The HTML rep',
output_latex=u'$a$',
- output_png=b'data',
- output_jpeg=b'data',
+ output_png=png,
+ output_jpeg=jpeg,
output_svg=u'<svg>',
output_json=u'json data',
output_javascript=u'var i=0;',
@@ -53,8 +58,8 @@
output_text=u'<array a>',
output_html=u'The HTML rep',
output_latex=u'$a$',
- output_png=b'data',
- output_jpeg=b'data',
+ output_png=png,
+ output_jpeg=jpeg,
output_svg=u'<svg>',
output_json=u'json data',
output_javascript=u'var i=0;'
View
2  IPython/parallel/apps/baseapp.py
@@ -29,6 +29,7 @@
from subprocess import Popen, PIPE
+from IPython.config.application import catch_config_error
from IPython.core import release
from IPython.core.crashhandler import CrashHandler
from IPython.core.application import (
@@ -144,6 +145,7 @@ def _loop_default(self):
aliases = Dict(base_aliases)
flags = Dict(base_flags)
+ @catch_config_error
def initialize(self, argv=None):
"""initialize the app"""
super(BaseParallelApplication, self).initialize(argv)
View
3  IPython/parallel/apps/ipclusterapp.py
@@ -31,7 +31,7 @@
import zmq
from zmq.eventloop import ioloop
-from IPython.config.application import Application, boolean_flag
+from IPython.config.application import Application, boolean_flag, catch_config_error
from IPython.config.loader import Config
from IPython.core.application import BaseIPythonApplication
from IPython.core.profiledir import ProfileDir
@@ -269,6 +269,7 @@ def _daemonize_changed(self, name, old, new):
flags = Dict(engine_flags)
_stopping = False
+ @catch_config_error
def initialize(self, argv=None):
super(IPClusterEngines, self).initialize(argv)
self.init_signal()
View
8 IPython/parallel/apps/ipcontrollerapp.py
@@ -41,9 +41,10 @@
BaseParallelApplication,
base_aliases,
base_flags,
+ catch_config_error,
)
from IPython.utils.importstring import import_item
-from IPython.utils.traitlets import Instance, Unicode, Bool, List, Dict
+from IPython.utils.traitlets import Instance, Unicode, Bool, List, Dict, TraitError
from IPython.zmq.session import (
Session, session_aliases, session_flags, default_secure
@@ -263,7 +264,9 @@ def init_hub(self):
self.factory = HubFactory(config=c, log=self.log)
# self.start_logging()
self.factory.init_hub()
- except:
+ except TraitError:
+ raise
+ except Exception:
self.log.error("Couldn't construct the Controller", exc_info=True)
self.exit(1)
@@ -385,6 +388,7 @@ def forward_logging(self):
self.log.addHandler(handler)
self._log_handler = handler
+ @catch_config_error
def initialize(self, argv=None):
super(IPControllerApp, self).initialize(argv)
self.forward_logging()
View
2  IPython/parallel/apps/ipengineapp.py
@@ -34,6 +34,7 @@
BaseParallelApplication,
base_aliases,
base_flags,
+ catch_config_error,
)
from IPython.zmq.log import EnginePUBHandler
from IPython.zmq.session import (
@@ -318,6 +319,7 @@ def init_mpi(self):
else:
mpi = None
+ @catch_config_error
def initialize(self, argv=None):
super(IPEngineApp, self).initialize(argv)
self.init_mpi()
View
4 IPython/parallel/apps/iploggerapp.py
@@ -30,7 +30,8 @@
from IPython.parallel.apps.baseapp import (
BaseParallelApplication,
- base_aliases
+ base_aliases,
+ catch_config_error,
)
from IPython.parallel.apps.logwatcher import LogWatcher
@@ -68,6 +69,7 @@ class IPLoggerApp(BaseParallelApplication):
classes = [LogWatcher, ProfileDir]
aliases = Dict(aliases)
+ @catch_config_error
def initialize(self, argv=None):
super(IPLoggerApp, self).initialize(argv)
self.init_watcher()
View
4 IPython/zmq/ipkernel.py
@@ -28,7 +28,7 @@
# Local imports.
from IPython.config.configurable import Configurable
-from IPython.config.application import boolean_flag
+from IPython.config.application import boolean_flag, catch_config_error
from IPython.core.application import ProfileDir
from IPython.core.error import StdinNotImplementedError
from IPython.core.shellapp import (
@@ -730,6 +730,8 @@ class IPKernelApp(KernelApp, InteractiveShellApp):
selecting a particular matplotlib backend and loop integration.
"""
)
+
+ @catch_config_error
def initialize(self, argv=None):
super(IPKernelApp, self).initialize(argv)
self.init_shell()
View
3  IPython/zmq/kernelapp.py
@@ -26,7 +26,7 @@
# IPython imports.
from IPython.core.ultratb import FormattedTB
from IPython.core.application import (
- BaseIPythonApplication, base_flags, base_aliases
+ BaseIPythonApplication, base_flags, base_aliases, catch_config_error
)
from IPython.utils import io
from IPython.utils.localinterfaces import LOCALHOST
@@ -275,6 +275,7 @@ def init_kernel(self):
)
self.kernel.record_ports(self.ports)
+ @catch_config_error
def initialize(self, argv=None):
super(KernelApp, self).initialize(argv)
self.init_blackhole()
Something went wrong with that request. Please try again.