fix(security): SecurityMiddleware no longer retries self.app() on exception

tests/test_security_middleware_base.py

@@ -0,0 +1,52 @@
+"""Regression test for issue #17: SecurityMiddleware.__call__ used to catch
+ANY exception from _on_request. Since PR #16, self.app(request) itself
+essentially never raises anymore -- the compiled middleware terminal
+already converts routing failures and handler-raised HTTPExceptions into
+real Responses before they get back here. So the exception this catch
+block actually swallowed was from a middleware's own code that runs AFTER
+a successful self.app(request) call (e.g. CSRFMiddleware setting a cookie
+on the response it got back). Catching that and calling self.app(request)
+again meant the downstream handler -- and any side effect it has -- ran a
+second time for one incoming request.
+"""
+
+import asyncio
+
+from velocix import TestClient, Velocix
+from velocix.security.base import SecurityMiddleware
+
+
+class _BuggyPostProcessSecurity(SecurityMiddleware):
+    """Calls through to self.app like every real subclass, then does its
+    own post-processing on the response -- which has a bug."""
+
+    async def _on_request(self, request):
+        _response = await self.app(request)
+        raise RuntimeError("bug in this middleware's own post-processing")
+
+
+def _run(coro):
+    return asyncio.run(coro)
+
+
+def test_downstream_handler_not_invoked_twice_on_post_processing_bug():
+    call_count = 0
+
+    app = Velocix()
+
+    @app.get("/side-effect")
+    async def side_effect():
+        nonlocal call_count
+        call_count += 1
+        return {"ok": True}
+
+    app.add_middleware(_BuggyPostProcessSecurity)
+
+    async def scenario():
+        async with TestClient(app) as client:
+            resp = await client.get("/side-effect")
+            assert resp.status_code == 500
+
+    _run(scenario())
+
+    assert call_count == 1

velocix/security/base.py

@@ -209,11 +209,24 @@ def emit(self, event_type: str, detail: str = "", **kwargs: Any) -> SecurityEven
         return event
 
     async def __call__(self, request: Request) -> Response:
-        try:
-            return await self._on_request(request)
-        except Exception:
-            # Security middleware must never crash the request pipeline.
-            return await self.app(request)
+        # No try/except here on purpose: _on_request implementations call
+        # self.app(request) themselves partway through their own checks, and
+        # a bare `except Exception: return await self.app(request)` around
+        # that can't tell "our own check logic raised" from "the call to
+        # self.app(request) we already made raised" -- the latter meant a
+        # single incoming request invoked the downstream handler chain
+        # TWICE, risking real double side effects (e.g. a double DB write)
+        # for any handler that raises after doing one.
+        #
+        # This used to be needed because an exception here would otherwise
+        # propagate past every middleware, bypassing all of it. Velocix's
+        # compiled middleware terminal now catches every exception (routing
+        # failures and handler-raised HTTPExceptions alike) and converts it
+        # to a Response before it ever reaches here, so self.app(request)
+        # returning normally is already the common case; whatever's left
+        # (a bug in this middleware's own pre-dispatch logic) is exactly
+        # what should surface as a real error, not be silently retried.
+        return await self._on_request(request)
 
     async def _on_request(self, request: Request) -> Response:
         """Override this to implement security logic.