starlette
starlette copied to clipboard
encode
AssertionError with middleware and TemplateResponse
diff --git i/tests/test_templates.py w/tests/test_templates.py
index b48373e..57dee91 100644
--- i/tests/test_templates.py
+++ w/tests/test_templates.py
@@ -20,6 +20,11 @@ def test_templates(tmpdir):
async def homepage(request):
return templates.TemplateResponse("index.html", {"request": request})
+ @app.middleware("http")
+ async def noop(request, call_next):
+ response = await call_next(request)
+ return response
+
client = TestClient(app)
response = client.get("/")
assert response.text == "<html>Hello, <a href='http://testserver/'>world</a></html>"
causes:
tests/test_templates.py:29: in test_templates
response = client.get("/")
.venv/lib/python3.7/site-packages/requests/sessions.py:546: in get
return self.request('GET', url, **kwargs)
starlette/testclient.py:421: in request
json=json,
.venv/lib/python3.7/site-packages/requests/sessions.py:533: in request
resp = self.send(prep, **send_kwargs)
.venv/lib/python3.7/site-packages/requests/sessions.py:646: in send
r = adapter.send(request, **kwargs)
starlette/testclient.py:238: in send
raise exc from None
starlette/testclient.py:235: in send
loop.run_until_complete(self.app(scope, receive, send))
/usr/lib/python3.7/asyncio/base_events.py:584: in run_until_complete
return future.result()
starlette/applications.py:134: in __call__
await self.error_middleware(scope, receive, send)
starlette/middleware/errors.py:122: in __call__
raise exc from None
starlette/middleware/errors.py:100: in __call__
await self.app(scope, receive, _send)
starlette/middleware/base.py:25: in __call__
response = await self.dispatch_func(request, self.call_next)
tests/test_templates.py:21: in noop
response = await call_next(request)
starlette/middleware/base.py:47: in call_next
assert message["type"] == "http.response.start"
E AssertionError
message is {'type': 'http.response.template', 'template': <Template 'index.html'>, 'context': {'request': <starlette.requests.Request object at 0x7ff4a2d21e48>}} there.
This appears to be an issue with the test client only though. Do I understand it correctly that the "http.response.template" extension is only used with tests?
btw: I think in general more verbose asserts should be used, e.g. in this case:
assert message["type"] == "http.response.start", message
or
assert message["type"] == "http.response.start", message["type"]
Do I understand it correctly that the "http.response.template" extension is only used with tests?
If so, its type could maybe have a "test." prefix?
I came up with a fix in https://github.com/encode/starlette/pull/473, but maybe this should be handled by the test client instead somehow?
It might also make sense to store this in request.state instead?
But it shows that currently the middleware fails in case of unexpected messages being send - but maybe this is a protocol violation with "http.response.template" already, and was only meant as a hack to get it to the test client.
Ah interesting one.
Yes it's generally only used in order to provide extra information back to the test client (tho it could actually also be useful info to middleware too).
We don't really want to pack it into the state because it's possible that some middleware might return a different response instead, in which case we'd erronously be reporting that we got a template response.
I'm not quite sure what the best way of dealing with this is. We might want to always send the template message as the first message. In that case the BaseHTTPMiddleware could always intercept that first, and we could have a StreamingTemplateResponse or something.
We might want to always send the template message as the first message.
This would basically mean to move it out of body_stream (in https://github.com/encode/starlette/pull/473/files#diff-ea43df86acedfeeae9fcba482f124a2cR49).
In that case the
BaseHTTPMiddlewarecould always intercept that first, and we could have aStreamingTemplateResponseor something.
Why a separate response type?
has there been any update on this issue @tomchristie ? In its current state it's effectively erroring out on unit-tests on resources that return templated responses if a middleware is present.
I see the workaround used in https://github.com/atviriduomenys/spinta/pull/22 but I'd rather not override such an important piece
I currently have a hacky workaround for this.
Subclass Jinja2Templates and modify it to not send the context, this means you can't test against the template context, but at least doesn't fail. Does anyone have a better workaround than this (e.g. doesn't involve changing anything in the application code, still allows tests with context)?
from starlette.responses import Response
from starlette.templating import Jinja2Templates as _Jinja2Templates, _TemplateResponse
class TestableJinja2Templates(_Jinja2Templates):
def TemplateResponse(
self,
name: str,
context: dict,
status_code: int = 200,
headers: dict = None,
media_type: str = None,
background=None,
) -> _TemplateResponse:
if "request" not in context:
raise ValueError('context must include a "request" key')
template = self.get_template(name)
return CustomTemplateResponse(
template,
context,
status_code=status_code,
headers=headers,
media_type=media_type,
background=background,
)
class CustomTemplateResponse(_TemplateResponse):
async def __call__(self, scope, receive, send) -> None:
# context sending removed
await Response.__call__(self, scope, receive, send)
I'm having problems with this as well on a project that's entirely HTML forms. I'm fine with subclassing JinjaTemplates because I don't mind not having the template context in the test client, but I would like to see a better solution as well.
Any update on this? I am getting the above error.
I found a solution that is working in the FastAPI repo issue list. Hope that helps someone else trying to solve this. https://github.com/tiangolo/fastapi/issues/806
Here's my hacky workaround to this for now in case it helps anyone:
@pytest.fixture
def client():
return TestClient(app)
# TODO: I've had to create an app without middleware for testing due to the following bug
# https://github.com/encode/starlette/issues/472
@pytest.fixture
def client_without_middleware(request):
def fin():
app.user_middleware = user_middleware
app.middleware_stack = app.build_middleware_stack()
user_middleware = app.user_middleware.copy()
app.user_middleware = []
app.middleware_stack = app.build_middleware_stack()
request.addfinalizer(fin)
return TestClient(app)
I then inject client_without_middleware into tests which render Jinja2 templates.
A slightly neater solution, which can be injected into any test which already has the client injected:
@pytest.fixture
def exclude_middleware():
user_middleware = app.user_middleware.copy()
app.user_middleware = []
app.middleware_stack = app.build_middleware_stack()
yield
app.user_middleware = user_middleware
app.middleware_stack = app.build_middleware_stack()
Keeping my fingers crossed that this problem gets fixed soon 😄
Huge thanks Fotis
Are there any updates on this? It has been 3 years since this issue has been raised and the only solution to this date is to test apps without middleware. Surely this should be possible?
It's upsetting that a bug like this is still happening 3 years later. Makes it seem like Starlette is not ready for prime time yet.
But if you're searching for how to work around this issue then please do not follow recommendations to run tests by doing things like disabling middleware or not sending context to your templates. That's a sure way to miss something.
The better approach, IMO, is simply to use a standard HTML response (you could wrap this all up in a helper function if you like). So everywhere you have this:
return templates.TemplateResponse("index.html", {"request": request})
Replace it with:
template = templates.get_template("index.html")
html = template.render({"request": request})
return HTMLResponse(html)
I'm not sure if performance is impacted by doing this, but it's probably negligible for small templates/contexts and at least you're covering all your bases.
It's upsetting that a bug like this is still happening 3 years later. Makes it seem like Starlette is not ready for prime time yet.
All volunteers here. If you are unsatisfied, you are free to investigate the issue, propose a fix, implement, explain why that's a good fix, and why the proposed alternatives over the years were not good.
That said... Since I really didn't like the previous message, I spent some time this morning to investigate the issue... :shrug:
Let's have a clear view of the problem here, and then I'm going to suggest some possible fixes.
Problem
The problem is that the TestClient supports an undocumented ASGI extension called http.response.template, and the application, specifically the TemplateResponse, when sees that the ASGI server (in this case the TestClient) supports it will send the http.response.template message as the first message between the application and the server, instead of sending the http.response.start. Just for context, this extension has the goal to give more information to the user that is testing the application.
First solution
The simplest thing to do would be to just remove the extension.
Second solution
The second solution would be to modify the BaseHTTPMiddleware, as implemented on https://github.com/encode/starlette/pull/1071/. Some changes are needed over there, because it's a bit old, but it's feasible. The problem is that we don't solve the issue globally, as every middleware around that wants to support this will need to add the same analogous logic.
As a note, we have discussions about the deprecation of the BaseHTTPMiddleware, but I'll not say it's a blocker.
What would be interesting to have a "go ahead" on the second solution will be to make that extension official, that way we are not forcing middlewares around to comply with what we say, but comply with the standard. There's a discussion about it on asgiref.
Conclusion
There seem to not be a clear way to solve this issue. It would be great if we could move the asgiref issue forward, but it doesn't seem something that got much attention over the years.
Since I don't have a good solution at the moment, my recommendation right now would be to not use the TestClient. Instead, use the httpx.AsyncClient for the TemplateResponse endpoints, and not benefit from the ASGI extension that the TestClient supports.
EDIT: For future reference, this is the code I've used to reproduce the issue:
from starlette.applications import Starlette
from starlette.middleware import Middleware
from starlette.middleware.base import BaseHTTPMiddleware
from starlette.routing import Mount, Route
from starlette.staticfiles import StaticFiles
from starlette.templating import Jinja2Templates
from starlette.testclient import TestClient
templates = Jinja2Templates(directory="templates")
async def homepage(request):
return templates.TemplateResponse("index.html", {"request": request})
routes = [
Route("/", endpoint=homepage),
Mount("/static", StaticFiles(directory="static"), name="static"),
]
class CustomMiddleware(BaseHTTPMiddleware):
async def dispatch(self, request, call_next):
response = await call_next(request)
return response
app = Starlette(debug=True, routes=routes, middleware=[Middleware(CustomMiddleware)])
def test_homepage():
client = TestClient(app)
response = client.get("/")
assert response.status_code == 200
assert response.template.name == "index.html"
assert "request" in response.context
This snippet was extracted from https://www.starlette.io/templates/.
@Kludex does this mean the problem is solved? Sorry I don't know much about asgiref, just trying to get tests to work with a FastAPI application that uses middleware and template responses.
Thank you for your quick reply. Sorry for asking again - how exactly do I solve the problem on my end now? I'm still getting this error
Edit I can see i don't have the most current version, nevermind
Locking the issue to avoid spam.
The problem was solved. If you have questions, or you think you've found a bug, feel free to open a discussion.