vmagamedov / grpclib

Pure-Python gRPC implementation for asyncio
http://grpclib.readthedocs.io
BSD 3-Clause "New" or "Revised" License
936 stars 92 forks source link

Register error handler? #112

Open bpeake-illuscio opened 4 years ago

bpeake-illuscio commented 4 years ago

First of all, this package is amazing!

I'm trying to write middleware that catches an RPC error and examines it before re-raising to the caller.

I have defined a number of custom errors for my backend, and am using the Status package to send them over the wire from the server. The grpclib.GRPCError contains all of my custom info (awesome!) when it gets raised.

What I am trying to do is add a middleware / interceptor that can take that exception and transform it into my custom exceptions for a better user experience. I would prefer to only have to write this once and register it with the channel in such a way that I know it will trigger for all RPCs.

Is there currently a way to do this? I have tried creating event handlers for both grpclib.events.RecvMessage, and grpclib.events.RecvTrailingMetadata but neither seem to give access to the response status or resulting errors.

Sorry if I missed something in the documentation. If it doesn't exist, I could see this being a huge plus for a lot of people.

vmagamedov commented 4 years ago

I think that it is fairly simple to add a status and related info into the Recv/SendTrailingMetadata events, but I don't think that this is the best way to re-raise exceptions. There are also other places which can be a source of exceptions (timeout, cancellation, disconnect).

The best way is to have some way of wrapping a call – middleware/interceptor as you mentioned. And this is not a trivial functionality.

gRPC has four different method types (unary-unary ... stream-stream), and I haven't seen a beautiful interceptors design to consistently cover all four of them. That's why we have events.

Maybe I will add some sort of middleware to solve also another problem - resources management, ability to wrap a call with a try..finally block or a context manager:

async def middleware(..., request_metadata):
    async with some_resource():
        yield

Events are not suitable for this task.

bpeake-illuscio commented 4 years ago

I think middleware would be an excellent addition, especially since it's familiar to those coming from HTTP/1.1 frameworks.

That being said, I do thing there are SOME benefits to implementation as an interceptor: namely, those coming from gRPC on other languages may find the metaphor more in line with their existing gRPC ecpsystem.

The ecosystem I am working in has both Go and Python gRPC services, and I found Go's interceptor semantics fairly easy to work with for error catching / casting: https://medium.com/@shijuvar/writing-grpc-interceptors-in-go-bf3e7671fe48

Obviously there would be some differences with Python (ie, errors are handled via raise rather than returns), but it might be a decent reference point.

Overall I personally wouldn't have a strong inclination one way or the other for how it is implemented, but having some sort if mechanism for error middleware (and middleware in general) could be really useful.

bpeake-illuscio commented 4 years ago

Additional thoughts:

I do not mind having to implement middleware "multiple times" for different types of calls. In the initial comment I mentioned registering it "once", but what I'm really after is global registration for all current and future RPCs added to the connection, to lessen the likelihood that a route gets missed during future development, and reduce the amount of boilerplate that needs to be written whenever a new RPC or service is added.

Having to register middleware for multiple RPC types still accomplishes that.

aaliddell commented 4 years ago

Just stumbled across this. I've previously implemented a middleware supporting servicer on top of grpclib, which uses async context managers for the middleware and requires no special handling for the different call types.

Since writing async context managers can sometimes be a pain to do manually, you can instead use @contextlib.asynccontextmanager; this allows you to write a single async generator that will be converted to the proper context manager. Here's a demo middleware:

@contextlib.asynccontextmanager
async def some_middleware(servicer, method, stream, context):
    # Do pre-RPC setup (e.g. check authentication)
    ...

    # Run next middleware or the handler method
    try:
        yield

    except:
        # Handle RPC errors (e.g. convert or catch error)
        ...

    finally:
        # Do post-RPC teardown (e.g. log response)
        ...

Using this implementation, the middleware can also abort the call rather than yielding, such as if you're writing authentication middleware. Internally it correctly handles the middleware stack unwinding on error/RPC completion. This means it almost behaves as if you've written:

async with middleware_1(servicer, method_name, stream, context):
    async with middleware_2(servicer, method_name, stream, context):
        async method_handler(servicer, stream, context)

In its current state, the servicer is used as a wrapper around an existing base servicer class but also moves the method implementations out (personal preference, but not required for this to work) and provides a call context object that is passed between middleware (such as for storing user data loaded in auth middleware):

servicer = MiddlewareServicer(
    base=SomeServiceBase,
    method_handlers={
        'DoSomething': do_something_implementation,
        'DoSomethingElse': do_something_else_implementation,
    },
    middleware_handlers=[some_middleware, some_other_middleware]  # <- run in this order
)

If there's interest, I can see if I can tidy it up to a state for you to take a look at?

bpeake-illuscio commented 3 years ago

Just checking back on this. Middleware is something I am definitely still interested in.

vmagamedov commented 3 years ago

FYI: Since 0.4.2rc2 there are status-related properties in the SendTrailingMetadata and RecvTrailingMetadata events: https://grpclib.readthedocs.io/en/latest/events.html#grpclib.events.RecvTrailingMetadata