Re: [Kimchi-devel] [PATCH] Controller: Improve Kimchi Specific Exception Reporting

From: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt; The resource and collection class catches exception in the index method and translates to HTTP error code with error message. While this is good and friendly to the user, the exception handling code is somewhat ad-hoc. The first problem is that it does not catch and translate all the Kimchi exception. When new code or feature is added to Kimchi, it's possible that the new code raises an exception that is not expected before. In this case it fails to translate the error message, so the cherrypy framework translates it to HTTP code 500 with a vague message saying "The server encountered an internal error". This is not friendly to the user. In this patch it "except KimchiException" at the last of the except sequence, and translates it into HTTP error code 500 with specific error message, so that the user knows what's going on.

The second problem is that the index method handles GET, POST, PUT and DELETE requests, and the error translating code for each type of request is duplicated. This is not necessary. So this patch merges the error handling code.

Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt; --- src/kimchi/control/base.py | 76 +++++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 44 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index d2d041e..320239b 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -26,6 +26,7 @@ from kimchi.control.utils import get_class_name, internal_redirect, model_fn from kimchi.control.utils import parse_request, validate_method from kimchi.control.utils import validate_params from kimchi.exception import InvalidOperation, InvalidParameter +from kimchi.exception import KimchiException from kimchi.exception import MissingParameter, NotFoundError, OperationFailed @@ -87,6 +88,8 @@ class Resource(object): raise cherrypy.HTTPError(500, e.message) except NotFoundError, e: raise cherrypy.HTTPError(404, e.message) + except KimchiException, e: + raise cherrypy.HTTPError(500, e.message) wrapper.__name__ = action_name wrapper.exposed = True @@ -116,31 +119,20 @@ class Resource(object): @cherrypy.expose def index(self): method = validate_method(('GET', 'DELETE', 'PUT')) - if method == 'GET': - try: - return self.get() - except NotFoundError, e: - raise cherrypy.HTTPError(404, e.message) - except InvalidOperation, e: - raise cherrypy.HTTPError(400, e.message) - except OperationFailed, e: - raise cherrypy.HTTPError(406, e.message) - elif method == 'DELETE': - try: - return self.delete() - except NotFoundError, e: - raise cherrypy.HTTPError(404, e.message) - except InvalidParameter, e: - raise cherrypy.HTTPError(400, e.message) - elif method == 'PUT': - try: - return self.update() - except InvalidParameter, e: - raise cherrypy.HTTPError(400, e.message) - except InvalidOperation, e: - raise cherrypy.HTTPError(400, e.message) - except NotFoundError, e: - raise cherrypy.HTTPError(404, e.message) + try: + return {'GET': self.get, + 'DELETE': self.delete, + 'PUT': self.update}[method]() + except InvalidOperation, e: + raise cherrypy.HTTPError(400, e.message) + except InvalidParameter, e: + raise cherrypy.HTTPError(400, e.message) + except NotFoundError, e: + raise cherrypy.HTTPError(404, e.message) + except OperationFailed, e: + raise cherrypy.HTTPError(500, e.message) + except KimchiException, e: + raise cherrypy.HTTPError(500, e.message) def update(self): try: @@ -269,29 +261,25 @@ class Collection(object): @cherrypy.expose def index(self, *args, **kwargs): method = validate_method(('GET', 'POST')) - if method == 'GET': - try: + try: + if method == 'GET': filter_params = cherrypy.request.params validate_params(filter_params, self, 'get_list') return self.get(filter_params) - except InvalidOperation, e: - raise cherrypy.HTTPError(400, e.message) - except NotFoundError, e: - raise cherrypy.HTTPError(404, e.message) - - elif method == 'POST': - try: + elif method == 'POST': return self.create(parse_request(), *args) - except MissingParameter, e: - raise cherrypy.HTTPError(400, e.message) - except InvalidParameter, e: - raise cherrypy.HTTPError(400, e.message) - except OperationFailed, e: - raise cherrypy.HTTPError(500, e.message) - except InvalidOperation, e: - raise cherrypy.HTTPError(400, e.message) - except NotFoundError, e: - raise cherrypy.HTTPError(404, e.message) + except InvalidOperation, e: + raise cherrypy.HTTPError(400, e.message) + except InvalidParameter, e: + raise cherrypy.HTTPError(400, e.message) + except MissingParameter, e: + raise cherrypy.HTTPError(400, e.message) + except NotFoundError, e: + raise cherrypy.HTTPError(404, e.message) + except OperationFailed, e: + raise cherrypy.HTTPError(500, e.message) + except KimchiException, e: + raise cherrypy.HTTPError(500, e.message) class AsyncCollection(Collection):