f4 typing issues

[jkmnt]

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.

[CaselIT]

Hi,

Thanks for testing the typing!
This seems unfortunate, but I'm not sure if python typing does support this correctly, since I don't think making app generic quite solves the issue where, since doing so it would prevent adding handlers that are typed to accept a plan Request (since the app would require MyRequest).

Overall we are likely looking for a way of making handler covariant (I think) on request/response, accepting request or any subclass, same for response. It seems we can use type vars for this. I will try looking how it would become

[CaselIT]

since I don't think making app generic quite solves the issue where, since doing so it would prevent adding handlers that are typed to accept a plan Request (since the app would require MyRequest).

I've thought a bit more about it, and I'm not sure about this one, since that case may actually work ok.
It may actually be an option for the App, but I don't like too much the requirement of the two typevars, since until 3.13 you could not give default to them.

Since currently falcon does not depend on typing_extension it would not be possible to provide a default, but I guess it may be an option if typing_extension becomes a dependency

[CaselIT]

indeed just tried it. this seems to pass:

import typing

class Context:
    pass

class ImprovedContext(Context):
    server_id: str

Callback = typing.Callable[[str, ImprovedContext], None]

class Caller:
    def __init__(self) -> None:
        self._callbacks: list[Callback] = []

    def set_callback(self, callback: Callback) -> None:
        self._callbacks.append(callback)

    def call_callback(self) -> None:
        for callback in self._callbacks:
            callback(type(self).__name__, ImprovedContext())

def typed_base(name: str, context: Context) -> None: ...

def typed_derived(name: str, context: ImprovedContext) -> None: ...

if __name__ == "__main__":
    caller = Caller()
    caller.set_callback(typed_base)
    caller.set_callback(typed_derived)
    caller.call_callback()

[jkmnt]

Thanks for implementing the typing!
It's already helped us to find a couple of subtle bugs, e.g. default passed instead of required for get_param_as.. (on a side note, I think these must be keyword-only).

I'll try to explore this approach with typevar bound to Request and generic. IMHO the 99% of falcon typing would require no changes since MyRequest conforms to the Request interface expected by falcon core. Only the stored callbacks should be typed differently. The most important thing here is the context defined inside the Request subclass. In our case Context is quite rich (auth, session info etc) and fully typed - it was the main reason for using the custom request at all.

[CaselIT]

Thanks!

I've tried something here CaselIT#1 but it's currently not working when using a protocol instead of a bare callable. If we find a way of making it work it should more or less do what we want here. If you want to play with it feel free

By not working I mean mypy complains with "Invariant type variable "..." used in protocol where contravariant one is expected "

This is a test case I'm trying https://github.com/CaselIT/falcon/pull/1/files#diff-0741e966c9c4658e96f9787d213d3ec58673c3914199d3d2ea4e0a5eeaed3531

on a side note, I think these must be keyword-only

makes sente. We have a decorator to complain here, maybe we should apply it here, cc @vytas7

The most important thing here is the context defined inside the Request subclass

Currently Request/Response are not typed, but if we decide to include typing-extension as dependency (to allow for defaults in the type vars added in 3.13) we could decide to to make it generic on media and/or context.

[jkmnt]

Ok, will play with tomorrow.

For typed Context, we just (re)declared context type at class level w/o generics etc. Pyright is completely ok with it, vscode is offering completion too.

class MyRequest(falcon.Request):
    context_type = MyContext
    context: MyContext

[vytas7]

Thanks for reporting this @jkmnt!
Just a couple of comments adding to what already has been written by @CaselIT.

I will add a note for the release candidate, since we cannot really find any elegant solution or workaround for the time being.

One can type req and resp there as Any, and that works, but that loses essentially almost all typing of add_error_handler() for the majority of users that use the standard request and response types... 🤔

Alternatively, you could do something along the below lines...

def my_error_handler(req: falcon.Request, ...):
    my_req = cast(MyRequest, req)

    # Alternatively, cast only the context
    context = cast(MyContext, req.context)

    # Handle...
    # <...>

But this ain't pretty, and it requires synthetic effort on your side. Moreover, it can be plain wrong if you manage to get another type of request (you shoudn't if you use a single app though).

I have otherwise created an issue [from this discussion] to investigate this more in depth: #2372, but as said, Falcon 4.0.0 will have to go out as-is wrt this aspect. 👿