diff options
| author | Xavier Morel <xmo@odoo.com> | 2019-11-04 08:25:10 +0000 |
|---|---|---|
| committer | Xavier Morel <xmo@odoo.com> | 2019-11-05 08:14:10 +0000 |
| commit | badb95fbce6754c35215af4c4e6a0c844ceec59e (patch) | |
| tree | 7b5d2630a33c4e645a8fb889a9a3f482aba228c2 | |
| parent | 4024f628f4d311782a8b256b6513a62d72771532 (diff) | |
[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) <xmo@odoo.com>
| -rw-r--r-- | odoo/http.py | 12 | ||||
| -rw-r--r-- | odoo/tools/convert.py | 10 | ||||
| -rw-r--r-- | odoo/tools/pycompat.py | 10 | ||||
| -rw-r--r-- | 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) |
