vmagamedov / grpclib

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

RFC - Allow handler methods to return early by exceptions #106

Closed tjstum closed 4 years ago

tjstum commented 4 years ago

This is somewhere between a question and a feature request.

Let's say you have a server that has boilerplate at the beginning of each method. Maybe it's like:

class FooServer(FooBase):
    async def GetTheBar(self, stream):
        try:
            bar = ...  # some loookup function
        except NoSuchBar:
            await stream.send_message(status=NoBar)
            return

        await stream.send_message(status=OK, bar=bar.to_proto())

That works well enough, but if every such method has to start with those same 4 lines, it becomes a bit repetitive. It would be nice if that could be refactored. Perhaps something like:

class FooServer(FooBase):
    async def get_bar(self, bar_name, stream):
        try:
            return ...  # the lookup function
        except NoSuchBar:
            await stream.send_message(status=NoBar)
            raise grpclib.ResponseAlreadySent

    async def GetTheBar(self, stream):
        request = await stream.recv_message()
        bar = await self.get_bar(..., stream)
        await stream.send_message(status=OK, bar=bar.to_proto())

and then request_handler would discard the ResponseAlreadySent exception.

It is a little weirdly magical, but only a little (at least to my eyes). I thought about using GRPCError for this, but if an application carries its own statuses in the protocol definition, that might sort of hard to integrate (like some errors might be immediately fatal, but other errors could still fill in some of the information of the response)

I'm happy to contribute a patch for this if it's a reasonable approach, but if there are other ways to solve this, I'm also happy to hear that. Thanks!

vmagamedov commented 4 years ago

I'm not against such feature (function composition for request processing). But I don't want to use exceptions for this purpose.

As for errors, gRPC protocol already gives you ability to send rich errors details: https://grpclib.readthedocs.io/en/latest/errors.html#error-details - is this not enough?

tjstum commented 4 years ago

I'm curious by what you mean by "function composition for request processing." How do you early return through an arbitrary stack other than by unwinding it with exceptions?

The rich error details could probably solve my problem. It just seems to me that it might be annoying to define both regular return and error types (granted, I am relatively new to gRPC and probably don't have a good grasp of the idioms).

vmagamedov commented 4 years ago

I was thinking about something like Railway Oriented Programming or other Functional Programming techniques to compose functions into one processing pipeline: https://fsharpforfunandprofit.com/rop/

BTW, I'm a slowpoke 😅, with grpclib you actually can return early:

from grpclib import GRPCError, Status

async def get_bar(self, bar_name, stream):
    try:
        return ...  # the lookup function
    except NoSuchBar:
        await stream.send_message(Response(...))
        raise GRPCError(Status.OK)

async def GetTheBar(self, stream):
    request = await stream.recv_message()
    bar = await self.get_bar(..., stream)
    await stream.send_message(Response(...))
vmagamedov commented 4 years ago

But I strongly suggest you to use standard gRPC errors and not invent your own: grpclib.Status.NOT_FOUND

tjstum commented 4 years ago

Ha! I didn't think to raise with the Status.OK!

Point taken on not reinventing error codes, though. I will give that more thought