From badb95fbce6754c35215af4c4e6a0c844ceec59e Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Mon, 4 Nov 2019 08:25:10 +0000 Subject: [PATCH] [FIX] core: further pycompat cleanup odoo/odoo#28519 removed large parts of pycompat, but left reraise despite that not having much value. Remove that helper and replace it by just a `raise` in most cases: when raising from an except block, the old exception is automatically chained to the new one, no need to mess around. There is one exception: in http we have to re-raise an existing exception explicitly (aka `raise exc` rather than just `raise). This is less than ideal as Python *concatenates* stacks: the previously reified stack (from the except clause) is stacked on top of the new stack (from this raises), this leads to tracebacks "jumping around" at the break point of the handler and is somewhat confusing. So we want to use explicit chaining (`raise a from b`) with the "source" providing the caught exception's original traceback and the child providing the rest. However since callers rely on the exception making sense, we need the re-raised exception to be the original[0]. Copying the exception doesn't work (see [0]), chaining an exception to itself doesn't do anything useful, and while we could probably copy exceptions using the pickle method[1] that's still risky. So the most reliable option seems to be to create a new "cause" exception, move the old traceback over to it, then re-raise the original exception having cleared its traceback, chained to new the cause. [0] or a copy thereof but Odoo exceptions don't all work properly with copy.copy and we don't want that to fail so not really an option, we can't rely / bet on every new exception being cleanly copy-able [1] create an "empty" instance using __new__ (or an instance of something else onto which we re-set the __class__ in case the exctype actually overrides __new__) then copy the __dict__ closes odoo/odoo#39709 Signed-off-by: Xavier Morel (xmo) --- odoo/http.py | 12 ++++++++++-- odoo/tools/convert.py | 10 +--------- odoo/tools/pycompat.py | 10 ---------- odoo/tools/safe_eval.py | 6 ++---- 4 files changed, 13 insertions(+), 25 deletions(-) diff --git a/odoo/http.py b/odoo/http.py index 1484b78b791..222a2eeabb4 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -5,6 +5,7 @@ import ast import collections import contextlib +import copy import datetime import functools import hashlib @@ -305,8 +306,15 @@ class WebRequest(object): and not isinstance(exception, werkzeug.exceptions.HTTPException): odoo.tools.debugger.post_mortem( odoo.tools.config, sys.exc_info()) - # otherwise "no active exception to reraise" - raise pycompat.reraise(type(exception), exception, sys.exc_info()[2]) + + # WARNING: do not inline or it breaks: raise...from evaluates strictly + # LTR so would first remove traceback then copy lack of traceback + new_cause = Exception().with_traceback(exception.__traceback__) + # tries to provide good chained tracebacks, just re-raising exception + # generates a weird message as stacks just get concatenated, exceptions + # not guaranteed to copy.copy cleanly & we want `exception` as leaf (for + # callers to check & look at) + raise exception.with_traceback(None) from new_cause def _call_function(self, *args, **kwargs): request = self diff --git a/odoo/tools/convert.py b/odoo/tools/convert.py index 4fd29e57417..d41b33a360a 100644 --- a/odoo/tools/convert.py +++ b/odoo/tools/convert.py @@ -712,15 +712,7 @@ form: module.record_id""" % (xml_id,) try: self._tag_root(de) except Exception as e: - exc_info = sys.exc_info() - pycompat.reraise( - ParseError, - ParseError( - ustr(e), - etree.tostring(de, encoding='unicode').rstrip(), - de.getroottree().docinfo.URL, de.sourceline), - exc_info[2] - ) + raise ParseError(ustr(e), etree.tostring(de, encoding='unicode').rstrip(), de.getroottree().docinfo.URL, de.sourceline) DATA_ROOTS = ['odoo', 'data', 'openerp'] def convert_file(cr, module, filename, idref, mode='update', noupdate=False, kind=None, report=None, pathname=None): diff --git a/odoo/tools/pycompat.py b/odoo/tools/pycompat.py index d13e706b934..81e41829ae2 100644 --- a/odoo/tools/pycompat.py +++ b/odoo/tools/pycompat.py @@ -1,19 +1,9 @@ # -*- coding: utf-8 -*- -# to remove if we decide to add a dependency on six or future -# very strongly inspired by https://github.com/pallets/werkzeug/blob/master/werkzeug/_compat.py #pylint: disable=deprecated-module import csv import codecs -import collections import io - -def reraise(tp, value, tb=None): - if value.__traceback__ != tb: - raise value.with_traceback(tb) - raise value - - _reader = codecs.getreader('utf-8') _writer = codecs.getwriter('utf-8') diff --git a/odoo/tools/safe_eval.py b/odoo/tools/safe_eval.py index 813fd822e39..060514fbf91 100644 --- a/odoo/tools/safe_eval.py +++ b/odoo/tools/safe_eval.py @@ -207,8 +207,7 @@ def test_expr(expr, allowed_codes, mode="eval"): except (SyntaxError, TypeError, ValueError): raise except Exception as e: - exc_info = sys.exc_info() - pycompat.reraise(ValueError, ValueError('"%s" while compiling\n%r' % (ustr(e), expr)), exc_info[2]) + raise ValueError('"%s" while compiling\n%r' % (ustr(e), expr)) assert_valid_codeobj(allowed_codes, code_obj, expr) return code_obj @@ -368,8 +367,7 @@ def safe_eval(expr, globals_dict=None, locals_dict=None, mode="eval", nocopy=Fal except odoo.exceptions.MissingError: raise except Exception as e: - exc_info = sys.exc_info() - pycompat.reraise(ValueError, ValueError('%s: "%s" while evaluating\n%r' % (ustr(type(e)), ustr(e), expr)), exc_info[2]) + raise ValueError('%s: "%s" while evaluating\n%r' % (ustr(type(e)), ustr(e), expr)) def test_python_expr(expr, mode="eval"): try: test_expr(expr, _SAFE_OPCODES, mode=mode)