Big rework of how errors are handled and propagated to the HTTP

response.  Now everything (status code, message, possibly tracebacks)
are passed to the client as JSON, so the client can display correct errors.
Nice!


git-svn-id: https://bucket.mit.edu/svn/nilm/nilmdb@10630 ddd99763-3ecb-0310-9145-efcb8ce7c51f

nilmdb/client.py

@@ -14,25 +14,33 @@ import urllib
 import urllib2
 from urllib2 import urlopen, HTTPError
 import pycurl
+import cStringIO
+import re
+import collections
 
 class NilmCommError(Exception):
     """Base exception for both ClientError and ServerError responses"""
-    # Whether to include the body in the __str__ expansion
-    show_body = 1
-    def __init__(self, code, message, body = None, url = "no URL"):
-        Exception.__init__(self, message)
-        self.code = code
-        self.body = body
-        self.url = url
-    def __str__(self): # pragma: no cover
-        s = sprintf("[%03d] %s (%s)", self.code, self.message, self.url)
-        if self.body and self.show_body:
-            s += "\n" + self.body
+    def __init__(self,
+                 status = "Unspecified error",
+                 message = None,
+                 url = None,
+                 traceback = None):
+        Exception.__init__(self, status)
+        self.status = status     # e.g. "400 Bad Request"
+        self.message = message   # textual message from the server
+        self.url = url           # URL we were requesting
+        self.traceback = traceback # server traceback, if available
+    def __str__(self):
+        s = sprintf("[%s]", self.status)
+        if self.message:
+            s += sprintf(" %s", self.message)
+        if self.url:
+            s += sprintf(" (%s)", self.url)
+        if self.traceback: # pragma: no cover
+            s += sprintf("\nServer traceback:\n%s", self.traceback)
         return s
-
 class ClientError(NilmCommError):
     pass
-
 class ServerError(NilmCommError):
     pass
 
@@ -56,26 +64,41 @@ class MyCurl(object):
 
     def _check_error(self, body = None):
         code = self.curl.getinfo(pycurl.RESPONSE_CODE)
+        if code == 200:
+            return
+        # Default variables for exception
+        args = { "url" : self.url,
+                 "status" : str(code),
+                 "message" : None,
+                 "traceback" : None }
+        try:
+            # Fill with server-provided data if we can
+            jsonerror = json.loads(body)
+            args["status"] = jsonerror["status"]
+            args["message"] = jsonerror["message"]
+            args["traceback"] = jsonerror["traceback"]
+        except Exception: # pragma: no cover
+            pass
         if code >= 400 and code <= 499:
-            raise ClientError(code, "HTTP client error", body, self.url)
-        elif code >= 500 and code <= 599: # pragma: no cover
-            raise ServerError(code, "HTTP server error", body, self.url)
-        elif code != 200: # pragma: no cover
-            raise NilmCommError(code, "Unknown error", body, self.url)
+            raise ClientError(**args)
+        else: # pragma: no cover
+            if code >= 500 and code <= 599:
+                raise ServerError(**args)
+            else:
+                raise NilmCommError(**args)
         
     def getjson(self, url, params = None):
         """Simple GET that returns JSON string"""
         self._setup_url(url, params)
-        self._body = ""
-        def body_callback(x):
-            self._body += x
-        self.curl.setopt(pycurl.WRITEFUNCTION, body_callback)
+        body = cStringIO.StringIO()
+        self.curl.setopt(pycurl.WRITEFUNCTION, body.write)
         try:
             self.curl.perform()
         except pycurl.error as e:
-            raise ServerError(500, e[1], self.url)
-        self._check_error(self._body)
-        return json.loads(self._body)
+            raise ServerError(500, self.url, "Curl error: " + e[1])
+        body_str = body.getvalue()
+        self._check_error(body_str)
+        return json.loads(body_str)
 
 class Client(object):
     def __init__(self, url):
@@ -120,55 +143,19 @@ class Client(object):
         return self.curl.getjson("stream/update_metadata", params)
 
     def stream_create(self, path, layout, index = None):
+        """Create a new stream"""
         params = { "path": path,
                    "layout" : layout }
         if index is not None:
             params["index"] = index
         return self.curl.getjson("stream/create", params)
 
-    def stream_insert(self, path):
+    def stream_insert(self, path, data):
+        """Insert data into a stream.  data should be a file-like object
+        that provides ASCII data that matches the database layout for path."""
         params = { "path": path }
         return self.curl.getjson("stream/insert", params)
 
