Merge pull request #186 from simphony/175-error-payload

Error payload

doc/Makefile

@@ -44,6 +44,7 @@ help:
 	@echo "  coverage   to run coverage check of the documentation (if enabled)"
 	@echo "  dummy      to check syntax errors of document sources"
 	@echo "  uml        to convert all uml files into images"
+	@echo "  apidoc     to rebuild the apidoc documents"
 
 .PHONY: clean
 clean:

doc/source/api/remoteappmanager.rest.http.rst

@@ -0,0 +1,30 @@
+remoteappmanager.rest.http package
+==================================
+
+Submodules
+----------
+
+remoteappmanager.rest.http.httpstatus module
+--------------------------------------------
+
+.. automodule:: remoteappmanager.rest.http.httpstatus
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
+remoteappmanager.rest.http.payloaded_http_error module
+------------------------------------------------------
+
+.. automodule:: remoteappmanager.rest.http.payloaded_http_error
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
+
+Module contents
+---------------
+
+.. automodule:: remoteappmanager.rest.http
+    :members:
+    :undoc-members:
+    :show-inheritance:

doc/source/api/remoteappmanager.rest.rst

@@ -1,6 +1,13 @@
 remoteappmanager.rest package
 =============================
 
+Subpackages
+-----------
+
+.. toctree::
+
+    remoteappmanager.rest.http
+
 Submodules
 ----------
 
@@ -12,14 +19,6 @@ remoteappmanager.rest.exceptions module
     :undoc-members:
     :show-inheritance:
 
-remoteappmanager.rest.httpstatus module
----------------------------------------
-
-.. automodule:: remoteappmanager.rest.httpstatus
-    :members:
-    :undoc-members:
-    :show-inheritance:
-
 remoteappmanager.rest.registry module
 -------------------------------------
 

doc/source/api/remoteappmanager.rst

@@ -58,6 +58,14 @@ remoteappmanager.jinja2_adapters module
     :undoc-members:
     :show-inheritance:
 
+remoteappmanager.netutils module
+--------------------------------
+
+.. automodule:: remoteappmanager.netutils
+    :members:
+    :undoc-members:
+    :show-inheritance:
+
 remoteappmanager.paths module
 -----------------------------
 

remoteappmanager/rest/exceptions.py

@@ -1,4 +1,4 @@
-from remoteappmanager.rest import httpstatus
+from remoteappmanager.rest.http import httpstatus
 
 
 class RESTException(Exception):
@@ -9,6 +9,26 @@ class RESTException(Exception):
     #: Missing any better info, default is a server error.
     http_code = httpstatus.INTERNAL_SERVER_ERROR
 
+    def __init__(self, message=None, **kwargs):
+        """Initializes the exception. keyword arguments will become
+        part of the representation as key/value pairs."""
+        self.message = message
+        self.info = kwargs if len(kwargs) else None
+
+    def representation(self):
+        """Returns a dictionary with the representation of the exception.
+        """
+        data = {
+            "type": type(self).__name__
+        }
+        if self.message is not None:
+            data["message"] = self.message
+
+        if self.info is not None:
+            data.update(self.info)
+
+        return data
+
 
 class NotFound(RESTException):
     """Exception raised when the resource is not found.
@@ -17,6 +37,11 @@ class NotFound(RESTException):
     """
     http_code = httpstatus.NOT_FOUND
 
