Move crash handling to the application level and simplify class structure.

Starting to try to take real advantage of the refactoring, to have generic
crash handling.  This also lets us initialize the app without needing all
the self.attempt() wrappers, since now there's a good system-wide crash
handler at the app level (not inside the shell instance).  I didn't yet
remove the attempt() method because we may have occasional uses for it (we
still do, but in one place only).

I also removed some extra class layers that weren't quite needed.  Creating
classes solely for the purpose of passing parameters makes the code (IMO)
harder to understand, I kept getting lost in parts of the class hierarchy.
I think these changes provide the same flexibility but with easier to follow
code (less things to remember, basically).  What I tried to do was to use
argument passing instead of inheritance for all cases I saw where the
inheritance wasn't really adding new functionality.  In some cases, this
actually allowed me to remove methods that were effectively duplicated in
the subclasses.

IPython/config/loader.py

@@ -298,14 +298,12 b' NoConfigDefault = NoConfigDefault()'
 
 class ArgParseConfigLoader(CommandLineConfigLoader):
     
-    # arguments = [(('-f','--file'),dict(type=str,dest='file'))]
-    arguments = ()
-
-    def __init__(self, argv=None, *args, **kw):
+    def __init__(self, argv=None, arguments=(), *args, **kw):
         """Create a config loader for use with argparse.
 
-        With the exception of argv, other args and kwargs arguments here are
-        passed onto the constructor of :class:`argparse.ArgumentParser`.
+        With the exception of ``argv`` and ``arguments``, other args and kwargs
+        arguments here are passed onto the constructor of
+        :class:`argparse.ArgumentParser`.
 
         Parameters
         ----------
@@ -313,11 +311,16 b' class ArgParseConfigLoader(CommandLineConfigLoader):'
         argv : optional, list
           If given, used to read command-line arguments from, otherwise
           sys.argv[1:] is used.
+
+        arguments : optional, tuple
+          Description of valid command-line arguments, to be called in sequence
+          with parser.add_argument() to configure the parser.
         """
         super(CommandLineConfigLoader, self).__init__()
         if argv == None:
             argv = sys.argv[1:]
         self.argv = argv
+        self.arguments = arguments
         self.args = args
         self.kw = kw
 

IPython/config/tests/test_loader.py

@@ -66,15 +66,13 b' class TestArgParseCL(TestCase):'
 
     def test_basic(self):
 