-
-    #     streams = getjson("/stream/list")
-
-    #     eq_(streams, [
-    #         ['/newton/prep', 'PrepData'],
-    #         ['/newton/raw', 'RawData'],
-    #         ['/newton/zzz/rawnotch', 'RawNotchedData'],
-    #         ])
-
-    #     streams = getjson("/stream/list?layout=RawData")
-    #     eq_(streams, [['/newton/raw', 'RawData']])
-
-    #     streams = getjson("/stream/list?layout=NoSuchLayout")
-    #     eq_(streams, [])
-
-    #     with assert_raises(HTTPError) as e:
-    #         getjson("/stream/get_metadata?path=foo")
-    #     eq_(e.exception.code, 404)
-
-    #     data = getjson("/stream/get_metadata?path=/newton/prep")
-    #     eq_(data, {'description': 'The Data', 'v_scale': '1.234'})
-
-    #     data = getjson("/stream/get_metadata?path=/newton/prep"
-    #                    "&key=v_scale")
-    #     eq_(data, {'v_scale': '1.234'})
-
-    #     data = getjson("/stream/get_metadata?path=/newton/prep"
-    #                    "&key=v_scale&key=description")
-    #     eq_(data, {'description': 'The Data', 'v_scale': '1.234'})
-
-    #     data = getjson("/stream/get_metadata?path=/newton/prep"
-    #                    "&key=v_scale&key=foo")
-    #     eq_(data, {'foo': None, 'v_scale': '1.234'})
-
-    #     data = getjson("/stream/get_metadata?path=/newton/prep"
-    #                    "&key=foo")
-    #     eq_(data, {'foo': None})
-
-
     # def test_insert(self):
     #     # invalid path first
     #     with assert_raises(HTTPError) as e:

nilmdb/nilmdb.py

@@ -146,7 +146,7 @@ class NilmDB(object):
 
         layout_name: one of the nilmdb.layout.layouts keys, e.g. 'PrepData'
 
-        index: layout columns listed here are marked as PyTables indices.
+        index: list of layout columns to be marked as PyTables indices.
         If index = none, the 'timestamp' column is indexed if it exists.
         Pass an empty list to prevent indexing.
         """

nilmdb/server.py

@@ -75,18 +75,18 @@ class Stream(NilmApp):
         """Create a new stream in the database.  Provide path
         and one of the nilmdb.layout.layouts keys.
 
