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/fetchmail/fetchmail.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'addons/fetchmail/fetchmail.py') diff --git a/addons/fetchmail/fetchmail.py b/addons/fetchmail/fetchmail.py index 38e7a1b9e4b..9f57e0fe616 100644 --- a/addons/fetchmail/fetchmail.py +++ b/addons/fetchmail/fetchmail.py @@ -37,6 +37,7 @@ from openerp import addons from openerp.osv import fields, osv from openerp import tools, api from openerp.tools.translate import _ +from openerp.exceptions import UserError _logger = logging.getLogger(__name__) @@ -159,8 +160,8 @@ openerp_mailgate: "|/path/to/openerp-mailgate.py --host=localhost -u %(uid)d -p connection = server.connect() server.write({'state':'done'}) except Exception, e: - _logger.exception("Failed to connect to %s server %s.", server.type, server.name) - raise osv.except_osv(_("Connection test failed!"), _("Here is what we got instead:\n %s.") % tools.ustr(e)) + _logger.info("Failed to connect to %s server %s.", server.type, server.name, exc_info=True) + raise UserError(_("Connection test failed: %s") % tools.ustr(e)) finally: try: if connection: @@ -206,7 +207,7 @@ openerp_mailgate: "|/path/to/openerp-mailgate.py --host=localhost -u %(uid)d -p strip_attachments=(not server.attach), context=context) except Exception: - _logger.exception('Failed to process mail from %s server %s.', server.type, server.name) + _logger.info('Failed to process mail from %s server %s.', server.type, server.name, exc_info=True) failed += 1 if res_id and server.action_id: action_pool.run(cr, uid, [server.action_id.id], {'active_id': res_id, 'active_ids': [res_id], 'active_model': context.get("thread_model", server.object_id.model)}) @@ -215,7 +216,7 @@ openerp_mailgate: "|/path/to/openerp-mailgate.py --host=localhost -u %(uid)d -p count += 1 _logger.info("Fetched %d email(s) on %s server %s; %d succeeded, %d failed.", count, server.type, server.name, (count - failed), failed) except Exception: - _logger.exception("General failure when trying to fetch mail from %s server %s.", server.type, server.name) + _logger.info("General failure when trying to fetch mail from %s server %s.", server.type, server.name, exc_info=True) finally: if imap_server: imap_server.close() @@ -237,14 +238,14 @@ openerp_mailgate: "|/path/to/openerp-mailgate.py --host=localhost -u %(uid)d -p context=context) pop_server.dele(num) except Exception: - _logger.exception('Failed to process mail from %s server %s.', server.type, server.name) + _logger.info('Failed to process mail from %s server %s.', server.type, server.name, exc_info=True) failed += 1 if res_id and server.action_id: action_pool.run(cr, uid, [server.action_id.id], {'active_id': res_id, 'active_ids': [res_id], 'active_model': context.get("thread_model", server.object_id.model)}) cr.commit() _logger.info("Fetched %d email(s) on %s server %s; %d succeeded, %d failed.", numMsgs, server.type, server.name, (numMsgs - failed), failed) except Exception: - _logger.exception("General failure when trying to fetch mail from %s server %s.", server.type, server.name) + _logger.info("General failure when trying to fetch mail from %s server %s.", server.type, server.name, exc_info=True) finally: if pop_server: pop_server.quit() -- cgit v1.2.3