From e468d5f081af576a3321a0d4a5007ed5422095c0 Mon Sep 17 00:00:00 2001 From: "Brandon T. Willard" Date: Sun, 28 Oct 2018 21:42:18 -0500 Subject: [PATCH] Refactor REPL error handling and filter Hy internal trace output These changes make the Hy REPL more closely follow `code.InteractiveConsole`'s class interface and provide minimally intrusive traceback print-out filtering via a context manager that temporarily alters `sys.excepthook`. In other words, exception messages from the REPL will no longer show Hy internal code (e.g. importer, compiler and parsing functions). The boolean variable `hy.errors._hy_filter_internal_errors` dynamically enables/disables trace filtering, and the env variable `HY_FILTER_INTERNAL_ERRORS` can be used as the initial value. --- docs/language/cli.rst | 6 -- hy/__init__.py | 10 ++++ hy/cmdline.py | 125 +++++++++++++++++++++--------------------- hy/compiler.py | 2 +- hy/errors.py | 82 ++++++++++++++++++++++++++- tests/test_lex.py | 80 ++++++++++++++++++++++----- 6 files changed, 218 insertions(+), 87 deletions(-) diff --git a/docs/language/cli.rst b/docs/language/cli.rst index 2c8a1f7..e59640d 100644 --- a/docs/language/cli.rst +++ b/docs/language/cli.rst @@ -48,12 +48,6 @@ Command Line Options `--spy` only works on REPL mode. .. versionadded:: 0.9.11 -.. cmdoption:: --show-tracebacks - - Print extended tracebacks for Hy exceptions. - - .. versionadded:: 0.9.12 - .. cmdoption:: --repl-output-fn Format REPL output using specific function (e.g., hy.contrib.hy-repr.hy-repr) diff --git a/hy/__init__.py b/hy/__init__.py index f188b64..eb1d91c 100644 --- a/hy/__init__.py +++ b/hy/__init__.py @@ -5,6 +5,16 @@ except ImportError: __version__ = 'unknown' +def _initialize_env_var(env_var, default_val): + import os, distutils.util + try: + res = bool(distutils.util.strtobool( + os.environ.get(env_var, str(default_val)))) + except ValueError as e: + res = default_val + return res + + from hy.models import HyExpression, HyInteger, HyKeyword, HyComplex, HyString, HyBytes, HySymbol, HyFloat, HyDict, HyList, HySet # NOQA diff --git a/hy/cmdline.py b/hy/cmdline.py index 70d7b72..f7cca52 100644 --- a/hy/cmdline.py +++ b/hy/cmdline.py @@ -21,7 +21,7 @@ import hy from hy.lex import hy_parse, mangle from hy.lex.exceptions import PrematureEndOfInput from hy.compiler import HyASTCompiler, hy_compile, hy_eval -from hy.errors import HyTypeError, HyLanguageError, HySyntaxError +from hy.errors import HySyntaxError, filtered_hy_exceptions from hy.importer import runhy from hy.completer import completion, Completer from hy.macros import macro, require @@ -90,47 +90,58 @@ class HyREPL(code.InteractiveConsole, object): self._repl_results_symbols = [mangle("*{}".format(i + 1)) for i in range(3)] self.locals.update({sym: None for sym in self._repl_results_symbols}) - def runsource(self, source, filename='', symbol='single'): - global SIMPLE_TRACEBACKS + def ast_callback(self, main_ast, expr_ast): + if self.spy: + # Mush the two AST chunks into a single module for + # conversion into Python. + new_ast = ast.Module(main_ast.body + + [ast.Expr(expr_ast.body)]) + print(astor.to_source(new_ast)) - def error_handler(e, use_simple_traceback=False): - self.locals[mangle("*e")] = e - if use_simple_traceback: - print(e, file=sys.stderr) - else: - self.showtraceback() + def _error_wrap(self, error_fn, *args, **kwargs): + sys.last_type, sys.last_value, sys.last_traceback = sys.exc_info() + + # Sadly, this method in Python 2.7 ignores an overridden + # `sys.excepthook`. + if sys.excepthook is sys.__excepthook__: + error_fn(*args, **kwargs) + else: + sys.excepthook(sys.last_type, sys.last_value, sys.last_traceback) + + self.locals[mangle("*e")] = sys.last_value + + def showsyntaxerror(self, filename=None): + if filename is None: + filename = self.filename + + self._error_wrap(super(HyREPL, self).showsyntaxerror, + filename=filename) + + def showtraceback(self): + self._error_wrap(super(HyREPL, self).showtraceback) + + def runsource(self, source, filename='', symbol='single'): try: do = hy_parse(source, filename=filename) except PrematureEndOfInput: return True except HySyntaxError as e: - error_handler(e, use_simple_traceback=SIMPLE_TRACEBACKS) + self.showsyntaxerror(filename=filename) return False try: - def ast_callback(main_ast, expr_ast): - if self.spy: - # Mush the two AST chunks into a single module for - # conversion into Python. - new_ast = ast.Module(main_ast.body + - [ast.Expr(expr_ast.body)]) - print(astor.to_source(new_ast)) - - value = hy_eval(do, self.locals, self.module, - ast_callback=ast_callback, - compiler=self.hy_compiler, - filename=filename, + # Our compiler doesn't correspond to a real, fixed source file, so + # we need to [re]set these. + self.hy_compiler.filename = filename + self.hy_compiler.source = source + value = hy_eval(do, self.locals, self.module, self.ast_callback, + compiler=self.hy_compiler, filename=filename, source=source) - - except HyTypeError as e: - if e.source is None: - e.source = source - e.filename = filename - error_handler(e, use_simple_traceback=SIMPLE_TRACEBACKS) - return False + except SystemExit: + raise except Exception as e: - error_handler(e, use_simple_traceback=SIMPLE_TRACEBACKS) + self.showtraceback() return False if value is not None: @@ -142,10 +153,12 @@ class HyREPL(code.InteractiveConsole, object): # Print the value. try: output = self.output_fn(value) - except Exception as e: - error_handler(e) + except Exception: + self.showtraceback() return False + print(output) + return False @@ -201,25 +214,12 @@ def ideas_macro(ETname): """)]) -SIMPLE_TRACEBACKS = True - - -def pretty_error(func, *args, **kw): - try: - return func(*args, **kw) - except HyLanguageError as e: - if SIMPLE_TRACEBACKS: - print(e, file=sys.stderr) - sys.exit(1) - raise - - def run_command(source, filename=None): tree = hy_parse(source, filename=filename) __main__ = importlib.import_module('__main__') require("hy.cmdline", __main__, assignments="ALL") - pretty_error(hy_eval, tree, None, __main__, filename=filename, - source=source) + with filtered_hy_exceptions(): + hy_eval(tree, None, __main__, filename=filename, source=source) return 0 @@ -232,9 +232,7 @@ def run_repl(hr=None, **kwargs): hr = HyREPL(**kwargs) namespace = hr.locals - - with completion(Completer(namespace)): - + with filtered_hy_exceptions(), completion(Completer(namespace)): hr.interact("{appname} {version} using " "{py}({build}) {pyversion} on {os}".format( appname=hy.__appname__, @@ -263,9 +261,10 @@ def run_icommand(source, **kwargs): else: filename = '' - hr = HyREPL(**kwargs) - hr.runsource(source, filename=filename, symbol='single') - return run_repl(hr) + with filtered_hy_exceptions(): + hr = HyREPL(**kwargs) + hr.runsource(source, filename=filename, symbol='single') + return run_repl(hr) USAGE = "%(prog)s [-h | -i cmd | -c cmd | -m module | file | -] [arg] ..." @@ -301,9 +300,6 @@ def cmdline_handler(scriptname, argv): "(e.g., hy.contrib.hy-repr.hy-repr)") parser.add_argument("-v", "--version", action="version", version=VERSION) - parser.add_argument("--show-tracebacks", action="store_true", - help="show complete tracebacks for Hy exceptions") - # this will contain the script/program name and any arguments for it. parser.add_argument('args', nargs=argparse.REMAINDER, help=argparse.SUPPRESS) @@ -328,10 +324,6 @@ def cmdline_handler(scriptname, argv): options = parser.parse_args(argv[1:]) - if options.show_tracebacks: - global SIMPLE_TRACEBACKS - SIMPLE_TRACEBACKS = False - if options.E: # User did "hy -E ..." _remove_python_envs() @@ -372,7 +364,8 @@ def cmdline_handler(scriptname, argv): try: sys.argv = options.args - runhy.run_path(filename, run_name='__main__') + with filtered_hy_exceptions(): + runhy.run_path(filename, run_name='__main__') return 0 except FileNotFoundError as e: print("hy: Can't open file '{0}': [Errno {1}] {2}".format( @@ -448,9 +441,11 @@ def hy2py_main(): if options.FILE is None or options.FILE == '-': source = sys.stdin.read() - hst = pretty_error(hy_parse, source, filename='') + with filtered_hy_exceptions(): + hst = hy_parse(source, filename='') else: - with io.open(options.FILE, 'r', encoding='utf-8') as source_file: + with filtered_hy_exceptions(), \ + io.open(options.FILE, 'r', encoding='utf-8') as source_file: source = source_file.read() hst = hy_parse(source, filename=options.FILE) @@ -468,7 +463,9 @@ def hy2py_main(): print() print() - _ast = pretty_error(hy_compile, hst, '__main__') + with filtered_hy_exceptions(): + _ast = hy_compile(hst, '__main__') + if options.with_ast: if PY3 and platform.system() == "Windows": _print_for_windows(astor.dump_tree(_ast)) diff --git a/hy/compiler.py b/hy/compiler.py index 5377c33..a02e71a 100755 --- a/hy/compiler.py +++ b/hy/compiler.py @@ -1834,7 +1834,7 @@ def get_compiler_module(module=None, compiler=None, calling_frame=False): def hy_eval(hytree, locals=None, module=None, ast_callback=None, - compiler=None, filename='', source=None): + compiler=None, filename=None, source=None): """Evaluates a quoted expression and returns the value. If you're evaluating hand-crafted AST trees, make sure the line numbers diff --git a/hy/errors.py b/hy/errors.py index e8e9f98..4ed56e5 100644 --- a/hy/errors.py +++ b/hy/errors.py @@ -2,13 +2,20 @@ # Copyright 2019 the authors. # This file is part of Hy, which is free software licensed under the Expat # license. See the LICENSE. - +import os +import sys import traceback +import pkgutil from functools import reduce +from contextlib import contextmanager +from hy import _initialize_env_var from clint.textui import colored +_hy_filter_internal_errors = _initialize_env_var('HY_FILTER_INTERNAL_ERRORS', + True) + class HyError(Exception): def __init__(self, message, *args): @@ -193,7 +200,8 @@ class HySyntaxError(HyLanguageError, SyntaxError): def __str__(self): - output = traceback.format_exception_only(SyntaxError, self) + output = traceback.format_exception_only(SyntaxError, + SyntaxError(*self.args)) output[-1] = colored.yellow(output[-1]) if len(self.source) > 0: @@ -206,3 +214,73 @@ class HySyntaxError(HyLanguageError, SyntaxError): # Avoid "...expected str instance, ColoredString found" return reduce(lambda x, y: x + y, output) + + +def _get_module_info(module): + compiler_loader = pkgutil.get_loader(module) + is_pkg = compiler_loader.is_package(module) + filename = compiler_loader.get_filename() + if is_pkg: + # Use package directory + return os.path.dirname(filename) + else: + # Normalize filename endings, because tracebacks will use `pyc` when + # the loader says `py`. + return filename.replace('.pyc', '.py') + + +_tb_hidden_modules = {_get_module_info(m) + for m in ['hy.compiler', 'hy.lex', + 'hy.cmdline', 'hy.lex.parser', + 'hy.importer', 'hy._compat', + 'hy.macros', 'hy.models', + 'rply']} + + +def hy_exc_handler(exc_type, exc_value, exc_traceback): + """Produce exceptions print-outs with all frames originating from the + modules in `_tb_hidden_modules` filtered out. + + The frames are actually filtered by each module's filename and only when a + subclass of `HyLanguageError` is emitted. + + This does not remove the frames from the actual tracebacks, so debugging + will show everything. + """ + try: + # frame = (filename, line number, function name*, text) + new_tb = [frame for frame in traceback.extract_tb(exc_traceback) + if not (frame[0].replace('.pyc', '.py') in _tb_hidden_modules or + os.path.dirname(frame[0]) in _tb_hidden_modules)] + + lines = traceback.format_list(new_tb) + + if lines: + lines.insert(0, "Traceback (most recent call last):\n") + + lines.extend(traceback.format_exception_only(exc_type, exc_value)) + output = ''.join(lines) + + sys.stderr.write(output) + sys.stderr.flush() + except Exception: + sys.__excepthook__(exc_type, exc_value, exc_traceback) + + +@contextmanager +def filtered_hy_exceptions(): + """Temporarily apply a `sys.excepthook` that filters Hy internal frames + from tracebacks. + + Filtering can be controlled by the variable + `hy.errors._hy_filter_internal_errors` and environment variable + `HY_FILTER_INTERNAL_ERRORS`. + """ + global _hy_filter_internal_errors + if _hy_filter_internal_errors: + current_hook = sys.excepthook + sys.excepthook = hy_exc_handler + yield + sys.excepthook = current_hook + else: + yield diff --git a/tests/test_lex.py b/tests/test_lex.py index c759624..b0a03dc 100644 --- a/tests/test_lex.py +++ b/tests/test_lex.py @@ -1,6 +1,7 @@ # Copyright 2019 the authors. # This file is part of Hy, which is free software licensed under the Expat # license. See the LICENSE. +import sys import traceback import pytest @@ -10,11 +11,36 @@ from hy.models import (HyExpression, HyInteger, HyFloat, HyComplex, HySymbol, HyString, HyDict, HyList, HySet, HyKeyword) from hy.lex import tokenize from hy.lex.exceptions import LexException, PrematureEndOfInput +from hy.errors import hy_exc_handler def peoi(): return pytest.raises(PrematureEndOfInput) def lexe(): return pytest.raises(LexException) +def check_ex(execinfo, expected): + output = traceback.format_exception_only(execinfo.type, execinfo.value) + assert output[:-1] == expected[:-1] + # Python 2.7 doesn't give the full exception name, so we compensate. + assert output[-1].endswith(expected[-1]) + + +def check_trace_output(capsys, execinfo, expected): + sys.__excepthook__(execinfo.type, execinfo.value, execinfo.tb) + captured_wo_filtering = capsys.readouterr()[-1].strip('\n') + + hy_exc_handler(execinfo.type, execinfo.value, execinfo.tb) + captured_w_filtering = capsys.readouterr()[-1].strip('\n') + + output = captured_w_filtering.split('\n') + + # Make sure the filtered frames aren't the same as the unfiltered ones. + assert output[:-1] != captured_wo_filtering.split('\n')[:-1] + # Remove the origin frame lines. + assert output[3:-1] == expected[:-1] + # Python 2.7 doesn't give the full exception name, so we compensate. + assert output[-1].endswith(expected[-1]) + + def test_lex_exception(): """ Ensure tokenize throws a fit on a partial input """ with peoi(): tokenize("(foo") @@ -32,8 +58,13 @@ def test_unbalanced_exception(): def test_lex_single_quote_err(): "Ensure tokenizing \"' \" throws a LexException that can be stringified" # https://github.com/hylang/hy/issues/1252 - with lexe() as e: tokenize("' ") - assert "Could not identify the next token" in str(e.value) + with lexe() as execinfo: + tokenize("' ") + check_ex(execinfo, [ + ' File "", line -1\n', + " '\n", + ' ^\n', + 'LexException: Could not identify the next token.\n']) def test_lex_expression_symbols(): @@ -76,7 +107,11 @@ def test_lex_strings_exception(): """ Make sure tokenize throws when codec can't decode some bytes""" with lexe() as execinfo: tokenize('\"\\x8\"') - assert "Can't convert \"\\x8\" to a HyString" in str(execinfo.value) + check_ex(execinfo, [ + ' File "", line 1\n', + ' "\\x8"\n', + ' ^\n', + 'LexException: Can\'t convert "\\x8" to a HyString\n']) def test_lex_bracket_strings(): @@ -184,20 +219,13 @@ def test_lex_digit_separators(): def test_lex_bad_attrs(): with lexe() as execinfo: tokenize("1.foo") - - expected = [ + check_ex(execinfo, [ ' File "", line 1\n', ' 1.foo\n', ' ^\n', - ('LexException: Cannot access attribute on anything other' - ' than a name (in order to get attributes of expressions,' - ' use `(. )` or `(. )`)\n') - ] - output = traceback.format_exception_only(execinfo.type, execinfo.value) - - assert output[:-1:1] == expected[:-1:1] - # Python 2.7 doesn't give the full exception name, so we compensate. - assert output[-1].endswith(expected[-1]) + 'LexException: Cannot access attribute on anything other' + ' than a name (in order to get attributes of expressions,' + ' use `(. )` or `(. )`)\n']) with lexe(): tokenize("0.foo") with lexe(): tokenize("1.5.foo") @@ -437,3 +465,27 @@ def test_discard(): assert tokenize("a '#_b c") == [HySymbol("a"), HyExpression([HySymbol("quote"), HySymbol("c")])] assert tokenize("a '#_b #_c d") == [HySymbol("a"), HyExpression([HySymbol("quote"), HySymbol("d")])] assert tokenize("a '#_ #_b c d") == [HySymbol("a"), HyExpression([HySymbol("quote"), HySymbol("d")])] + + +def test_lex_exception_filtering(capsys): + """Confirm that the exception filtering works for lexer errors.""" + + # First, test for PrematureEndOfInput + with peoi() as execinfo: + tokenize(" \n (foo") + check_trace_output(capsys, execinfo, [ + ' File "", line 2', + ' (foo', + ' ^', + 'PrematureEndOfInput: Premature end of input']) + + # Now, for a generic LexException + with lexe() as execinfo: + tokenize(" \n\n 1.foo ") + check_trace_output(capsys, execinfo, [ + ' File "", line 3', + ' 1.foo', + ' ^', + 'LexException: Cannot access attribute on anything other' + ' than a name (in order to get attributes of expressions,' + ' use `(. )` or `(. )`)'])