[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>
This commit is contained in:
Xavier Morel
2019-11-05 08:14:10 +00:00
parent 4024f628f4
commit badb95fbce
4 changed files with 13 additions and 25 deletions
+10 -2
View File
@@ -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
+1 -9
View File
@@ -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):
-10
View File
@@ -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')
+2 -4
View File
@@ -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)