From 0fd773a486fbc1aeef7f00ec5eab3eaf0e4c6a9d Mon Sep 17 00:00:00 2001 From: Goffin Simon Date: Thu, 18 Dec 2014 16:38:09 +0100 Subject: [IMP] Cleanup and refactoring of exception handling Unify and refactor exception handling in framework and addons. The generic `except_osv` is now deprecated, and replaced by more specialized exception subtypes: - `UserError` (renamed from Warning, as it conflicts with the built-in `Warning`) raised when a non-technical error occurs during a business operation. It could be a missing information in the data provided by the user, or a misconfiguration. - `AccessError`: raised when any operation is denied because the user conducting it does not have the required access rights. - `AccessDenied`: raised when an operation that requires authenticated access is attempted via an unauthenticated request. - `MissingError`: raised when an operation is attempted on a record that does not exist. - `ValidationError`: raised when an operation violates a SQL or Python constraint. - All other exceptions are internal errors due to a system problem or bug, and raised untouched to the client-side, which should display a traceback. All exceptions take a single message argument. The `test_exceptions` module has been updated to showcase both new and old (deprecated) exceptions. A great many old `except_osv` had a useless title with "Error!" or "Warning", those have been removed, as this is handled by the client-side widget that displays the messages. This commit introduces a more consistent policy for logging errors and warnings: - All messages that do not require administrator attention should be logged at INFO level or lower. This includes all errors that are notified to the user in a friendly manner, even for access right problems or validation errors during business operations. - All messages that indicate a likely misconfiguration or malicious use by the users should be logged at WARNING level, as they typically require administrator attention. - All other unhandled internal errors cannot typically be handled by the user and should be logged at ERROR or higher level, as they require immediate administrator attention. --- addons/document/document.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'addons/document/document.py') diff --git a/addons/document/document.py b/addons/document/document.py index 86eea66cab2..0d08d54e7fb 100644 --- a/addons/document/document.py +++ b/addons/document/document.py @@ -34,7 +34,7 @@ import openerp from openerp import tools from openerp import SUPERUSER_ID from openerp.osv import fields, osv -from openerp.osv.orm import except_orm +from openerp.exceptions import UserError, AccessError, ValidationError import openerp.report.interface from openerp.tools.misc import ustr from openerp.tools.translate import _ @@ -290,7 +290,7 @@ class document_directory(osv.osv): try: self.check_access_rule(cr, uid, ids, pperms[0], context=context) res |= pperms[1] - except except_orm: + except AccessError: pass return res @@ -334,17 +334,17 @@ class document_directory(osv.osv): def write(self, cr, uid, ids, vals, context=None): if not self._check_duplication(cr, uid, vals, ids, op='write'): - raise osv.except_osv(_('ValidateError'), _('Directory name must be unique!')) + raise ValidationError(_('Directory name must be unique!')) return super(document_directory,self).write(cr, uid, ids, vals, context=context) def create(self, cr, uid, vals, context=None): if not self._check_duplication(cr, uid, vals): - raise osv.except_osv(_('ValidateError'), _('Directory name must be unique!')) + raise ValidationError(_('Directory name must be unique!')) newname = vals.get('name',False) if newname: for illeg in ('/', '@', '$', '#'): if illeg in newname: - raise osv.except_osv(_('ValidateError'), _('Directory name contains special characters!')) + raise ValidationError(_('Directory name contains special characters!')) return super(document_directory,self).create(cr, uid, vals, context) class document_directory_dctx(osv.osv): @@ -567,10 +567,10 @@ class document_storage(osv.osv): file_node.content_type = mime return True except Exception, e : - _logger.warning("Cannot save data.", exc_info=True) + _logger.info("Cannot save data.", exc_info=True) # should we really rollback once we have written the actual data? # at the db case (only), that rollback would be safe - raise except_orm(_('Error at doc write!'), str(e)) + raise UserError(ustr(e)) def _str2time(cre): """ Convert a string with time representation (from db) into time (float) @@ -862,13 +862,13 @@ class node_class(object): def create_child(self, cr, path, data=None): """ Create a regular file under this node """ - _logger.warning("Attempted to create a file under %r, not possible.", self) + _logger.info("Attempted to create a file under %r, not possible.", self) raise IOError(errno.EPERM, "Not allowed to create file(s) here.") def create_child_collection(self, cr, objname): """ Create a child collection (directory) under self """ - _logger.warning("Attempted to create a collection under %r, not possible.", self) + _logger.info("Attempted to create a collection under %r, not possible.", self) raise IOError(errno.EPERM, "Not allowed to create folder(s) here.") def rm(self, cr): @@ -1477,7 +1477,7 @@ class node_res_obj(node_class): def get_dav_eprop_DEPR(self, cr, ns, prop): # Deprecated! if ns != 'http://groupdav.org/' or prop != 'resourcetype': - _logger.warning("Who asks for %s:%s?" % (ns, prop)) + _logger.info("Who asks for %s:%s?", ns, prop) return None cntobj = self.context._dirobj.pool.get('document.directory.content') uid = self.context.uid @@ -1980,7 +1980,7 @@ class nodefd_content(StringIO, node_descriptor): elif mode == 'a': StringIO.__init__(self, None) else: - _logger.error("Incorrect mode %s is specified.", mode) + _logger.info("Incorrect mode %s is specified.", mode) raise IOError(errno.EINVAL, "Invalid file mode.") self.mode = mode @@ -2006,7 +2006,7 @@ class nodefd_content(StringIO, node_descriptor): raise NotImplementedError cr.commit() except Exception: - _logger.exception('Cannot update db content #%d for close.', par.cnt_id) + _logger.info('Cannot update db content #%d for close.', par.cnt_id) raise finally: cr.close() @@ -2033,7 +2033,7 @@ class nodefd_static(StringIO, node_descriptor): elif mode == 'a': StringIO.__init__(self, None) else: - _logger.error("Incorrect mode %s is specified.", mode) + _logger.info("Incorrect mode %s is specified.", mode) raise IOError(errno.EINVAL, "Invalid file mode.") self.mode = mode @@ -2058,7 +2058,7 @@ class nodefd_static(StringIO, node_descriptor): raise NotImplementedError cr.commit() except Exception: - _logger.exception('Cannot update db content #%d for close.', par.cnt_id) + _logger.info('Cannot update db content #%d for close.', par.cnt_id) raise finally: cr.close() @@ -2086,7 +2086,7 @@ class nodefd_db(StringIO, node_descriptor): elif mode == 'a': StringIO.__init__(self, None) else: - _logger.error("Incorrect mode %s is specified.", mode) + _logger.info("Incorrect mode %s is specified.", mode) raise IOError(errno.EINVAL, "Invalid file mode.") self.mode = mode -- cgit v1.2.3