rollbar/pyrollbar

Rollbar is not handling errors with FastAPI

narmatov-asylbek opened this issue · 4 comments

Python 3.8
Rollbar 0.16.2

I am trying to integrate Rollbar with FastAPI using the recommended way(via FastAPI router). But I have some trouble with it.

Rollbar not sending errors with nested routers.

import fastapi
import rollbar
from rollbar.contrib.fastapi import add_to as rollbar_add_to

# Works fine
app = fastapi.FastAPI(...)
rollbar_add_to(app)

@app.get("/error")
async def some_error():
    raise UndefinedException()


# Not sending any errors
app = fastapi.FastAPI(...)
rollbar_add_to(app)

router = fastapi.APIRouter(...)
app.include_router(router)

@router.get("/error")
async def some_error():
    raise UndefinedException()

The problem is Rollbar not sending any errors with of router.

In our project, we have more than 20 routers. And adding to every single of them rollbar_add_to(router) can be problematic. Is there any way to fix this?

PS: Please do not suggest integrating via Middleware. I need to integrate Rollbar this way

Hi @narmatov-asylbek, thanks for opening this issue.

After looking at it the router/app method will not work on a router unless explicitly added. The reason for this is because the Rollbar router integration method simple replaces the APIRouter.route_class (default APIRoute) with the RollbarLoggingRoute class. At first glance it looked like FastAPI should support this with nested routers. However, when a new router is being added with the include_router() method, A route_class must be specified or it will default to APIRoute.

This means you would need to add...

router = APIRouter(...)
rollbar_add_to(router)

However, as you mentioned this is a bit problematic. It is also worth mentioning that the above is the same as...

from rollbar.contrib.fastapi.routing import RollbarLoggingRoute

router = APIRouter(...)
router.route_class = RollbarLoggingRoute

This means that if you needed any advanced functionality from your route class and created a custom route class, you would not get the expected outcome.

from fastapi.routing import APIRoute

class CustomRoute(APIRoute):
     ...
     # advanced route class stuff

router = APIRouter(
    # router stuff
    route_class=CustomRoute,
)
rollbar_add_to(router)
# the route_class is now RollbarLoggingRoute not CustomRoute

Here are some possible solutions...

  1. Create a class that extends APIRouter and uses RollbarLoggingRoute as the default route_class. Then always extend RollbarLoggingRoute not APIRoute when creating custom route classses.
  2. Create a function that returns a new APIRouter router instance, but does not accept a route_class argument. The route_class is always RollbarLoggingRoute, unless you create a custom class that extends RollbarLoggingRoute, and is set via assigning the route_class attribute on the new APIRouter instance.
  3. Call rollbar_add_to(router) after you create every router, and hope it never breaks anything.
  4. Fork FastAPI and make APIRoute include the logic from RollbarLoggingRoute. This is a terrible idea. Don't do this.

These options range from generally unadvisable, to terrible. I would use the middleware option. Things, like Rollbar, are why middleware exists. However, because you said you need to integrate via a router you will likely need to chose one of the above options.

@danielmorell thank you for the thoughtful response.

@danielmorell thank you for your response. It will definitely help

@danielmorell the in-depth explanation of how the Rollbar middleware instruments the FastAPI router is very illuminating, as is the comparison of ways to get this working for @narmatov-asylbek's use case. Cheers!