[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.

3 files changed, 27 insertions, 30 deletions

diff --git a/addons/document/content_index.py b/addons/document/content_index.py
index b6f89604c3e..8c2c9209ced 100644
--- a/addons/document/content_index.py
+++ b/addons/document/content_index.py
@@ -23,8 +23,6 @@ import os
 import tempfile
 from subprocess import Popen, PIPE
 _logger = logging.getLogger(__name__)
-class NhException(Exception):
-    pass
 
 
 class indexer(object):
@@ -61,13 +59,13 @@ class indexer(object):
         try:
             if content != None:
                 return self._doIndexContent(content)
-        except NhException:
+        except NotImplementedError:
             pass
 
         if realfile != None:
             try:
                 return self._doIndexFile(realfile)
-            except NhException:
+            except NotImplementedError:
                 pass
 
             fp = open(realfile,'rb')
@@ -90,16 +88,16 @@ class indexer(object):
                 res = self._doIndexFile(rfname)
                 os.unlink(rfname)
                 return res
-            except NhException:
+            except NotImplementedError:
                 pass
 
-        raise NhException('No appropriate method to index file.')
+        raise ValueError('No appropriate method to index file.')
 
     def _doIndexContent(self, content):
-        raise NhException("Content cannot be handled here.")
+        raise NotImplementedError("Content cannot be handled here.")
 
     def _doIndexFile(self, fpath):
-        raise NhException("Content cannot be handled here.")
+        raise NotImplementedError("Content cannot be handled here.")
 
     def __repr__(self):
         return "<indexer %s.%s>" %(self.__module__, self.__class__.__name__)
@@ -130,10 +128,9 @@ class contentIndex(object):
             self.exts[ext] = obj
             f = True
 
-        if f:
-            _logger.debug('Register content indexer: %r.', obj)
         if not f:
-            raise Exception("Your indexer should at least support a mimetype or extension.")
+            raise ValueError("Your indexer should at least support a mimetype or extension.")
+        _logger.debug('Register content indexer: %r.', obj)
 
     def doIndex(self, content, filename=None, content_type=None, realfname=None, debug=False):
         fobj = None
@@ -174,7 +171,7 @@ class contentIndex(object):
                 if not mime:
                     mime = mime2
             except Exception:
-                _logger.exception('Cannot determine mime type.')
+                _logger.info('Cannot determine mime type.', exc_info=True)
 
         try:
             if fobj:
@@ -183,8 +180,8 @@ class contentIndex(object):
                 _logger.debug("Have no object, return (%s, None).", mime)
                 res = (mime, '')
         except Exception:
-            _logger.exception("Cannot index file %s (%s).",
-                                    filename, fname or realfname)
+            _logger.info("Cannot index file %s (%s).",
+                                    filename, fname or realfname, exc_info=True)
             res = (mime, '')
 
         # If we created a tmp file, unlink it now
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
 
diff --git a/addons/document/test/document_test2.yml b/addons/document/test/document_test2.yml
index 0c6345ef6c4..1eed98aa901 100644
--- a/addons/document/test/document_test2.yml
+++ b/addons/document/test/document_test2.yml
@@ -73,7 +73,7 @@
             new_dir_id = self.create(cr, demo_uid, dir_vals, context=None)
             self.unlink(cr, uid, [new_dir_id], context=None)
         except except_osv, e:
-            duplicate_detected = e.value == u'Directory name must be unique!'
+            duplicate_detected = e.name == u'Directory name must be unique!'
         assert duplicate_detected is True, 'Failed to detect duplicate directory'
 -
     I delete the attachments