Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve error handling by capturing the database_name of previous transaction in with_pool decorator #205

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions trytond/error_handling.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# This file is part of Coog. The COPYRIGHT file at the top level of
# this repository contains the full copyright notices and license terms.
import sys

from inspect import getfullargspec
from werkzeug.exceptions import Forbidden, Unauthorized

from trytond.tools import resolve
Expand Down Expand Up @@ -51,21 +51,23 @@ def _get_handling_class():
return HandlingClass

@staticmethod
def handle_exception(error, reraise=True):
def handle_exception(error, db_name, reraise=True):
'''
Exception Handling method.

If reraise is True, a HandledError will be raised to communicate with
the end user, else the error will be returned so the caller can do
something with it.
If db_name is set, we will pass the real database name of previous
transaction.
Comment on lines +61 to +62
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Si c'est optionnel, ajouter =None dans la déclaration

'''
Handler = ErrorHandler._get_handling_class()
if Handler is None:
if reraise:
raise
return error

error_id = Handler.do_handle_exception(error)
error_id = Handler.do_handle_exception(error, db_name)
wrapped_error = HandledError(Handler.get_message(), error_id)
if reraise:
raise wrapped_error
Expand All @@ -76,7 +78,7 @@ def get_message(cls):
return cls._message

@classmethod
def do_handle_exception(e):
def do_handle_exception(e, db_name):
'''
The actual error handling code, that must be implemented in subclasses.

Expand All @@ -91,13 +93,27 @@ def error_wrap(func):
A decorator that can be placed on a function to plug raised errors on the
handler
'''

def wrap(*args, **kwargs):
def _get_database_name(func, args):
'''
This method captures the database_name of previous transaction in
with_pool decorator since it is passed as argument
to the wrapper
'''
arguments = getfullargspec(func)[0]
if 'database_name' in arguments:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il y a le cas des erreurs celery (error_wrap est aussi utilisé côté celery, et le nom n'est pas le même > modifier le proto côté celery ? Pas d'impact a priori...)

return args[arguments.index('database_name')]
return None

try:
return func(*args, **kwargs)
except (UserError, UserWarning, ConcurrencyException, Forbidden,
Unauthorized):
# Those errors are supposed to make their way to the end user
raise
except Exception as e:
ErrorHandler.handle_exception(e)
db_name = _get_database_name(func, args)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

à blinder, il ne faut surtout pas que ça crashe

ErrorHandler.handle_exception(e, db_name)

return wrap