Improve typing of custom req/resp type

[vytas7]

Discussed in #2370

^{Originally posted by jkmnt October 14, 2024}
Testing f4 beta with the our falcon-powered app. No runtime issues so far, but a few errors from the typechecker.

The app.add_error_handler and app.add_error_handler expects the callables with signature of [falcon.Request, falcon.Response, ...etc]. Our app is using the custom Request class:

falcon.App(request_type=MyRequest)

def my_error_handler(req: MyRequest, ...): ...

Maybe the request/response types should be typed as generics bound to the base type ?
Something like

TReq = TypeVar('TReq', bound=Request)

class App(Generic[TReq, TResp]):
    def __init__( self,  request_type: Optional[type[TReq]] = None, ...): ...

    def add_error_handler(self, exception, handler: Callable[[TReq, TResp, TCtx, ...], None]): ...

The same issue is with before/after hook adders . Yes, it's way harder to type since they know nothing about the app class.

[jkmnt]

Hi. My POC on the issue here: https://github.com/jkmnt/falcon/tree/generics_poc
Diff: jkmnt@a868ff5

I typed wsgi App class with generics. The concrete types are seems to be correctly inferred.
The only problem was with the standard App: Request/Response are Unknown in this case and it's bad!
I solved it by introducing the SimpleApp type alias declaring standard App[Request, Response].
(It should be done the opposite way of course: App and CustomApp)

I agree it's should be deferred for 4.1+. Too much changes for 4.0 )

BTW, there was some (missed by mypy) errors reported by pyright. I think I'll try to fix some and submit a separate PR.

[CaselIT]

BTW, there was some (missed by mypy) errors reported by pyright. I think I'll try to fix some and submit a separate PR.

that would be great. Feel free to also open an issue when you do that.
also if you know tox and github action we could add a gate with it too, so that it stays tested

[vytas7]

I have just created an issue to track the Pyright gate: #2373.

And echoing @CaselIT, re PR, your help is really appreciated, because we are no huge experts of typing (OK, @CaselIT maybe is, but I am definitely not 😅).

Edit: and as to typing extensions, I don't want Falcon to depend on any 3rd party runtime packages for the majority of use cases. We can add it as a conditional requirement for Python versions older than 3.X where they total less than Y% of PyPI downloads. Y being, say, 30% or so.

[jkmnt]

Another thing related to the typing:
I noticed the middleware type is just the object. It's the user-facing interface so imho typing should be better.

I suggest something like

class MwWithReqHandler(Protocol):
    def process_request(self, req: Request, resp: Response) -> None: ...

class MwWithRespHandler(Protocol):
    def process_response(self, req: Request, resp: Response, resource: object, req_succeeded: bool) -> None: ...

class MwWithProcessHandler(Protocol):
    def process_resource(self, req: Request, resp: Response, resource: object, params: dict[str, Any]) -> None: ...

Middleware = Union[MwWithReqHandler, MwWithRespHandler, MwWithProcessHandler]

This is not perfect but better than nothing. The bad thing is that it's enough to have one conforming (with others nonconformin) methods to pass the type check. The good thing: the user may inherit it's middleware class from these protocols (kind of abc but w/o runtime overhead) and get full typesafety.

class MyMiddleware(MwWithReqHandler, MwWithRespHandler):
    def process_req(...)
    def process_resp(...)

[CaselIT]

I thought about it while doing that part, but if we type it as Middleware = Union[MwWithReqHandler, MwWithRespHandler, MwWithProcessHandler] won't that require that all 3 method are provided?

If not then that's for sure an option!

[jkmnt]

No afaik its enough to satisfy just one protocol of the union.

[CaselIT]

Seems like a good option then!

[CaselIT]

it probably makes sense to open a new issue with it. If you can work on a PR it would be great!

[vytas7]

Yes, let's divide and conquer these typing improvements. Like improvement of the middleware protocols could be one issue/PR 🙂

[jkmnt]

it probably makes sense to open a new issue with it. If you can work on a PR it would be great!

Ok, I'll draft the PR then.

[davetapley]

I managed to type this all up at #2397 before looking for this, but looks like I came to the same conclusions 🙈

Thanks!