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 1 commit
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
24 changes: 20 additions & 4 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, reraise)
Copy link
Contributor

Choose a reason for hiding this comment

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

tu dois changer do_handle_exception aussi non?

wrapped_error = HandledError(Handler.get_message(), error_id)
if reraise:
raise wrapped_error
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_data_base_name(func, args):
'''
This method captures the database_name of previous transaction in
with_pool decorator since it is passed as second 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[1]
Copy link
Member

@jmousset jmousset Oct 19, 2022

Choose a reason for hiding this comment

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

pas forcément args[1],

chercher le bonne indice dans arguments

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_data_base_name(func, args)
ErrorHandler.handle_exception(e, db_name)

return wrap