return 400 status for client action errors

chisel/action.py

@@ -30,6 +30,10 @@
 from .url import decode_query_string
 
 
+STATUS_400 = '400 Bad Request'
+STATUS_500 = '500 Internal Server Error'
+
+
 def action(_action_callback=None, **kwargs):
     """
     Chisel action request decorator
@@ -42,7 +46,7 @@ class ActionError(Exception):
     Action error response exception
     """
 
-    __slots__ = ('error', 'message')
+    __slots__ = ('error', 'message', 'status')
 
     def __init__(self, error, message=None, status=None):
         Exception.__init__(self, error)
@@ -51,13 +55,11 @@ def __init__(self, error, message=None, status=None):
         self.status = status
 
 
-class _ActionErrorInternal(Exception):
-    __slots__ = ('error', 'message', 'member')
+class _ActionErrorInternal(ActionError):
+    __slots__ = ('member')
 
-    def __init__(self, error, message=None, member=None):
-        Exception.__init__(self, error)
-        self.error = error
-        self.message = message
+    def __init__(self, error, message=None, status=None, member=None):
+        ActionError.__init__(self, error, message=message, status=status)
         self.member = member
 
 
@@ -99,7 +101,7 @@ def onload(self, app):
         # Get the action model, if necessary
         if self.model is None:
             self.model = app.specs.actions.get(self.name)
-            assert self.model is not None, "No spec defined for action '%s'" % (self.name,)
+            assert self.model is not None, "No spec defined for action '{0}'".format(self.name)
             self.doc = self.model.doc
 
     def __call__(self, environ, dummy_start_response):
@@ -114,7 +116,7 @@ def __call__(self, environ, dummy_start_response):
                 content = None if is_get else environ['wsgi.input'].read()
             except:
                 ctx.log.warning("I/O error reading input for action '%s'", self.name)
-                raise _ActionErrorInternal('IOError', 'Error reading request content')
+                raise _ActionErrorInternal('IOError', message='Error reading request content')
 
             # De-serialize the JSON content
             validate_mode = VALIDATE_JSON_INPUT
@@ -127,7 +129,7 @@ def __call__(self, environ, dummy_start_response):
                     request = {}
             except Exception as exc:
                 ctx.log.warning("Error decoding JSON content for action '%s'", self.name)
-                raise _ActionErrorInternal('InvalidInput', 'Invalid request JSON: ' + str(exc))
+                raise _ActionErrorInternal('InvalidInput', message='Invalid request JSON: ' + str(exc), status=STATUS_400)
 
             # Decode the query string
             query_string = environ.get('QUERY_STRING')
@@ -137,12 +139,14 @@ def __call__(self, environ, dummy_start_response):
                     request_query_string = decode_query_string(query_string)
                 except Exception as exc:
                     ctx.log.warning("Error decoding query string for action '%s': %s", self.name, environ.get('QUERY_STRING', ''))
-                    raise _ActionErrorInternal('InvalidInput', str(exc))
+                    raise _ActionErrorInternal('InvalidInput', message=str(exc), status=STATUS_400)
 
                 for request_key, request_value in request_query_string.items():
                     if request_key in request:
                         ctx.log.warning("Duplicate query string argument member '%s' for action '%s'", request_key, self.name)
-                        raise _ActionErrorInternal('InvalidInput', "Duplicate query string argument member '%s'" % (request_key,))
+                        raise _ActionErrorInternal('InvalidInput',
+                                                   message="Duplicate query string argument member '{0}'".format(request_key),
+                                                   status=STATUS_400)
                     request[request_key] = request_value
 
             # Add url arguments
@@ -151,7 +155,9 @@ def __call__(self, environ, dummy_start_response):
                 for url_arg, url_value in ctx.url_args.items():
                     if url_arg in request:
                         ctx.log.warning("Duplicate URL argument member '%s' for action '%s'", url_arg, self.name)