+    def representation(self):
+        """NotFound is special as it does not have a representation,
+        just an error status"""
+        return None
+
 
 class BadRequest(RESTException):
     """Exception raised when the resource representation is
@@ -27,5 +52,8 @@ class BadRequest(RESTException):
     http_code = httpstatus.BAD_REQUEST
 
 
-class InternalServerError(RESTException):
-    pass
+class Unable(RESTException):
+    """Exception raised when the CRUD request cannot be performed
+    for whatever reason that is not dependent on the client.
+    """
+    http_code = httpstatus.INTERNAL_SERVER_ERROR

remoteappmanager/rest/http/payloaded_http_error.py

@@ -0,0 +1,36 @@
+from tornado.web import HTTPError
+
+
+class PayloadedHTTPError(HTTPError):
+    def __init__(self, status_code,
+                 payload=None,
+                 content_type=None,
+                 log_message=None,
+                 *args, **kwargs):
+        """Provides a HTTPError that contains a string payload to output
+        as a response. If the payload is None, behaves like a regular
+        HTTPError, producing no payload in the response.
+
+        Parameters
+        ----------
+        payload: str or None
+            The payload as a string
+        content_type: str or None
+            The content type of the payload
+        log_message: str or None
+            The log message. Passed to the HTTPError.
+        """
+        super().__init__(status_code, log_message, *args, **kwargs)
+
+        if payload is not None:
+            if not isinstance(payload, str):
+                raise ValueError("payload must be a string.")
+
+            if content_type is None:
+                content_type = "text/plain"
+        else:
+            if content_type is not None:
+                raise ValueError("Content type specified, but no payload")
+
+        self.content_type = content_type
+        self.payload = payload

remoteappmanager/rest/rest_handler.py

@@ -1,9 +1,10 @@
 from remoteappmanager.handlers.base_handler import BaseHandler
-from tornado import gen, web, escape
-
+from remoteappmanager.rest import exceptions
+from remoteappmanager.rest.http import httpstatus
+from remoteappmanager.rest.http.payloaded_http_error import PayloadedHTTPError
 from remoteappmanager.rest.registry import registry
 from remoteappmanager.utils import url_path_join, with_end_slash
-from remoteappmanager.rest import httpstatus, exceptions
+from tornado import gen, web, escape
 
 
 class RESTBaseHandler(BaseHandler):
@@ -25,6 +26,44 @@ def get_resource_handler_or_404(self, collection_name):
         except KeyError:
             raise web.HTTPError(httpstatus.NOT_FOUND)
 
+    def write_error(self, status_code, **kwargs):
+        """Provides appropriate payload to the response in case of error.
+        """
+        exc_info = kwargs.get("exc_info")
+
+        if exc_info is None:
+            self.clear_header('Content-Type')
+            self.finish()
+
+        exc = exc_info[1]
+
+        if isinstance(exc, PayloadedHTTPError) and exc.payload is not None:
+            self.set_header('Content-Type', exc.content_type)
+            self.finish(exc.payload)
+        else:
+            # For non-payloaded http errors or any other exception
+            # we don't want to return anything as payload.
+            # The error code is enough.
+            self.clear_header('Content-Type')
+            self.finish()
+
+    def rest_to_http_exception(self, rest_exc):
+        """Converts a REST exception into the appropriate HTTP one."""
+
+        representation = rest_exc.representation()
+        payload = None
+        content_type = None
+
+        if representation is not None:
+            payload = escape.json_encode(representation)
+            content_type = "application/json"
+
+        return PayloadedHTTPError(
+            status_code=rest_exc.http_code,
+            payload=payload,
+            content_type=content_type
+        )
+
 
 class RESTCollectionHandler(RESTBaseHandler):
     """Handler for URLs addressing a collection.
@@ -38,7 +77,7 @@ def get(self, collection_name):
         try:
             items = yield res_handler.items()
         except exceptions.RESTException as e:
-            raise web.HTTPError(e.http_code)
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
@@ -67,7 +106,7 @@ def post(self, collection_name):
         try:
             resource_id = yield res_handler.create(data)
         except exceptions.RESTException as e:
-            raise web.HTTPError(e.http_code)
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
@@ -87,6 +126,7 @@ def post(self, collection_name):
 
         self.set_status(httpstatus.CREATED)
         self.set_header("Location", location)
+        self.clear_header('Content-Type')
         self.flush()
 
 
@@ -104,7 +144,7 @@ def get(self, collection_name, identifier):
         try:
             representation = yield res_handler.retrieve(identifier)
         except exceptions.RESTException as e:
-            raise web.HTTPError(e.http_code)
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
@@ -128,6 +168,8 @@ def post(self, collection_name, identifier):
 
         try:
             exists = yield res_handler.exists(identifier)
+        except exceptions.RESTException as e:
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
@@ -156,7 +198,7 @@ def put(self, collection_name, identifier):
         try:
             yield res_handler.update(identifier, representation)
         except exceptions.RESTException as e:
-            raise web.HTTPError(e.http_code)
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
@@ -166,6 +208,7 @@ def put(self, collection_name, identifier):
                     identifier))
             raise web.HTTPError(httpstatus.INTERNAL_SERVER_ERROR)
 
+        self.clear_header('Content-Type')
         self.set_status(httpstatus.NO_CONTENT)
 
     @web.authenticated
@@ -176,7 +219,7 @@ def delete(self, collection_name, identifier):
         try:
             yield res_handler.delete(identifier)
         except exceptions.RESTException as e:
-            raise web.HTTPError(e.http_code)
+            raise self.rest_to_http_exception(e)
         except NotImplementedError:
             raise web.HTTPError(httpstatus.METHOD_NOT_ALLOWED)
         except Exception:
@@ -186,4 +229,5 @@ def delete(self, collection_name, identifier):
                     identifier))
             raise web.HTTPError(httpstatus.INTERNAL_SERVER_ERROR)
 
+        self.clear_header('Content-Type')
         self.set_status(httpstatus.NO_CONTENT)