[FIX] Better exception handling in tools.convert:

- The exception is not logged in tools.convert. It is loggeg by the netsv layer anyway.
- The exception is re-raised instead, but with the full traceback available.
- A fix in openerp.service.db where the cr is closed twice.

bzr revid: vmt@openerp.com-20130219163047-3q766awd66wkzsn7

openerp/service/db.py

@@ -20,39 +20,44 @@ self_id_protect = threading.Semaphore()
 
 # This should be moved to openerp.modules.db, along side initialize().
 def _initialize_db(id, db_name, demo, lang, user_password):
-    cr = None
     try:
-        self_actions[id]['progress'] = 0
-        cr = openerp.sql_db.db_connect(db_name).cursor()
-        openerp.modules.db.initialize(cr) # TODO this should be removed as it is done by pooler.restart_pool.
-        openerp.tools.config['lang'] = lang
-        cr.commit()
-        cr.close()
+        cr = None
+        try:
+            self_actions[id]['progress'] = 0
+            cr = openerp.sql_db.db_connect(db_name).cursor()
+            openerp.modules.db.initialize(cr) # TODO this should be removed as it is done by pooler.restart_pool.
+            openerp.tools.config['lang'] = lang
+            cr.commit()
+        finally:
+            if cr:
+                cr.close()
+                cr = None
 
         pool = openerp.pooler.restart_pool(db_name, demo, self_actions[id],
                                    update_module=True)[1]
 
-        cr = openerp.sql_db.db_connect(db_name).cursor()
+        try:
+            cr = openerp.sql_db.db_connect(db_name).cursor()
 
-        if lang:
-            modobj = pool.get('ir.module.module')
-            mids = modobj.search(cr, SUPERUSER_ID, [('state', '=', 'installed')])
-            modobj.update_translations(cr, SUPERUSER_ID, mids, lang)
+            if lang:
+                modobj = pool.get('ir.module.module')
+                mids = modobj.search(cr, SUPERUSER_ID, [('state', '=', 'installed')])
+                modobj.update_translations(cr, SUPERUSER_ID, mids, lang)
 
-        # update admin's password and lang
-        values = {'password': user_password, 'lang': lang}
-        pool.get('res.users').write(cr, SUPERUSER_ID, [SUPERUSER_ID], values)
+            # update admin's password and lang
+            values = {'password': user_password, 'lang': lang}
+            pool.get('res.users').write(cr, SUPERUSER_ID, [SUPERUSER_ID], values)
 
-        cr.execute('SELECT login, password FROM res_users ORDER BY login')
-        self_actions[id].update(users=cr.dictfetchall(), clean=True)
-        cr.commit()
-        cr.close()
+            cr.execute('SELECT login, password FROM res_users ORDER BY login')
+            self_actions[id].update(users=cr.dictfetchall(), clean=True)
+            cr.commit()
+        finally:
+            if cr:
+                cr.close()
     except Exception, e:
         self_actions[id].update(clean=False, exception=e)
         _logger.exception('CREATE DATABASE failed:')
         self_actions[id]['traceback'] = traceback.format_exc()
-        if cr:
-            cr.close()
 
 def dispatch(method, params):
     if method in [ 'create', 'get_progress', 'drop', 'dump',

openerp/tools/convert.py

@@ -25,6 +25,7 @@ import logging
 import os.path
 import pickle
 import re
+import sys
 
 # for eval context:
 import time
@@ -61,13 +62,16 @@ from misc import unquote
 unsafe_eval = eval
 from safe_eval import safe_eval as eval
 
-class ConvertError(Exception):
-    def __init__(self, doc, orig_excpt):
-        self.d = doc
-        self.orig = orig_excpt
+class ParseError(Exception):
+    def __init__(self, msg, text, filename, lineno):
+        self.msg = msg
+        self.text = text
+        self.filename = filename
+        self.lineno = lineno
 
     def __str__(self):
-        return 'Exception:\n\t%s\nUsing file:\n%s' % (self.orig, self.d)
+        return '"%s" while parsing %s:%s, near\n%s' \
+            % (self.msg, self.filename, self.lineno, self.text)
 
 def _ref(self, cr):
     return lambda x: self.id_get(cr, x)
@@ -841,13 +845,10 @@ form: module.record_id""" % (xml_id,)
                 if rec.tag in self._tags:
                     try:
                         self._tags[rec.tag](self.cr, rec, n)
-                    except:
-                        _logger.error('Parse error in %s:%d: \n%s',
-                                      rec.getroottree().docinfo.URL,
-                                      rec.sourceline,
-                                      etree.tostring(rec).strip(), exc_info=True)
+                    except Exception, e:
                         self.cr.rollback()
-                        raise
+                        exc_info = sys.exc_info()
+                        raise ParseError, (str(e), etree.tostring(rec).rstrip(), rec.getroottree().docinfo.URL, rec.sourceline), exc_info[2]
         return True
 
     def __init__(self, cr, module, idref, mode, report=None, noupdate=False):