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

[zhshzhou at linux.vnet.ibm.com]

From: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>

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 at linux.vnet.ibm.com>
---
 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):
-- 
1.8.5.3