Add Custom Error Mapper Support

[jeffsawatzky]

This allows you to specify extra exception mapper functions to map other types of exceptions and not just the Grpc ones.

I also had some issues running things in python 3.10 so I had to update some dependencies. I also renamed some request and response vars to request_or_iterator and response_or_iterator to make it clearer what you get in the unary vs streaming scenerios.

[d5h]

Hi Jeff,

I want to understand the feature you’re trying to add here. The test case uses DATA_LOSS as an example, but there’s already an exception defined for that. Is it that you want to provide a custom string in the details?

If so, it seems like a simpler solution would be for you to subclass GrpcException in your code and set the details string there. Then in your code, raise those instead of the ones in the exceptions module.

[jeffsawatzky]

In hexagonal/ports&adapters architecture it isn't good to tie your domain/business logic to any host adapter library (flask, grpc, etc) and you should define domain specific errors that are raised. Then your host adapter would convert the domain error into either a specific error depending on the host (flask/werkzeug error or a grpc error).

I would like to be able to raise my domain specific errors and then have the ExceptionServiceInterceptor map my domain exceptions to a grpc one, and I would like to be able to do that by writing a mapping function like in this PR that would do that for me.

By having my domain exceptions subclass your GrpcException I am now tying my domain to grpc which isn't good.

[jeffsawatzky]

Also, we have a bunch of flask apps which we are trying to convert over to grpc, but they didn't follow hexagonal/ports&adapters pattern and raise werkzeug errors everywhere. Instead of going through all of the code and creating new exceptions for everything and raising them, I would like to create a mapper that would map werkzeug errors to grpc ones too. Eventually we will fix this up, but for now while we are doing a proof of concept I don't want to be messing around too much, and just be able to map werkzeug errors to grpc ones. Not sure if that makes sense.

[d5h]

Ok, I get it now. Thanks for the explanation. That makes sense. This seems worth doing, although I'd like to solve this in a slightly different way. The way I'm thinking of should solve your need to handle domain errors, make it easier to support your request in https://github.com/d5h-foss/grpc-interceptor/issues/14, and also just be more flexible to allow people to handle other cases that might come up.

What I'm thinking is to add a handle_exception method to ExceptionToStatusInterceptor, and move the current logic to handle GrpcException into it. The idea is that if people want to handle other exception types, they can subclass ExceptionToStatusInterceptor and override handle_exception. So basically, we would modify the current code to look like this:

    @contextmanager
    def _handle_exceptions(self, context):
        try:
            yield
        except Exception as ex:
            self.handle_exception(ex, context)
            raise

    def handle_exception(self, ex: Exception, context: grpc.ServicerContext) -> None:
        """Override this if extending ExceptionToStatusInterceptor.

        E.g., you can catch and handle exceptions that don't derive from GrpcException.
        Or you can set rich error statuses with context.abort_with_status(...).
        """
        if isinstance(ex, GrpcException):
            context.set_code(ex.status_code)
            context.set_details(ex.details)
        else:
            if self._status_on_unknown_exception is not None:
                context.set_code(self._status_on_unknown_exception)
                context.set_details(repr(ex))

This has a few benefits:

• It's flexible enough for people to easily extend to other cases (e.g., returning rich statuses, sending exception reports to Sentry or Datadog, etc.)
• It provides a reasonable default while abstracting away the messiness of dealing with streaming RPC
• It's backwards compatible (of course the approach in this PR is as well)
• No need for more args on the constructor

Would this work for you? If so, if you'd like to take a stab at it, that would be great. Otherwise it's simple enough I could probably do it when I have some free time this weekend.

BTW, the above example is maybe a little oversimplified. We should probably add method_name and maybe request to the handle_exception parameters, as those could be useful for some use cases like logging or sending extra info in rich statuses.

[jeffsawatzky]

@d5h I made the changes you suggested.

[d5h]

Thanks Jeff! Much appreciated! 🙌