Python: returning an empty typed response during error handling

[Amit Saha]

Hey all,

I am writing a experimental logging server side interceptor (based of https://github.com/grpc/grpc/pull/12778)  which will log any unhandled exceptions and set an appropriate code and details as follows:

def log_errors(func):
    from functools import wraps

    @wraps(func)
    def wrapper(*args, **kw):
        metadata = {}
        metadata['timestamp'] = datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%S.%fZ')
        if isinstance(args[4], grpc._server._Context):
            servicer_context = args[4]
            metadata.update(dict(servicer_context.invocation_metadata()))
        try:
            result = func(*args, **kw)
        except Exception as e:
            logger.error(e, exc_info=True, extra=metadata)
            servicer_context.set_details(str(e))
            servicer_context.set_code(grpc.StatusCode.UNKNOWN)
            # TODO: need to return an appropriate response type here
            # Currently this will raise a serialization error on the server
            # side
            return None
        else:
            return result
    return wrapper

The problem here is that i get a serialisation error since I return None if there is an exception. As per https://github.com/grpc/grpc/issues/4417#issuecomment-242998794, basically I am likely seeing the manifestation of "Even after setting a non-OK status code, service-side code must still return a value of the appropriate type (perhaps we should soften this to "may", but there's one edge-case use case that might get broken by that, so I'll have to think about it)."

Is there a way to achieve returning an empty response without serialisation error? Thanks for any insights.

Best Wishes,

Amit.

[Mehrdad Afshari]

+nath...@google.com

We were just discussing this with Nathaniel and the primary reason behind that API contract constraint, in addition to future-proofing by being conservative, is to prevent the user from accidentally not returning the return value of an OK-status RPC and having Python return None on your behalf without us being able to check for that bug.

However, we do realize that is an issue, especially in the face of interceptors, and we were brainstorming three possible solutions (please let us know if you can think of more good ones or strong arguments for or against the following ones):

1. Relaxing the API docs to allow None to be returned from RPCs when the status is explicitly set to not OK, but still enforce that requirement for OK RPCs.

2. Defining a sentinel value e.g. grpc.NoResponse() that you would be able to return from non-OK status RPCs. This way, we can still enforce that you are explicitly returning something and not accidentally returning None.

3. Allowing the handler to raise a sentinel exception type e.g. grpc.Abandon to accomplish the same thing.

--
You received this message because you are subscribed to the Google Groups "grpc.io" group.
To unsubscribe from this group and stop receiving emails from it, send an email to grpc-io+unsubscribe@googlegroups.com.
To post to this group, send email to grp...@googlegroups.com.
Visit this group at https://groups.google.com/group/grpc-io.
To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/CANODV3kW_dE_GMzaQeRifkSa_az_C_uo%3DMULgviXN5tVb4ru9g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Amit Saha]

On Wed, Oct 4, 2017 at 7:53 AM 'Mehrdad Afshari' via grpc.io <grp...@googlegroups.com> wrote:

+nath...@google.com

We were just discussing this with Nathaniel and the primary reason behind that API contract constraint, in addition to future-proofing by being conservative, is to prevent the user from accidentally not returning the return value of an OK-status RPC and having Python return None on your behalf without us being able to check for that bug.

However, we do realize that is an issue, especially in the face of interceptors, and we were brainstorming three possible solutions (please let us know if you can think of more good ones or strong arguments for or against the following ones):

1. Relaxing the API docs to allow None to be returned from RPCs when the status is explicitly set to not OK, but still enforce that requirement for OK RPCs.

2. Defining a sentinel value e.g. grpc.NoResponse() that you would be able to return from non-OK status RPCs. This way, we can still enforce that you are explicitly returning something and not accidentally returning None.

I think I like the combination of both (1) and (2). That is, if the status is non-OK, only then allow grpc.NoResponse().

 

3. Allowing the handler to raise a sentinel exception type e.g. grpc.Abandon to accomplish the same thing.

Do you mean, instead of having to set_details() and set_code(), we can just do raise grpc.Abandon(code=<BLAH>, message=<BLAH>)? I think that would be a friendly higher level addition to make.

Thanks for looking into this!

 

To unsubscribe from this group and stop receiving emails from it, send an email to grpc-io+u...@googlegroups.com.

To post to this group, send email to grp...@googlegroups.com.
Visit this group at https://groups.google.com/group/grpc-io.
To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/CANODV3kW_dE_GMzaQeRifkSa_az_C_uo%3DMULgviXN5tVb4ru9g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

--

You received this message because you are subscribed to the Google Groups "grpc.io" group.

To unsubscribe from this group and stop receiving emails from it, send an email to grpc-io+u...@googlegroups.com.

To post to this group, send email to grp...@googlegroups.com.
Visit this group at https://groups.google.com/group/grpc-io.

To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/CABdARdAZH-yo-Dp6DQ0E_%3D6yqvtyrKt7A4cwEqoaxSU6w%2B4eoA%40mail.gmail.com.

[Nathaniel Manista]

On Tue, Oct 3, 2017 at 2:03 PM, Amit Saha <amits...@gmail.com> wrote:

On Wed, Oct 4, 2017 at 7:53 AM 'Mehrdad Afshari' via grpc.io <grp...@googlegroups.com> wrote:

+nath...@google.com

We were just discussing this with Nathaniel and the primary reason behind that API contract constraint, in addition to future-proofing by being conservative, is to prevent the user from accidentally not returning the return value of an OK-status RPC and having Python return None on your behalf without us being able to check for that bug.

However, we do realize that is an issue, especially in the face of interceptors, and we were brainstorming three possible solutions (please let us know if you can think of more good ones or strong arguments for or against the following ones):

1. Relaxing the API docs to allow None to be returned from RPCs when the status is explicitly set to not OK, but still enforce that requirement for OK RPCs.

2. Defining a sentinel value e.g. grpc.NoResponse() that you would be able to return from non-OK status RPCs. This way, we can still enforce that you are explicitly returning something and not accidentally returning None.

I think I like the combination of both (1) and (2). That is, if the status is non-OK, only then allow grpc.NoResponse().

This may not be possible depending on how issue 12824 is resolved.

3. Allowing the handler to raise a sentinel exception type e.g. grpc.Abandon to accomplish the same thing.

Do you mean, instead of having to set_details() and set_code(), we can just do raise grpc.Abandon(code=<BLAH>, message=<BLAH>)? I think that would be a friendly higher level addition to make.

Possibly.

Thanks for looking into this!

Let's take further discussion over to issue 12826.

-Nathaniel