pelme / htpy

Generate HTML in Python
http://htpy.dev
MIT License
261 stars 11 forks source link

direct return of Element does not work in FastAPI route handler with default_response_class set #54

Closed blast25 closed 2 months ago

blast25 commented 2 months ago

https://htpy.dev/starlette/

Usage with FastAPI:

app=FastAPI()

test_route= APIRouter(
    prefix="/test",
    tags=["test"],
    default_response_class=HTMLResponse,
)

@test_route.get("/")
async def test():
    return div["test"]

app.include_router(test_route)

if __name__ == "__main__":
    uvicorn.run("test:app", host="0.0.0.0", port=9001, reload=True)

Calling localhost:9001/test/ returns error

Does not work for me as the internal FastApi Encoding does not know what to do with it (expects pydantic model). Using HTMLResponse as return works. But using a response from default response class fails for me, this would be necassary as to not delete changes in middleware or dependencies for routes that manipulate eg. the response header. Initializing HTMLReponse yourself would entail setting everything again from previous functions, every time. Workaround is using return str(div["test"]).

ggranger commented 2 months ago

Hello, I don't know much about FastAPI being mostly a Flask user, but casting your htpy elemnt to str() is the way to do it. Another option would be to subclass the Response class for it to accept htpy nodes. Probably you can have a look here: Custom Response Class

blast25 commented 2 months ago

Thank you for your input. I tried rewriting a custom response as default, until i found out that before fastapi evaluates the render function on the response class it has an internal encoding mechanism that executes before the render function of the response class, when used as default. Error shows up before ever hitting the response class.

pelme commented 2 months ago

So I guess it is not possible to just return htpy elements to fastapi? @blast25 can you point to the fastapi source code where it does the internal encoding? Maybe it is possible to add some method to the htpy elements to have it recognized directly by fastapi.

If not, we could document the need to call str() when using default_response_class.


Related to this is PR #38 that would allow using Starlettes StreamingResponse directly. I think that would be the preferred way of using Starlette/FastAPI with htpy. Combined with an async ORM, that will make the entire stack fully async and improve the speed of the page loads for the end user. I think it make

Here are the docs for how to use Starlette + async streaming response:

blast25 commented 2 months ago

https://github.com/fastapi/fastapi/blob/master/fastapi/routing.py line 143 async def serialize_response(...) calling jsonable_encoder(response_content) defined at https://github.com/fastapi/fastapi/blob/master/fastapi/encoders.py line 102 def jsonable_encoder(...) nothing for the evaluation of this method hits, failing at line 324 dict(obj) I hope this helps. I tested for StreamingResponse as default, this didnt throw an error like the HTMLResponse but the body of the Response was empty as again jsonable_encoder(...) failed.

pelme commented 2 months ago

Thanks for the pointer!

Reading the FastAPI docs, it says that when returning "any data", it will be passed to jsonable_encoder (exactly the same function/lines you point to in the source): https://fastapi.tiangolo.com/advanced/response-directly/

So that is just the way FastAPI works by design. There does not seem to be any way around that.

I do not think this contradicts what the docs says about Starlette: You can still return HTMLResponse(a htpy element) with FastAPI.

Do you have any suggestions for any changes to htpy or the htpy docs in this regards?

blast25 commented 2 months ago

After revising the problem not really, may have been nice to not call str on every object but thats a luxury problem i suppose. Thanks for considering.

pelme commented 2 months ago

Closing it for now then, if anyone comes up with something else, feel free to open a new issue!