Error management

.gitignore

@@ -75,3 +75,4 @@ htmlcov
 *.tmp
 *~
 
+*.sqlite

dev-requirements.txt

@@ -1,6 +1,6 @@
-pytest>=2.8,<2.9
-pytest-cov>=2.2,<2.3
-pytest-asyncio>=0.3,<0.4
-flake8>=2.5,<2.6
-tox>=2.3,<2.4
-sphinx>=1.3,<1.4
+pytest>=3.0
+pytest-cov>=2.2
+pytest-asyncio>=0.3
+flake8>=2.5
+tox>=2.3
+sphinx>=1.3

requirements.txt

@@ -1,5 +1,5 @@
 requests>=2.8,<2.9
-aiohttp>=0.22
+aiohttp>=0.22,<1.0.0
 aiohttp-jinja2>=0.6
 jinja2>=2.8,<2.9
 multidict>=1.2.2,<2.0.0

src/tygs/app.py

@@ -161,23 +161,35 @@ def on_main_future_done(fut):
                 self._finish()
 
     async def async_stop(self):
+        await self.change_state('stopping')
         return await self.change_state('stop')
 
     def stop(self):
         """
         Stops the loop, which will trigger a clean app stop later.
         """
 
-        self.state = 'stopping'
-        if self.loop.is_running():
+        def close_loop(*args, **kwargs):
             # This stops the loop, and activate ready()'s finally which
             # will enventually call self._stop().
             self.loop.stop()
 
+        if self.loop.is_running():
+            coro = self.change_state('stopping')
+            fut = asyncio.ensure_future(coro)
+            fut.add_done_callback(close_loop)
+        elif not self.loop.is_closed():
+            coro = self.change_state('stopping')
+            self.loop.run_until_complete(coro)
+        else:
+            self.state = 'stopping'
+
     def break_loop_with_error(self, msg, exception=RuntimeError):
         # Silence other exception handlers, since we want to break
         # everything.
         with silence_loop_error_log(self.loop):
+            if isinstance(msg, DebugException):
+                raise msg
             raise DebugException(exception(msg))
 
     def _add_signal_handlers(self, names, callback):

src/tygs/components.py

@@ -12,7 +12,7 @@
 from aiohttp.web import RequestHandlerFactory, RequestHandler
 import werkzeug
 
-from .utils import ensure_coroutine, HTTP_VERBS
+from .utils import ensure_coroutine, HTTP_VERBS, DebugException
 from .http.server import HttpRequestController, Router
 from .exceptions import HttpResponseControllerError
 
@@ -121,7 +121,7 @@ def decorator(func):
 
             @wraps(func)
             async def handler_wrapper(req, res):
-                if not lazy_body:
+                if req.expect_body and req.body.is_present and not lazy_body:
                     await req.load_body()
                 return await func(req, res)
 
@@ -133,15 +133,13 @@ async def handler_wrapper(req, res):
         return decorator
 
     def on_error(self, code, lazy_body=False):
+
         def decorator(func):
 
             func = ensure_coroutine(func)
 
             @wraps(func)
             async def handler_wrapper(req, res):
-
-                if not lazy_body and req._aiohttp_request.has_body:
-                    await req.load_body()
                 return await func(req, res)
 
             # TODO: allow passing explicit endpoint
@@ -194,7 +192,7 @@ async def _call_request_handler(self, req, handler):
 
         try:
             await handler(req, req.response)
-        except Exception:
+        except Exception as e:
             # TODO: provide a debug web page and disable this
             # on prod
             handler = await self._router.get_error_handler(500)
@@ -208,6 +206,21 @@ async def _call_request_handler(self, req, handler):
             # logging.error(e, exc_info=True)
             await handler(req, resp)
 
+            return e
+
+    async def handle_error(self, status=500, message=None,
+                           payload=None, exc=None, headers=None, reason=None):
+
+            # keep aiohttp behavior when the exception is
+            # errors.HttpProcessingError since it's part of the HTTP
+            # workflow
+            if headers is not None:
+                return await super().handle_error(status, message, payload,
+                                                  exc, headers, reason)
+
+            # else we do nothing and let it the behavior in
+            # _call_request_handler take precendence
+
     async def handle_request(self, message, payload):
         if self.access_log:
             now = self._loop.time()
@@ -230,7 +243,7 @@ async def handle_request(self, message, payload):
         ############
 
         ###############
-        await self._call_request_handler(tygs_request, handler)
+        exception = await self._call_request_handler(tygs_request, handler)
 
         ###############
 
@@ -240,6 +253,12 @@ async def handle_request(self, message, payload):
         response = tygs_request.response
         resp_msg = await self._write_response_to_client(tygs_request, response)
 
+        if exception is not None and self.tygs_app.fail_fast_mode:
+            # we still return the response because we have to (i.e. our tests
+            # depends on an HTTP response)
+            # import pdb; pdb.set_trace()
+            raise DebugException(exception)
+
         # for repr
         self._meth = 'none'
         self._path = 'none'

src/tygs/http/server.py

@@ -4,15 +4,18 @@
 
 from textwrap import dedent
 
+import http.cookies
+
 from aiohttp.web_reqrep import Response
 from aiohttp.helpers import reify
+from multidict import CIMultiDict
 
 from werkzeug.routing import Map, Rule
 
 from tygs.exceptions import (HttpRequestControllerError,
                              HttpResponseControllerError,
                              RoutingError)
-from tygs.utils import HTTP_VERBS, removable_property
+from tygs.utils import HTTP_VERBS
 
 
 # TODO: move this function and other renderers to a dedicated module