-                        raise _ActionErrorInternal('InvalidInput', "Duplicate URL argument member '%s'" % (url_arg,))
+                        raise _ActionErrorInternal('InvalidInput',
+                                                   message="Duplicate URL argument member '{0}'".format(url_arg),
+                                                   status=STATUS_400)
                     request[url_arg] = url_value
 
             # JSONP?
@@ -164,7 +170,7 @@ def __call__(self, environ, dummy_start_response):
                 request = self.model.input_type.validate(request, validate_mode)
             except ValidationError as exc:
                 ctx.log.warning("Invalid input for action '%s': %s", self.name, str(exc))
-                raise _ActionErrorInternal('InvalidInput', str(exc), exc.member)
+                raise _ActionErrorInternal('InvalidInput', message=str(exc), member=exc.member, status=STATUS_400)
 
             # Call the action callback
             try:
@@ -175,14 +181,13 @@ def __call__(self, environ, dummy_start_response):
                 elif response is None:
                     response = {}
                 elif 'error' in response and not jsonp:
-                    status = '500 Internal Server Error'
+                    status = STATUS_500
             except ActionError as exc:
-                status = exc.status or '500 Internal Server Error'
+                status = exc.status or STATUS_500
                 response = {'error': exc.error}
                 if exc.message is not None:
                     response['message'] = exc.message
             except Exception as exc:
-                status = '500 Internal Server Error'
                 ctx.log.exception("Unexpected error in action '%s'", self.name)
                 raise _ActionErrorInternal('UnexpectedError')
 
@@ -199,10 +204,10 @@ def __call__(self, environ, dummy_start_response):
                     response_type.validate(response, mode=VALIDATE_DEFAULT)
                 except ValidationError as exc:
                     ctx.log.error("Invalid output returned from action '%s': %s", self.name, str(exc))
-                    raise _ActionErrorInternal('InvalidOutput', str(exc), exc.member)
+                    raise _ActionErrorInternal('InvalidOutput', message=str(exc), member=exc.member)
 
         except _ActionErrorInternal as exc:
-            status = '500 Internal Server Error'
+            status = exc.status or STATUS_500
             response = {'error': exc.error}
             if exc.message is not None:
                 response['message'] = exc.message

chisel/doc.py

@@ -60,7 +60,7 @@ def _action_callback(ctx, req):
             content = root.serialize(indent='  ' if ctx.app.pretty_output else '')
             return ctx.response_text('200 OK', content, content_type='text/html')
         else:
-            return ctx.response_text('500 Internal Server Error', 'Unknown Request')
+            return ctx.response_text('404 Not Found', 'Unknown Request')
 
 
 class DocPage(Action):

chisel/tests/test_action.py

@@ -289,7 +289,7 @@ def my_action(dummy_app, req):
 
         # Duplicate query string argument
         status, headers, response = app.request('POST', '/my_action', query_string='a=7', wsgi_input=b'{"a": 7, "b": 8}')
-        self.assertEqual(status, '500 Internal Server Error')
+        self.assertEqual(status, '400 Bad Request')
         self.assertEqual(sorted(headers), [('Content-Length', '79'), ('Content-Type', 'application/json')])
         self.assertEqual(response.decode('utf-8'), '{"error":"InvalidInput","message":"Duplicate query string argument member \'a\'"}')
 
@@ -472,7 +472,7 @@ def my_action(dummy_app, dummy_req):
         app.add_request(my_action)
 
         status, headers, response = app.request('GET', '/my_action', query_string='a')
-        self.assertEqual(status, '500 Internal Server Error')
+        self.assertEqual(status, '400 Bad Request')
         self.assertEqual(sorted(headers), [('Content-Length', '63'),
                                            ('Content-Type', 'application/json')])
         self.assertEqual(response.decode('utf-8'), '{"error":"InvalidInput","message":"Invalid key/value pair \'a\'"}')
