strawberry
strawberry copied to clipboard
Missing 1 required positional argument when accessing graphiql page.
In a nutshell
I found attribute error when accessing graphiql page. I use custom context class.
This is the original issue #96.
Describe the Bug
When accessing graphiql endpoint I always see errors in logs. Like this:
TypeError: get_redis_connection() missing 1 required positional argument: 'request'
File "fastapi/applications.py", line 261, in __call__
await super().__call__(scope, receive, send)
File "starlette/applications.py", line 112, in __call__
await self.middleware_stack(scope, receive, send)
File "starlette/middleware/errors.py", line 146, in __call__
await self.app(scope, receive, send)
File "starlette/middleware/base.py", line 22, in __call__
await self.app(scope, receive, send)
File "starlette/exceptions.py", line 58, in __call__
await self.app(scope, receive, send)
File "fastapi/middleware/asyncexitstack.py", line 21, in __call__
raise e
File "fastapi/middleware/asyncexitstack.py", line 18, in __call__
await self.app(scope, receive, send)
File "starlette/routing.py", line 656, in __call__
await route.handle(scope, receive, send)
File "starlette/routing.py", line 315, in handle
await self.app(scope, receive, send)
File "starlette/routing.py", line 77, in app
await func(session)
File "fastapi/routing.py", line 264, in app
solved_result = await solve_dependencies(
File "fastapi/dependencies/utils.py", line 498, in solve_dependencies
solved_result = await solve_dependencies(
File "fastapi/dependencies/utils.py", line 498, in solve_dependencies
solved_result = await solve_dependencies(
File "fastapi/dependencies/utils.py", line 498, in solve_dependencies
solved_result = await solve_dependencies(
File "fastapi/dependencies/utils.py", line 523, in solve_dependencies
solved = await solve_generator(
File "fastapi/dependencies/utils.py", line 443, in solve_generator
cm = asynccontextmanager(call)(**sub_values)
File "contextlib.py", line 296, in helper
return _AsyncGeneratorContextManager(func, args, kwds)
File "contextlib.py", line 87, in __init__
self.gen = func(*args, **kwds)
This error doesn't affect the application itself. It doesn't affect logic or request handling, but it's constantly appear in logs.
I use custom context like this:
from aio_pika import Channel
from aio_pika.pool import Pool
from aiokafka import AIOKafkaProducer
from fastapi import Depends
from redis.asyncio import Redis
from strawberry.fastapi import BaseContext
from testkaftemp.services.rabbit.dependencies import get_rmq_channel_pool
from testkaftemp.services.redis.dependency import get_redis_connection
class Context(BaseContext):
"""Global graphql context."""
def __init__(
self,
redis: Redis = Depends(get_redis_connection),
rabbit: Pool[Channel] = Depends(get_rmq_channel_pool),
) -> None:
self.redis = redis
self.rabbit = rabbit
def get_context(context: Context = Depends(Context)) -> Context:
"""
Get custom context.
:param context: graphql context.
:return: context
"""
return context
# In my router.py file
...
gql_router = GraphQLRouter(
schema,
graphiql=True,
context_getter=get_context,
)
This works for handlers, but it throws an error when accessing graphiql page.
System Information
- Operating system: linux
- Python version: 3.9
- Strawberry version: 0.114.7
- FastAPI-Template version: 3.3.7
Additional Context
You can reproduce this issue easily by generating a project with FastAPI-template with graphql api type and with at least one dependency, such as redis.
This seems to be related to websockets, I'll see if this is something we need to fix on our end 😊
@patrick91, thank you.
hi @s3rius, sorry for the late reply!
Looks like this is happening when doing websockets connections, which might not happen anymore in your case (we upgraded to GraphiQL 2 and it doesn't do a WS connection until needed).
But you'll still get this issue for subscriptions, since we don't pass the request in that case (we use ws). I wonder if we should change this and use request for both ws and request.
@Kludex what do you think?
Do you have a minimal reproducible example?
The code I attached to the pull request is an MRE.
@Kludex I reproduced this by using the fastapi template provided above and trying to do a subscription, I can see if I can make a more minimal reproduction in the next week :)
The code above is missing the code for get_redis_connection, which is where the exception happens (it is a function that accepts requests as a parameter)
@patrick91, there seems to be a similar issue as I just linked. I will look into how the GraphiQL interface passes request-level data to the FastAPI server and the context_getter implementation.
Here's an MRE that I'm using for anyone else to try:
import asyncio
import strawberry
from strawberry.fastapi import BaseContext, GraphQLRouter
from strawberry.types import Info
from fastapi import Depends, FastAPI, Request
from typing import AsyncGenerator
def get_request(request: Request) -> Request:
return request
class Context(BaseContext):
"""Global graphql context."""
request: Request
def __init__(
self,
request: Request = Depends(get_request),
) -> None:
self.request = request
def get_context(context: Context = Depends(Context)) -> Context:
"""
Get custom context.
:param context: graphql context.
:return: context
"""
return context
#-----------------------------------------------------------------------------
@strawberry.type
class Subscription:
@strawberry.subscription
async def count(self, info: Info, target: int = 100) -> AsyncGenerator[str, None]:
for i in range(target):
yield f"Counting {i}"
await asyncio.sleep(0.5)
@strawberry.type
class Query:
@strawberry.field
async def say_hi(self, info: Info, name: str) -> str:
return f"Hi {name}"
#-----------------------------------------------------------------------------
schema = strawberry.Schema(
query=Query,
subscription=Subscription,
)
graphql_api = GraphQLRouter(
schema,
graphiql=True,
context_getter=get_context,
)
app = FastAPI(title='GraphQL API')
app.include_router(graphql_api, prefix='/graphql')
So, @s3rius, it appears that this is in fact an issue with FastAPI itself when using sub-dependencies within a parent class dependency over a websocket connection: https://github.com/tiangolo/fastapi/issues/2031.
The OP in that issue has a problem using the in-built TestClient but subsequent commenters mention that they observe it in normal usage too. I can confirm that the problem goes away if you change get_request in the above MRE to:
def get_request(request: Request | None = None, websocket: WebSocket | None = None) -> Request:
return request or websocket
as mentioned here: https://github.com/tiangolo/fastapi/issues/2031#issuecomment-972105581. So it seems to be an issue with the action of Dependency resolution within FastAPI. Indeed, when inspecting the stack-trace for the error, the offending function is https://github.com/tiangolo/fastapi/blob/master/fastapi/dependencies/utils.py#L457.
My conclusion is that FastAPI resolves these dependencies in a non-agnostic way before any of Strawberry's context creation takes effect. Due to the way it has been implemented, it would seem that request and websocket are different objects at the point of FastAPI dependency resolution. However, these two are then merged together into a single request during the Strawberry context creation, hence your confusion!
So, @Kludex, do we mention this in the docs as a potential gotcha with class-based dependency contexts over websockets or do we find a way to refactor the Strawberry FastAPI integration to take into account this quirk of FastAPI's Dependency system?
we find a way to refactor the Strawberry FastAPI integration to take into account this quirk of FastAPI's Dependency system
This :+1:
Since there is a workaround for now, @s3rius, I will leave this issue hanging open and return to it with an attempted fix when I get the opportunity to in the next few weeks. ~~Hopefully progress might be made on the FastAPI issue in that time with Tiangolo regarding it as a bug to be fixed.~~
What is there to be fixed on that issue? :thinking: The HTTPBearer only receives a Request.
It was my takeaway from that issue, and this issue too, that the intended FastAPI behaviour would be to have request refer to both request and websocket, which is to say that the difference is abstracted away and any dependencies defined to work over HTTP will also work over WS without explicitly specifying it.
Having written this out, however, it's clearer to me now that FastAPI is behaving as intended and Strawberry isn't. FastAPI makes clear the difference between HTTP and WS by design while Strawberry's implementation muddles it by having Queries & Mutations work over HTTP and Subscriptions over WS while promising a layer of abstraction. Strawberry users expect the underlying protocol to be abstracted away while FastAPI users do not.
I agree then that Strawberry must be refactored to deal with this :grin:
@s3rius, I've done some more digging into this issue to find, with the help of @kristjanvalur's changes, that the FastAPI dependency resolution successfully resolves either the Request of Websocket type if you use the more general HTTPConnection type. This removes any of the hacky-ness surrounding the above implementation using nullable request and ws objects. So the working implementation is:
from fastapi.requests import HTTPConnection
def get_request(request: HTTPConnection) -> HTTPConnection:
return request
Using this method, you notify to FastAPI to populate your field with either object depending on the transport type in the same way that Strawberry eventually creates the context, by also populating request with either object: https://github.com/strawberry-graphql/strawberry/blob/main/strawberry/fastapi/router.py#L74. Depends then correctly injects the relevant object depending on the type annotation(s) provided.
@Kludex, I could not find a way of refactoring Strawberry to account for this and think that such refactoring would require either doing away with Depends or forking our own Depends and refactoring that. If you know of a potential way for refactoring this in a better way then please let me know! :grin:
Finally, this will require some more documentation for the FastAPI integration to detail this nuance so I'll create that soon!
@Kludex, I could not find a way of refactoring Strawberry to account for this and think that such refactoring would require either doing away with Depends or forking our own Depends and refactoring that. If you know of a potential way for refactoring this in a better way then please let me know!
I didn't understand what you want to do on Strawberry.
Having written this out, however, it's clearer to me now that FastAPI is behaving as intended and Strawberry isn't. FastAPI makes clear the difference between HTTP and WS by design while Strawberry's implementation muddles it by having Queries & Mutations work over HTTP and Subscriptions over WS while promising a layer of abstraction. Strawberry users expect the underlying protocol to be abstracted away while FastAPI users do not.
It needs repeating that Strawberry also allows queries and mutations over WS. It's the client's choice which to use and a client which already does subscriptions via WS, will probably elect to use websockets too for its queries and mutations. The resolvers in Strawberry are, however, supposed to be protocol unaware, yet the behaviour is subtly (and not so subtly) different, in particular when it comes to the scope of the get_context() FastAPI dependency.