[trunk] the Exception class is not meant to be raised

Bug #1056657 reported by Alexandre Fayolle - camptocamp
34
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Confirmed
Wishlist
OpenERP R&D Addons Team 1
Odoo Server (MOVED TO GITHUB)
Confirmed
Wishlist
OpenERP's Framework R&D

Bug Description

There are several occurences of "raise Exception" in the OpenERP code base:

./openerp/modules/module.py:197: raise Exception('Cannot access file outside the module')
./openerp/modules/module.py:270: raise Exception('Unable to find path for module %s' % modulename)
./openerp/service/web_services.py:173: raise Exception, e
./openerp/service/web_services.py:200: raise Exception("Couldn't drop database %s: %s" % (db_name, e))
./openerp/service/web_services.py:248: raise Exception, "Couldn't dump database"
./openerp/service/web_services.py:258: raise Exception, "Database already exists"
./openerp/service/web_services.py:285: raise Exception, "Couldn't restore database"
./openerp/service/web_services.py:302: raise Exception("Couldn't rename database %s to %s: %s" % (old_name, new_name, e))
./openerp/service/web_services.py:394: raise Exception("Method not found: %s" % method)
./openerp/service/web_services.py:775: raise Exception, 'AccessDenied'
./openerp/service/web_services.py:777: raise Exception, 'ReportNotFound'
./openerp/service/workers.py:123: raise Exception(msg % wpid)
./openerp/service/workers.py:270: raise Exception('CPU time limit exceeded.')
./openerp/tests/addons/test_exceptions/models.py:29: raise Exception('AccessDenied')
./openerp/netsvc.py:118: raise Exception("stub dispatch at %s" % self.__name)
./openerp/netsvc.py:271: # raise Exception('All instances of servers must be inited before the startAll()')
./openerp/osv/fields.py:126: raise Exception(_('undefined get method !'))
./openerp/osv/fields.py:635: raise Exception(_('The second argument of the many2many field %s must be a SQL table !'\
./openerp/osv/orm.py:399: raise Exception(_('Language with code "%s" is not defined in your system !\nDefine it through the Administration menu.') % (lang,))
./openerp/osv/orm.py:1265: raise Exception(_("Database ID doesn't exist: %s : %s") %(model_name, id))
./openerp/osv/orm.py:1305: raise Exception(_('Please check that all your lines have %d columns.'
./openerp/report/interface.py:48: raise Exception('ConceptionError, bad report name, should start with "report."')
./openerp/report/pyPdf/generic.py:392: raise Exception("no information about original bytes")
./openerp/report/pyPdf/pdf.py:654: raise Exception, "file has not been decrypted"
./openerp/addons/base/module/module.py:301: raise Exception('Unable to find %r in path' % (binary,))
./openerp/tools/config.py:454: raise Exception('ERROR: The Lang name must take max 5 chars, Eg: -lfr_BE')
./openerp/tools/misc.py:92: raise Exception('Couldn\'t find %s' % name)
./openerp/tools/misc.py:100: raise Exception('Couldn\'t find %s' % name)
./openerp/tools/misc.py:111: raise Exception('Couldn\'t find %s' % name)
./openerp/tools/misc.py:394: raise Exception("No database cursor found, please pass one explicitly")
./openerp/tools/translate.py:322: raise Exception("malformed file: bad line: %s" % line)
./openerp/tools/translate.py:336: raise Exception('malformed file at %d'% self.cur_line())
./openerp/tools/translate.py:463: raise Exception(_('Unrecognized extension: must be one of '
./openerp/tools/translate.py:892: raise Exception(_('Bad file format'))
./openerp/tools/convert.py:849: raise Exception( "Mismatch xml format: only terp or openerp as root tag" )
./openerp/tools/convert.py:941: raise Exception(_('Module loading failed: file %s/%s could not be processed:\n %s') % (module, fname, warning_msg))
./openerp/tools/test_reports.py:280: raise Exception("Cannot handle action of type %s" % act_model)
./openerp/tools/test_reports.py:291: raise Exception("Too many loops at action")
./history/migrate/4.0.0-4.2.0/tiny/pre-tiny.py:58:raise Exception('This script is provided as an example, you must custom it before')

This is an evil pattern, and it must be fixed.

The reason this is a Bad Thing is that it forces exception handling code to catch Exception, which will catch more than what is intended, and therefore hide bugs which trigger unrelated exceptions.

OpenERP should follow PEP8 in that regard and use the proper exception classes from the standard library or framework specific exceptions, deriving from a common openerp specific exception class, so that error handling code can handle that base class and be certain that the error originated in OpenERP.

Note that the exception classes in openerp.exceptions and elsewhere do not follow that convention of having a common framework specific base class.

Changed in openobject-server:
status: New → Confirmed
Revision history for this message
Numérigraphe (numerigraphe) wrote :

Oh, another lengthy cleanup needed...

Can somebody from the core team acknowledge the problem, and make sure all the developers are informed and trained about this issue?
Courage!

Lionel Sausin.

Amit Parik (amit-parik)
Changed in openobject-addons:
assignee: nobody → OpenERP R&D Addons Team 1 (openerp-dev-addons1)
importance: Undecided → Wishlist
status: New → Confirmed
Changed in openobject-server:
assignee: nobody → OpenERP's Framework R&D (openerp-dev-framework)
importance: Undecided → Wishlist
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.