@@ -492,7 +492,7 @@ def my_action(dummy_app, dummy_req):
         app.add_request(my_action)
 
         status, headers, response = app.request('POST', '/my_action', wsgi_input=b'{a: 7}')
-        self.assertEqual(status, '500 Internal Server Error')
+        self.assertEqual(status, '400 Bad Request')
         self.assertTrue(any(header for header in headers if header[0] == 'Content-Length'))
         self.assertEqual(sorted(header for header in headers if header[0] != 'Content-Length'),
                          [('Content-Type', 'application/json')])
@@ -533,7 +533,7 @@ def my_action(dummy_app, dummy_req):
         app.add_request(my_action)
 
         status, headers, response = app.request('POST', '/my_action', wsgi_input=b'{"a": 7}')
-        self.assertEqual(status, '500 Internal Server Error')
+        self.assertEqual(status, '400 Bad Request')
         self.assertEqual(sorted(headers), [('Content-Length', '117'),
                                            ('Content-Type', 'application/json')])
         self.assertEqual(response.decode('utf-8'),

chisel/tests/test_app.py

@@ -199,7 +199,7 @@ def test_request(self):
         # Request action matched by regex - duplicate member error
         status, headers, response = self.app.request('GET', '/my_action3/123', query_string='myArg=321')
         self.assertEqual(response.decode('utf-8'), '{"error":"InvalidInput","message":"Duplicate URL argument member \'myArg\'"}')
-        self.assertEqual(status, '500 Internal Server Error')
+        self.assertEqual(status, '400 Bad Request')
         self.assertTrue(('Content-Type', 'application/json') in headers)
 
     def test_log_format_callable(self):

chisel/tests/test_doc.py

@@ -248,7 +248,8 @@ def test_action(self):
         # Validate the first my_action1's HTML
         environ = dict(self._environ)
         environ['QUERY_STRING'] = 'name=my_action1'
-        dummy_status, dummy_headers, response = self.app.request('GET', '/doc', environ=environ)
+        status, dummy_headers, response = self.app.request('GET', '/doc', environ=environ)
+        self.assertEqual(status, '200 OK')
         html = response.decode('utf-8')
         html_expected = '''\
 <!doctype html>
@@ -614,7 +615,8 @@ def test_action(self):
         # Validate the my_action2's HTML
         environ = dict(self._environ2)
         environ['QUERY_STRING'] = 'name=my_action2'
-        dummy_status, dummy_headers, response = self.app.request('GET', '/doc', environ=environ)
+        status, dummy_headers, response = self.app.request('GET', '/doc', environ=environ)
+        self.assertEqual(status, '200 OK')
         html = response.decode('utf-8')
         html_expected = '''\
 <!doctype html>
@@ -829,7 +831,8 @@ def my_request(dummy_environ, dummy_start_response):
         application.add_request(DocAction())
         application.add_request(my_request)
 
-        dummy_status, dummy_headers, response = application.request('GET', '/doc', query_string='name=my_request')
+        status, dummy_headers, response = application.request('GET', '/doc', query_string='name=my_request')
+        self.assertEqual(status, '200 OK')
         html = response.decode('utf-8')
         html_expected = '''\
 <!doctype html>
@@ -976,6 +979,20 @@ def my_request(dummy_environ, dummy_start_response):
 </html>'''
         self.assertEqual(html_expected, html)
 
+    def test_request_not_found(self):
+
+        @request(doc='''
+This is the request documentation.
+''')
+        def my_request(dummy_environ, dummy_start_response):
+            pass
+        application = Application()
+        application.add_request(DocAction())
+        application.add_request(my_request)
+
+        status, dummy_headers, response = application.request('GET', '/doc', query_string='name=my_unknown_request')
+        self.assertEqual(status, '404 Not Found')
+        self.assertEqual(response, b'Unknown Request')
 
     # Test doc generation element class
     def test_element(self):