Fastapi: [BUG] Async class method dependency raises a ValueError

Created on 4 Nov 2019 · 6Comments · Source: tiangolo/fastapi

Describe the bug
If you use an async class method as a dependency, a ValueError is thrown. It doesn't happen for a non-async method.

Complete error: ValueError: [KeyError(<class 'coroutine'>), TypeError("'coroutine' object is not iterable"), TypeError('vars() argument must have __dict__ attribute')] (at fastapi/encoders.py:106)

To Reproduce

from fastapi import Depends, FastAPI
from starlette.requests import Request

class DependencyClass:
    async def async_dep(self, request: Request):
        return True

    def sync_dep(self, request: Request):
        return True

app = FastAPI()
dependency = DependencyClass()

# Error
@app.get('/async-dep')
def async_dep(r=Depends(dependency.async_dep)):
    return r

# Everything is fine
@app.get('/sync-dep')
def sync_dep(r=Depends(dependency.sync_dep)):
    return r

Expected behavior
The async class method dependency should be called and its return value injected.

Environment:

OS: macOS
FastAPI Version: 0.42.0
- Python version: 3.7.2

Additional context
I believe the issue comes from here:

https://github.com/tiangolo/fastapi/blob/65536cbf63318d111bf608960378d651b6c1596a/fastapi/dependencies/utils.py#L353-L359

Indeed, inspect.isfunction(call) will return False in case of a class method. Hence, it is sent to run_in_threadpool, which never awaits the coroutine, and we end up trying to serialize it instead of its result (hence the ValueError).

Changing the check by:

if inspect.isfunction(call) or inspect.ismethod(call):

solves the issue. I can make a PR with the fix and unit tests if it helps.

bug

Source

frankie567

All 6 comments

not sure what the "right" behavior should be but in your async_dep (wich itself is not async) you return a coroutine r without awaiting it so it's kind of expected that it doesn't return it's value, is it ?

somehting like this "works" even if I fail to see the use case

import asyncio

import uvicorn
from fastapi import Depends, FastAPI
from starlette.requests import Request


class DependencyClass:
    async def async_dep(self, request: Request):
        await asyncio.sleep(0)
        return False

    def sync_dep(self, request: Request):
        return True


app = FastAPI()
dependency = DependencyClass()

# Error
@app.get('/async-dep')
async def authenticate(r=Depends(dependency.async_dep)):
    s = await r
    return s

# Everything is fine
@app.get('/sync-dep')
def authenticate(r=Depends(dependency.sync_dep)):
    return r


if __name__ == '__main__':
    uvicorn.run("679_async_dep_class:app", reload= True)

euri10 on 4 Nov 2019

Well yes, for the sake of simplicity in the example, my async method don't do anything async, but still is a coroutine that needs to be awaited.

The use case is inspired from the async class dependencies, like OAuth2PasswordBearer:

https://github.com/tiangolo/fastapi/blob/65536cbf63318d111bf608960378d651b6c1596a/fastapi/security/oauth2.py#L138-L163

__call__ is async here and, when you inject this dependency, we do get param, we don't need to await it in the controller function.

So, I think we should be able to do something similar with class methods. For example:

class Authentication:
    def __init__(self, params):
        self.params = params

    async def get_user(self, request: Request) -> User:
        return await self._authenticate(request)

    async def get_active_user(self, request: Request) -> User:
        user = await self._authenticate(request)
        if not user.is_active:
            raise HTTPException()
        return user

    async def _authenticate(self, request: Request) -> User:
        # Do some authentication logic
        return user

authentication = Authentication(params)

@app.get('/active-user')
def active_user(user=Depends(authentication.get_active_user)):
    return user

This is useful to share common logic between all the dependencies while providing some specialized behaviour.

IMO, this is clearly a bug as it works flawlessly with async __call__ and non-async methods.

frankie567 on 4 Nov 2019

makes lot of sense indeed, was stuck on your version before edit

euri10 on 4 Nov 2019

👍1

This makes sense to me; @frankie567 I think it's worth a PR.