Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Prevent prefilter from crashing IPython #415

Merged
merged 2 commits into from

3 participants

@minrk
Owner

Since user code can be executed in the prefilter step (e.g. via Aliases), catch and print those errors like normal code.

I could also be less aggressive and only catch AliasErrors.

Closes #216

@rkern

Why except SystemExit: instead of except Exception:?

@minrk
Owner

Because I managed to commit while I was checking that the test I added failed when the exception wasn't caught.

Fixed now.

@takluyver
Owner

Looks good to me. I'll test it later.

@takluyver
Owner

That's working here. Is it worth catching the specific case (wrong number of arguments for an alias) and displaying a short error message, rather than the full traceback?

@minrk
Owner

If we want IPython to die at every other error, I can only catch the AliasError. I don't actually think that's a good idea, but it would conform with current practices of making IPython crash and exit at the first sign of unrecognized trouble.

I think IPython should only crash when something is unrecoverable, not at every possible bug in IPython, but that's something of a separate issue.

@rkern

I think he's suggesting that you catch the AliasError specifically to make a more focused error message, then catch the rest of the exceptions with the general except Exception:

@takluyver
Owner

As Robert says - I'm not suggesting you remove the general except handler. Just that, in addition, I'd prefer to avoid a traceback of IPython internals for a simple mistake in user code. E.g. for magic functions, if the user's command is wrong in some way, we often just print a single line explanation of the problem, and return from the function.

@minrk minrk short error message on AliasError in run_cell
instead of full traceback

use prefilter_failed flag to prevent duplicate code
2bcbe90
@minrk
Owner

Okay, now the message is just the short 'ERROR: ' on AliasErrors, but all exceptions are caught.

@takluyver
Owner

OK, looks good to me.

@minrk minrk merged commit 285a7f7 into from
@ellisonbg ellisonbg referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@damianavila damianavila 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 May 4, 2011
  1. @minrk
Commits on May 5, 2011
  1. @minrk

    short error message on AliasError in run_cell

    minrk authored
    instead of full traceback
    
    use prefilter_failed flag to prevent duplicate code
This page is out of date. Refresh to see the latest.
View
61 IPython/core/interactiveshell.py
@@ -38,7 +38,7 @@
from IPython.core import prefilter
from IPython.core import shadowns
from IPython.core import ultratb
-from IPython.core.alias import AliasManager
+from IPython.core.alias import AliasManager, AliasError
from IPython.core.autocall import ExitAutocall
from IPython.core.builtin_trap import BuiltinTrap
from IPython.core.compilerop import CachingCompiler
@@ -2129,9 +2129,18 @@ def run_cell(self, raw_cell, store_history=True):
cell = self.input_splitter.source_reset()
with self.builtin_trap:
+ prefilter_failed = False
if len(cell.splitlines()) == 1:
- cell = self.prefilter_manager.prefilter_lines(cell)
-
+ try:
+ cell = self.prefilter_manager.prefilter_lines(cell)
+ except AliasError as e:
+ error(e)
+ prefilter_failed=True
+ except Exception:
+ # don't allow prefilter errors to crash IPython
+ self.showtraceback()
+ prefilter_failed = True
+
# Store raw and processed history
if store_history:
self.history_manager.store_inputs(self.execution_count,
@@ -2139,30 +2148,32 @@ def run_cell(self, raw_cell, store_history=True):
self.logger.log(cell, raw_cell)
- cell_name = self.compile.cache(cell, self.execution_count)
+ if not prefilter_failed:
+ # don't run if prefilter failed
+ cell_name = self.compile.cache(cell, self.execution_count)
- with self.display_trap:
- try:
- code_ast = ast.parse(cell, filename=cell_name)
- except (OverflowError, SyntaxError, ValueError, TypeError,
- MemoryError):
- self.showsyntaxerror()
- self.execution_count += 1
- return None
-
- self.run_ast_nodes(code_ast.body, cell_name,
- interactivity="last_expr")
-
- # Execute any registered post-execution functions.
- for func, status in self._post_execute.iteritems():
- if not status:
- continue
+ with self.display_trap:
try:
- func()
- except:
- self.showtraceback()
- # Deactivate failing function
- self._post_execute[func] = False
+ code_ast = ast.parse(cell, filename=cell_name)
+ except (OverflowError, SyntaxError, ValueError, TypeError,
+ MemoryError):
+ self.showsyntaxerror()
+ self.execution_count += 1
+ return None
+
+ self.run_ast_nodes(code_ast.body, cell_name,
+ interactivity="last_expr")
+
+ # Execute any registered post-execution functions.
+ for func, status in self._post_execute.iteritems():
+ if not status:
+ continue
+ try:
+ func()
+ except:
+ self.showtraceback()
+ # Deactivate failing function
+ self._post_execute[func] = False
if store_history:
# Write output to the database. Does nothing unless
View
16 IPython/core/tests/test_interactiveshell.py
@@ -20,7 +20,10 @@
#-----------------------------------------------------------------------------
# stdlib
import unittest
+from cStringIO import StringIO
+
from IPython.testing import decorators as dec
+from IPython.utils import io
#-----------------------------------------------------------------------------
# Tests
@@ -91,3 +94,16 @@ def test_magic_names_in_string(self):
ip = get_ipython()
ip.run_cell('a = """\n%exit\n"""')
self.assertEquals(ip.user_ns['a'], '\n%exit\n')
+
+ def test_alias_crash(self):
+ """Errors in prefilter can't crash IPython"""
+ ip = get_ipython()
+ ip.run_cell('%alias parts echo first %s second %s')
+ # capture stderr:
+ save_err = io.stderr
+ io.stderr = StringIO()
+ ip.run_cell('parts 1')
+ err = io.stderr.getvalue()
+ io.stderr = save_err
+ self.assertEquals(err.split(':')[0], 'ERROR')
+
Something went wrong with that request. Please try again.