-        index: layout columns listed here are marked as PyTables indices.
+        index: list of layout columns to be marked as PyTables indices.
         If index = none, the 'timestamp' column is indexed if it exists.
         Pass an empty list to prevent indexing.
         """
+        # Index needs to be a list, if it's not None
+        if (index is not None) and (not isinstance(index, list)):
+            index = [ index ]
         try:
             return self.db.stream_create(path, layout, index)
-        except ValueError as e:
-            raise cherrypy.HTTPError("400 Bad Request",
-                                     "ValueError: " + e.message)
-        except KeyError as e:
-            raise cherrypy.HTTPError("400 Bad Request",
-                                     "KeyError: " + e.message)
+        except Exception as e:
+            message = sprintf("%s: %s", type(e).__name__, e.message)
+            raise cherrypy.HTTPError("400 Bad Request", message)
 
     # /stream/get_metadata?path=/newton/prep
     # /stream/get_metadata?path=/newton/prep&key=foo&key=bar
@@ -123,8 +123,8 @@ class Stream(NilmApp):
             data_dict = json.loads(data)
             self.db.stream_set_metadata(path, data_dict)
         except Exception as e:
-            raise cherrypy.HTTPError("400 Bad Request",
-                                     "Error: " + e.message)
+            message = sprintf("%s: %s", type(e).__name__, e.message)
+            raise cherrypy.HTTPError("400 Bad Request", message)
         return "ok"
     
     # /stream/update_metadata?path=/newton/prep&data=<json>
@@ -137,8 +137,8 @@ class Stream(NilmApp):
             data_dict = json.loads(data)
             self.db.stream_update_metadata(path, data_dict)
         except Exception as e:
-            raise cherrypy.HTTPError("400 Bad Request",
-                                     "Error: " + e.message)
+            message = sprintf("%s: %s", type(e).__name__, e.message)
+            raise cherrypy.HTTPError("400 Bad Request", message)
         return "ok"
 
     # /stream/upload?path=/newton/prep
@@ -180,10 +180,15 @@ class Server(object):
             'server.socket_host': host,
             'server.socket_port': port,
             'engine.autoreload_on': False,
+            'error_page.default': self.json_error_page,
             })
         if self.embedded:
             cherrypy.config.update({ 'environment': 'embedded' })
 
+        # Send tracebacks in error responses.  They're hidden by the
+        # error_page function for client errors (code 400-499).
+        cherrypy.config.update({ 'request.show_tracebacks' : True })
+
         cherrypy.tree.apps = {}
         cherrypy.tree.mount(Root(self.db, self.version), "/")
         cherrypy.tree.mount(Stream(self.db), "/stream")
@@ -198,6 +203,23 @@ class Server(object):
         else:
             cherrypy.server.shutdown_timeout = 5
 
+    def json_error_page(self, status, message, traceback, version):
+        """Return a custom error page in JSON so the client can parse it"""
+        errordata = { "status" : status,
+                      "message" : message,
+                      "traceback" : traceback }
+        # Don't send a traceback if the error was 400-499 (client's fault)
+        try:
+            code = int(status.split()[0])
+            if code >= 400 and code <= 499:
+                errordata["traceback"] = ""
+        except Exception: # pragma: no cover
+            pass
+        # Override the response type, which was previously set to text/html
+        cherrypy.serving.response.headers['Content-Type'] = (
+            "application/json;charset=utf-8" )
+        return json.dumps(errordata, separators=(',',':'))
+
     def start(self, blocking = False, event = None):
 
         if not self.embedded: # pragma: no cover

tests/test_client.py

@@ -23,30 +23,32 @@ def ne_(a, b):
     if not a != b:
         raise AssertionError("unexpected %r == %r" % (a, b))
 
-class TestClient(object):
+def setup_module():
+    global test_server, test_db
+    # Clear out DB
+    try:
+        shutil.rmtree(testdb)
+    except:
+        pass
+    try:
+        os.unlink(testdb)
+    except:
+        pass
+
+    # Start web app on a custom port
+    test_db = nilmdb.NilmDB(testdb)
+    test_server = nilmdb.Server(test_db, host = "127.0.0.1",
+                                port = 12380, stoppable = False,
+                                fast_shutdown = True)
+    test_server.start(blocking = False)
+
+def teardown_module():
+    global test_server, test_db
+    # Close web app
+    test_server.stop()
+    test_db.close()
 
-    def setUp(self):
-        # Clear out DB
-        try:
-            shutil.rmtree(testdb)
-        except:
-            pass
-        try:
-            os.unlink(testdb)
-        except:
-            pass
-
-        # Start web app on a custom port
-        self.db = nilmdb.NilmDB(testdb)
-        self.server = nilmdb.Server(self.db, host = "127.0.0.1",
-                                    port = 12380, stoppable = False,
-                                    fast_shutdown = True)
-        self.server.start(blocking = False)
-
-    def tearDown(self):
-        # Close web app
-        self.server.stop()
-        self.db.close()
+class TestClient(object):
 
     def test_client_basic(self):
         # Test a fake host
@@ -63,7 +65,7 @@ class TestClient(object):
         client = nilmdb.Client(url = "http://localhost:12380/")
         version = client.version()
         eq_(distutils.version.StrictVersion(version),
-            distutils.version.StrictVersion(self.server.version))
+            distutils.version.StrictVersion(test_server.version))
 
     def test_client_nilmdb(self):
         # Basic stream tests, like those in test_nilmdb:test_stream
@@ -80,9 +82,10 @@ class TestClient(object):
         # Bad layout type
         with assert_raises(ClientError):
             client.stream_create("/newton/prep", "NoSuchLayout")
-        # Bad index columns
-        with assert_raises(ClientError):
+        # Bad index column
+        with assert_raises(ClientError) as e:
             client.stream_create("/newton/prep", "PrepData", ["nonexistant"])
+        assert("KeyError: nonexistant" in str(e.exception))
         client.stream_create("/newton/prep", "PrepData")
         client.stream_create("/newton/raw", "RawData")
         client.stream_create("/newton/zzz/rawnotch", "RawNotchedData")
@@ -119,3 +122,5 @@ class TestClient(object):
         with assert_raises(ClientError):
             client.stream_update_metadata("/newton/prep", [1,2,3])
 
+    def test_client_insert(self):
+        pass