-        class MyLoader(ArgParseConfigLoader):
-            arguments = (
+        arguments = (
                 (('-f','--foo'), dict(dest='Global.foo', type=str)),
                 (('-b',), dict(dest='MyClass.bar', type=int)),
                 (('-n',), dict(dest='n', action='store_true')),
                 (('Global.bam',), dict(type=str))
             )
-
-        cl = MyLoader()
+        cl = ArgParseConfigLoader(arguments=arguments)
         config = cl.load_config('-f hi -b 10 -n wow'.split())
         self.assertEquals(config.Global.foo, 'hi')
         self.assertEquals(config.MyClass.bar, 10)

IPython/core/application.py

@@ -33,7 +33,7 b' import logging'
 import os
 import sys
 
-from IPython.core import release
+from IPython.core import release, crashhandler
 from IPython.utils.genutils import get_ipython_dir, get_ipython_package_dir
 from IPython.config.loader import (
     PyFileConfigLoader,
@@ -77,6 +77,29 b' class ApplicationError(Exception):'
     pass
 
 
+app_cl_args = (
+        (('--ipython-dir', ), dict(
+            dest='Global.ipython_dir',type=unicode,
+            help='Set to override default location of Global.ipython_dir.',
+            default=NoConfigDefault,
+            metavar='Global.ipython_dir') ),
+        (('-p', '--profile',), dict(
+            dest='Global.profile',type=unicode,
+            help='The string name of the ipython profile to be used.',
+            default=NoConfigDefault,
+            metavar='Global.profile') ),
+        (('--log-level',), dict(
+            dest="Global.log_level",type=int,
+            help='Set the log level (0,10,20,30,40,50).  Default is 30.',
+            default=NoConfigDefault,
+            metavar='Global.log_level')),
+        (('--config-file',), dict(
+            dest='Global.config_file',type=unicode,
+            help='Set the config file name to override default.',
+            default=NoConfigDefault,
+            metavar='Global.config_file')),
+    )
+
 class Application(object):
     """Load a config, construct components and set them running."""
 
@@ -94,11 +117,17 b' class Application(object):'
     ipython_dir = None
     #: A reference to the argv to be used (typically ends up being sys.argv[1:])
     argv = None
+    #: Default command line arguments.  Subclasses should create a new tuple
+    #: that *includes* these.
+    cl_arguments = app_cl_args
 
     # Private attributes
     _exiting = False
     _initialized = False
 
+    # Class choices for things that will be instantiated at runtime.
+    _CrashHandler = crashhandler.CrashHandler
+
     def __init__(self, argv=None):
         self.argv = sys.argv[1:] if argv is None else argv
         self.init_logger()
@@ -125,39 +154,49 b' class Application(object):'
         
         if self._initialized:
             return
-        
-        self.attempt(self.create_default_config)
+
+        # The first part is protected with an 'attempt' wrapper, that will log
+        # failures with the basic system traceback machinery.  Once our crash
+        # handler is in place, we can let any subsequent exception propagate,
+        # as our handler will log it with much better detail than the default.
+        self.attempt(self.create_crash_handler)
+        self.create_default_config()
         self.log_default_config()
         self.set_default_config_log_level()
-        self.attempt(self.pre_load_command_line_config)
-        self.attempt(self.load_command_line_config, action='abort')
+        self.pre_load_command_line_config()
+        self.load_command_line_config()
         self.set_command_line_config_log_level()
-        self.attempt(self.post_load_command_line_config)
+        self.post_load_command_line_config()
         self.log_command_line_config()
-        self.attempt(self.find_ipython_dir)
-        self.attempt(self.find_resources)
-        self.attempt(self.find_config_file_name)
-        self.attempt(self.find_config_file_paths)
-        self.attempt(self.pre_load_file_config)
-        self.attempt(self.load_file_config)
+        self.find_ipython_dir()
+        self.find_resources()
+        self.find_config_file_name()
+        self.find_config_file_paths()
+        self.pre_load_file_config()
+        self.load_file_config()
         self.set_file_config_log_level()
-        self.attempt(self.post_load_file_config)
+        self.post_load_file_config()
         self.log_file_config()
-        self.attempt(self.merge_configs)
+        self.merge_configs()
         self.log_master_config()
-        self.attempt(self.pre_construct)
-        self.attempt(self.construct)
-        self.attempt(self.post_construct)
+        self.pre_construct()
+        self.construct()
+        self.post_construct()
         self._initialized = True
 
     def start(self):
         self.initialize()
-        self.attempt(self.start_app)
+        self.start_app()
 
     #-------------------------------------------------------------------------
     # Various stages of Application creation
     #-------------------------------------------------------------------------
 
+    def create_crash_handler(self):
+        """Create a crash handler, typically setting sys.excepthook to it."""
+        self.crash_handler = self._CrashHandler(self, self.name)
+        sys.excepthook = self.crash_handler
+
     def create_default_config(self):
         """Create defaults that can't be set elsewhere.
 
@@ -185,10 +224,9 b' class Application(object):'
 
     def create_command_line_config(self):
         """Create and return a command line config loader."""
-        return BaseAppArgParseConfigLoader(self.argv,
-            description=self.description, 
-            version=release.version
-        )
+        return ArgParseConfigLoader(self.argv, self.cl_arguments,
+                                    description=self.description, 
+                                    version=release.version)
 
     def pre_load_command_line_config(self):
         """Do actions just before loading the command line config."""
@@ -384,7 +422,10 b' class Application(object):'
             raise
         except:
             if action == 'abort':
+                self.log.critical("Aborting application: %s" % self.name,
+                                  exc_info=True)
                 self.abort()
+                raise
             elif action == 'exit':
                 self.exit(0)
 

IPython/core/crashhandler.py

@@ -28,10 +28,8 b' from IPython.core import release'
 from IPython.core import ultratb
 from IPython.external.Itpl import itpl
 
-from IPython.utils.genutils import *
-
 #****************************************************************************
-class CrashHandler:
+class CrashHandler(object):
     """Customizable crash handlers for IPython-based systems.
 
     Instances of this class provide a __call__ method which can be used as a
@@ -41,15 +39,15 b' class CrashHandler:'
 
     """
 
-    def __init__(self,IP,app_name,contact_name,contact_email,
-                 bug_tracker,crash_report_fname,
+    def __init__(self,app, app_name, contact_name=None, contact_email=None, 
+                 bug_tracker=None, crash_report_fname='CrashReport.txt', 
                  show_crash_traceback=True):
         """New crash handler.
 
         Inputs:
 
-        - IP: a running IPython instance, which will be queried at crash time
-        for internal information.
+        - app: a running application instance, which will be queried at crash
+        time for internal information.
 
         - app_name: a string containing the name of your application.
 
@@ -77,13 +75,14 b' class CrashHandler:'
         """
 
         # apply args into instance
-        self.IP = IP  # IPython instance
+        self.app = app
         self.app_name = app_name
         self.contact_name = contact_name
         self.contact_email = contact_email
         self.bug_tracker = bug_tracker
         self.crash_report_fname = crash_report_fname
         self.show_crash_traceback = show_crash_traceback
+        self.section_sep = '\n\n'+'*'*75+'\n\n'
         
         # Hardcoded defaults, which can be overridden either by subclasses or
         # at runtime for the instance.
@@ -124,7 +123,7 b' $self.bug_tracker'
         #color_scheme = 'Linux'   # dbg
         
         try:
-            rptdir = self.IP.ipython_dir
+            rptdir = self.app.ipython_dir
         except:
             rptdir = os.getcwd()
         if not os.path.isdir(rptdir):
@@ -134,7 +133,7 b' $self.bug_tracker'
         # properly expanded out in the user message template
         self.crash_report_fname = report_name
         TBhandler = ultratb.VerboseTB(color_scheme=color_scheme,
-                                           long_header=1)
+                                      long_header=1)
         traceback = TBhandler.text(etype,evalue,etb,context=31)
 
         # print traceback to screen
@@ -159,70 +158,62 b' $self.bug_tracker'
 
     def make_report(self,traceback):
         """Return a string containing a crash report."""
-
-        sec_sep = '\n\n'+'*'*75+'\n\n'
-
+        import platform
+        
+        sec_sep = self.section_sep
+        
         report = []
         rpt_add = report.append
         
         rpt_add('*'*75+'\n\n'+'IPython post-mortem report\n\n')
-        rpt_add('IPython version: %s \n\n' % release.version)
-        rpt_add('BZR revision   : %s \n\n' % release.revision)
-        rpt_add('Platform info  : os.name -> %s, sys.platform -> %s' %
+        rpt_add('IPython version: %s \n' % release.version)
+        rpt_add('BZR revision   : %s \n' % release.revision)
+        rpt_add('Platform info  : os.name -> %s, sys.platform -> %s\n' %
                      (os.name,sys.platform) )
-        rpt_add(sec_sep+'Current user configuration structure:\n\n')
-        rpt_add(pformat(self.IP.dict()))
-        rpt_add(sec_sep+'Crash traceback:\n\n' + traceback)
+        rpt_add('               : %s\n' % platform.platform())
+        rpt_add('Python info    : %s\n' % sys.version)	     
+
         try:
-            rpt_add(sec_sep+"History of session input:")
-            for line in self.IP.user_ns['_ih']:
-                rpt_add(line)
-            rpt_add('\n*** Last line of input (may not be in above history):\n')
-            rpt_add(self.IP._last_input_line+'\n')
+            config = pformat(self.app.config)
+            rpt_add(sec_sep+'Current user configuration structure:\n\n')
+            rpt_add(config)
         except:
             pass
+        rpt_add(sec_sep+'Crash traceback:\n\n' + traceback)
 
         return ''.join(report)
 
+
 class IPythonCrashHandler(CrashHandler):
     """sys.excepthook for IPython itself, leaves a detailed report on disk."""
     
-    def __init__(self,IP):
+    def __init__(self, app, app_name='IPython'):
 
         # Set here which of the IPython authors should be listed as contact
         AUTHOR_CONTACT = 'Fernando'
         
         # Set argument defaults
-        app_name = 'IPython'
         bug_tracker = 'https://bugs.launchpad.net/ipython/+filebug'
         contact_name,contact_email = release.authors[AUTHOR_CONTACT][:2]
         crash_report_fname = 'IPython_crash_report.txt'
         # Call parent constructor
-        CrashHandler.__init__(self,IP,app_name,contact_name,contact_email,
+        CrashHandler.__init__(self,app,app_name,contact_name,contact_email,
                               bug_tracker,crash_report_fname)
 
     def make_report(self,traceback):
         """Return a string containing a crash report."""
 
-        sec_sep = '\n\n'+'*'*75+'\n\n'
-
-        report = []
+        sec_sep = self.section_sep
+        # Start with parent report
+        report = [super(IPythonCrashHandler, self).make_report(traceback)]
+        # Add interactive-specific info we may have
         rpt_add = report.append
-        
-        rpt_add('*'*75+'\n\n'+'IPython post-mortem report\n\n')
-        rpt_add('IPython version: %s \n\n' % release.version)
-        rpt_add('BZR revision   : %s \n\n' % release.revision)
-        rpt_add('Platform info  : os.name -> %s, sys.platform -> %s' %
-                     (os.name,sys.platform) )
-        rpt_add(sec_sep+'Current user configuration structure:\n\n')
-        # rpt_add(pformat(self.IP.dict()))
-        rpt_add(sec_sep+'Crash traceback:\n\n' + traceback)
         try:
             rpt_add(sec_sep+"History of session input:")
-            for line in self.IP.user_ns['_ih']:
+            for line in self.app.shell.user_ns['_ih']:
                 rpt_add(line)
             rpt_add('\n*** Last line of input (may not be in above history):\n')
-            rpt_add(self.IP._last_input_line+'\n')
+            rpt_add(self.app.shell._last_input_line+'\n')
         except:
             pass
 

IPython/core/ipapp.py

@@ -28,6 +28,7 b' import logging'
 import os
 import sys
 
+from IPython.core import crashhandler
 from IPython.core import release
 from IPython.core.application import Application, BaseAppArgParseConfigLoader
 from IPython.core.error import UsageError
@@ -280,19 +281,18 b' cl_args = ('
 )
 
 
-class IPythonAppCLConfigLoader(BaseAppArgParseConfigLoader):
-
-    arguments = cl_args
-
-
 default_config_file_name = u'ipython_config.py'
 
-
 class IPythonApp(Application):
     name = u'ipython'
     description = 'IPython: an enhanced interactive Python shell.'
     config_file_name = default_config_file_name
 
+    cl_arguments = Application.cl_arguments + cl_args
+
+    # Private and configuration attributes
+    _CrashHandler = crashhandler.IPythonCrashHandler
+    
     def __init__(self, argv=None, **shell_params):
         """Create a new IPythonApp.
 
@@ -309,6 +309,7 b' class IPythonApp(Application):'
         super(IPythonApp, self).__init__(argv)
         self.shell_params = shell_params
 
+
     def create_default_config(self):
         super(IPythonApp, self).create_default_config()
         # Eliminate multiple lookups
@@ -340,13 +341,6 b' class IPythonApp(Application):'
         Global.wthread = False
         Global.gthread = False
 
-    def create_command_line_config(self):
-        """Create and return a command line config loader."""
-        return IPythonAppCLConfigLoader(self.argv,
-            description=self.description, 
-            version=release.version
-        )
-
     def load_file_config(self):
         if hasattr(self.command_line_config.Global, 'quick'):
             if self.command_line_config.Global.quick:

IPython/core/iplib.py

@@ -1167,37 +1167,14 b' class InteractiveShell(Component, Magic):'
                                                      color_scheme='NoColor',
                                                      tb_offset = 1)
 
-        # IPython itself shouldn't crash. This will produce a detailed
-        # post-mortem if it does.  But we only install the crash handler for
-        # non-threaded shells, the threaded ones use a normal verbose reporter
-        # and lose the crash handler.  This is because exceptions in the main
-        # thread (such as in GUI code) propagate directly to sys.excepthook,
-        # and there's no point in printing crash dumps for every user exception.
-        if self.isthreaded:
-            ipCrashHandler = ultratb.FormattedTB()
-        else:
-            from IPython.core import crashhandler
-            ipCrashHandler = crashhandler.IPythonCrashHandler(self)
-        self.set_crash_handler(ipCrashHandler)
+        # The instance will store a pointer to the system-wide exception hook,
+        # so that runtime code (such as magics) can access it.  This is because
+        # during the read-eval loop, it may get temporarily overwritten.
+        self.sys_excepthook = sys.excepthook
 
         # and add any custom exception handlers the user may have specified
         self.set_custom_exc(*custom_exceptions)
 
-    def set_crash_handler(self, crashHandler):
-        """Set the IPython crash handler.
-
-        This must be a callable with a signature suitable for use as
-        sys.excepthook."""
-
-        # Install the given crash handler as the Python exception hook
-        sys.excepthook = crashHandler
-        
-        # The instance will store a pointer to this, so that runtime code
-        # (such as magics) can access it.  This is because during the
-        # read-eval loop, it gets temporarily overwritten (to deal with GUI
-        # frameworks).
-        self.sys_excepthook = sys.excepthook
-
     def set_custom_exc(self,exc_tuple,handler):
         """set_custom_exc(exc_tuple,handler)
 
@@ -1880,7 +1857,7 b' class InteractiveShell(Component, Magic):'
                 if (self.SyntaxTB.last_syntax_error and
                     self.autoedit_syntax):
                     self.edit_syntax_error()
-            
+
         # We are off again...
         __builtin__.__dict__['__IPYTHON__active'] -= 1
 

IPython/frontend/prefilterfrontend.py

@@ -77,12 +77,6 b' class PrefilterFrontEnd(LineFrontEndBase):'
         """
         if argv is None:
             argv = ['--no-banner']
-        # This is a hack to avoid the IPython exception hook to trigger
-        # on exceptions (https://bugs.launchpad.net/bugs/337105)
-        # XXX: This is horrible: module-leve monkey patching -> side
-        # effects.
-        from IPython.core import iplib
-        iplib.InteractiveShell.isthreaded = True
 
         LineFrontEndBase.__init__(self, *args, **kwargs)
         self.shell.output_trap = RedirectorOutputTrap(

IPython/frontend/tests/test_prefilterfrontend.py

@@ -88,9 +88,6 b' def isolate_ipython0(func):'
                 del user_ns[k]
             for k in new_globals:
                 del user_global_ns[k]
-        # Undo the hack at creation of PrefilterFrontEnd
-        from IPython.core import iplib
-        iplib.InteractiveShell.isthreaded = False
         return out
 
     my_func.__name__ = func.__name__