alexschimpf / fastapi-versionizer

FastAPI Versionizer
MIT License
81 stars 13 forks source link

Stack trace missing from logs when middleware is added after app is versioned with fastapi-0.88 #1

Closed mikeedjones closed 1 year ago

mikeedjones commented 1 year ago
from fastapi import FastAPI
import uvicorn
from fastapi import Request, Response
from fastapi_versionizer.versionizer import versionize
from starlette.middleware.base import (BaseHTTPMiddleware,
                                       RequestResponseEndpoint)

class TestMiddleware(BaseHTTPMiddleware):
    async def dispatch(self, request: Request, call_next: RequestResponseEndpoint) -> Response:
        response = await call_next(request)
        return response

app = FastAPI()

@app.get("/err")
async def root():
    raise Exception("test")

versionize(app, version_format='{major}',
           prefix_format='/v{major}')

app.add_middleware(TestMiddleware)

if __name__ == "__main__":
    uvicorn.run(app, host="0.0.0.0", port=8081)

calling /err does not result in a stack trace when using fastapi-0.88 and uvicorn-0.18.2. Is that behaviour expected?

Expected behaviour Logging should not be affected by versioning.

Weirdly,

from fastapi import FastAPI
import uvicorn
from fastapi import Request, Response
from fastapi_versionizer.versionizer import versionize
from starlette.middleware.base import (BaseHTTPMiddleware,
                                       RequestResponseEndpoint)

class TestMiddleware(BaseHTTPMiddleware):
    async def dispatch(self, request: Request, call_next: RequestResponseEndpoint) -> Response:
        response = await call_next(request)
        return response

app = FastAPI()

@app.get("/err")
async def root():
    raise Exception("test")

app.add_middleware(TestMiddleware)

versionize(app, version_format='{major}',
           prefix_format='/v{major}')

if __name__ == "__main__":
    uvicorn.run(app, host="0.0.0.0", port=8081)

with the middlware added before versionize is called on the app does produce a stack trace.

alexschimpf commented 1 year ago

Findings so far:

  1. fastapi==0.83.0 and uvicorn==0.16.0 does not have this problem
  2. With fastapi==0.88.0 and uvicorn==0.18.2, I can reproduce the issue with your first example, but I still get no stack trace when attempting your second example. What version of starlette are you using?
  3. This may be related: https://github.com/tiangolo/fastapi/issues/4531

It seems to work for me if I pass the middleware to versionize instead of adding it directly to the main app:

versionize(
    app,
    version_format='{major}',
    prefix_format='/v{major}',
    middleware=[Middleware(TestMiddleware)]
)

By doing this, it will automatically get applied to all versioned sub-applications.

Clearly there is some quirkiness with how FastAPI/Starlette is handling middleware and sub-applications. Since there are simple workarounds for this at the moment, it may be best to just add a note about this in the README.

mikeedjones commented 1 year ago

Thank you so much for looking at this and finding that issue.

starlette-0.22 is required by fastapi-0.88.

I retried my second example and it seems that I can't reproduce my stack traces from this morning.

I'll close this issue. Cheers again.