@@ -40,10 +43,66 @@ def no_renderer(response):
     """.format(response.request.handler, response)))
 
 
+class HttpBody(CIMultiDict):
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.is_loaded = False
+        self.is_present = False
+
+    def fail_on_body_no_loaded(self):
+        if self.is_present and not self.is_loaded:
+
+            raise HttpRequestControllerError(dedent("""
+                You must await "HttpRequestController.load_body()" before
+                accessing "HttpRequestController.body" mapping.
+
+                This error can happen when you read "req.body['something']" or
+                "req['something']" (because it looks up in "req.body"
+                after "req.query_args").
+
+                "HttpRequestController.load_body()" is called automatically
+                unless you used "lazy_body=True" in your routing code, so
+                check it out.
+            """))
+
+    def __repr__(self):
+
+        if self.is_present:
+            if self.is_loaded:
+                return super().__repr__().replace('CIMultiDict',
+                                                  'HttpBody')
+            else:
+                return "<HttpBody [body in request but not loaded]>"
+        else:
+            return "<HttpBody [no body in request]>"
+
+
+# inject test guarding against accessing an not loaded body
+# in all the data manipulation methods
+for method_name in ('__getitem__', '__iter__', 'items', 'keys', 'values',
+                    '__len__', 'copy', 'getone', 'getall', 'get',
+                    'pop', 'popitem', 'setdefault'):
+
+    # small hack with a closure to ensure the method name is stored with
+    # the function
+    def closure():
+        name = method_name
+
+        def method(self, *args, **kwargs):
+            self.fail_on_body_no_loaded()
+            return getattr(super(HttpBody, self), name)(*args, **kwargs)
+        return method
+
+    setattr(HttpBody, method_name, closure())
+
+
+# TODO : remove "Controller" is this name
 class HttpRequestController:
 
     # TODO: decouple aiothttp_request from HttpRequestController
     # TODO: decouple httprequestcontroller from httprequest
+    # TODO: make an alternative constructor called from_aiothttp_request
     def __init__(self, app, aiohttp_request):
         self.app = app
         self._aiohttp_request = aiohttp_request
@@ -52,7 +111,10 @@ def __init__(self, app, aiohttp_request):
         self.script_name = None  # TODO : figure out what script name does
         self.subdomain = None  # TODO: figure out subdomain handling
         self.method = aiohttp_request.method
+        self.expect_body = self.method in ('POST', 'PATCH', 'PUT')
         self.response = HttpResponseController(self)
+        self.body = HttpBody()
+        self.body.is_present = aiohttp_request.has_body
 
         # TODO: check if we can improve the user experience with the multidict
         # API (avoid the confusion of when getting multiple values, etc)
@@ -82,25 +144,26 @@ def __getitem__(self, name):
         except KeyError:
             pass
 
-        try:
-            return self.body[name]
-        except HttpRequestControllerError as e:
-            raise HttpRequestControllerError(dedent("""
-                When you call HttpRequestController.__getitem__() (e.g: when
-                you do req['something']), it implicitly tries to access
-                HttpRequestController.body.
-
-                If you see this error, it means you did it but you didn't call
-                HttpRequestController.load_body() before, which is necessary
-                to load and parse the request body.
-
-                HttpRequestController.load_body is called automatically
-                unless you used "lazy_body=True" in your routing code,
-                so check it out.
-                it out.
-                """)) from e
-        except KeyError as e:
-            pass
+        if self.body.is_present:
+            try:
+                return self.body[name]
+            except HttpRequestControllerError as e:
+                raise HttpRequestControllerError(dedent("""
+                    When you call HttpRequestController.__getitem__() (e.g:
+                    when you do req['something']), it implicitly tries to
+                    access HttpRequestController.body.
+
+                    If you see this error, it means you did it but you didn't
+                    call HttpRequestController.load_body() before, which is
+                    necessary to load and parse the request body.
+
+                    HttpRequestController.load_body is called automatically
+                    unless you used "lazy_body=True" in your routing code,
+                    so check it out.
+                    it out.
+                    """)) from e
+            except KeyError as e:
+                pass
 
         try:
             return self.cookies[name]
@@ -166,24 +229,10 @@ def __getattr__(self, name):
     def url_query(self):
         return self._aiohttp_request.GET
 
-    @removable_property
-    def body(self):
-        raise HttpRequestControllerError(dedent("""
-            You must await "HttpRequestController.load_body()" before accessing
-            "HttpRequestController.body".
-
-            This error can happen when you read "req.body" or
-            "req['something']" (because it looks up in "req.body"
-            after "req.query_args").
-
-            "HttpRequestController.load_body()" is called automatically unless
-            you used "lazy_body=True" in your routing code, so check it out.
-        """))
-
     async def load_body(self):
-        body = await self._aiohttp_request.post()
-        self.body = body
-        return body
+        self.body.update(await self._aiohttp_request.post())
+        self.body.is_loaded = True
+        return self.body
 
 
 # TODO: send log info about multidict values: the user should know if she tries
@@ -208,6 +257,7 @@ def __init__(self, request):
         self.reason = "OK"
         self.charset = "utf-8"
         self.headers = {}
+        self._cookies = http.cookies.SimpleCookie()
 
     def __repr__(self):
         req = self.request
@@ -224,6 +274,16 @@ def __getattr__(self, name):
         # Do not try super().__getattr__ since the parent doesn't define it.
         raise object.__getattribute__(self, name)
 
+    def set_cookie(self, *args, **kwargs):  # TODO: be a little less lazy
+        return Response.set_cookie(self, *args, **kwargs)
+
+    def del_cookie(self, *args, **kwargs):  # TODO: be a little less lazy
+        return Response.del_cookie(self, *args, **kwargs)
+
+    @property
+    def cookies(self):
+        return self._cookies
+
     # TODO: allow template engine to be passed here as a parameter, but
     # also be retrieved from the app configuration. And remove it as an
     # attribute of the HttpResponseController.
@@ -252,7 +312,9 @@ def render_response(self):
         return self._renderer(self)
 
     def _build_aiohttp_response(self):
-        return Response(**self.render_response())
+        resp = Response(**self.render_response())
+        resp._cookies = self._cookies
+        return resp
 
 
 class Router:
@@ -333,11 +395,13 @@ def __init__(self, app):
                                                        8080)
 
     async def start(self):
-        self.server = await asyncio.ensure_future(self._server_factory)
+        self.aiohttp_server = await asyncio.ensure_future(self._server_factory)
         self.app.register('stop', self.stop)
 
     async def stop(self):
         await self.handler.finish_connections(1.0)
-        self.server.close()
-        await self.server.wait_closed()
+        if hasattr(self, 'aiohttp_server'):
+            # the HTTP server may have never started
+            self.aiohttp_server.close()
+            await self.aiohttp_server.wait_closed()
         await self.app._aiohttp_app.finish()

src/tygs/webapp.py

@@ -29,6 +29,7 @@ def __init__(self, *args, factory_adapter=rh, server_class=Server,
     async def async_ready(self, cwd=None):
         self.http_server = self.server_class(self)
         self.register('ready', self.http_server.start)
+        self.register('stopping', self.http_server.stop)
         return await super().async_ready(cwd)
         # TODO: start the aiohttp server, for now
         # TODO: implement