AssertionError with middleware and TemplateResponse #472
Comments
|
btw: I think in general more verbose asserts should be used, e.g. in this case:
or
|
If so, its type could maybe have a "test." prefix? I came up with a fix in #473, but maybe this should be handled by the test client instead somehow? |
|
It might also make sense to store this in 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 |
This would basically mean to move it out of
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 atviriduomenys/spinta#22 but I'd rather not override such an important piece |
|
I currently have a hacky workaround for this. Subclass 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. tiangolo/fastapi#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 |
|
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 |
After a lot of debugging, I dug out, that streaming wasn't working because of BaseHTTPMiddleware, which it seems collects all the stream into memory and then returns it all at once with the response. That means, if you stream a lot of data, request will not give any answer until all data is collected into memory. This can take time and can result in read timeout or can simply run out of memory. Streaming was the main reason, why I chose Starlette, and one and most important thing didn't worked. Not good. But at least I found how to fix it. Related issues: encode/starlette#1012 encode/starlette#472
|
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. |
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... 🤷 Let's have a clear view of the problem here, and then I'm going to suggest some possible fixes. ProblemThe problem is that the First solutionThe simplest thing to do would be to just remove the extension. Second solutionThe second solution would be to modify the As a note, we have discussions about the deprecation of the 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. ConclusionThere seem to not be a clear way to solve this issue. It would be great if we could move the Since I don't have a good solution at the moment, my recommendation right now would be to not use the 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.contextThis snippet was extracted from https://www.starlette.io/templates/. |
Kludex
added
bug
|
The Debug extension was merged on |
|
@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. |
|
Yeah, that's why the issue was closed. |
|
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. |
causes:
messageis{'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?
The text was updated successfully, but these errors were encountered: