In general, we do not have a hard-and-fast rule about how implementation
code handles this.  It's up to the developers and code reviewers to make
sure the overall behavior is correct.

I don't see a problem with the presence of one function deep inside of the
implementation returning a status that gRPC would not return from the
application.  It's still quite possible for some other piece of code to
remap the status before it gets returned to the application.

If you are seeing a specific case in which gRPC is returning an incorrect
status code to the application, you can file an issue about that.
Otherwise, I don't see any concrete concern here.

On Wed, Aug 19, 2020 at 10:32 AM Wensheng Tang <[email protected]> wrote:

> Hi Mark,
>
> I fully understand that the gRPC does not allow some status codes to
> return to its user. However, the second function `verify_frame_header()`
> does return `GRPC_STATUS_FAILED_PRECONDITION`, which is also on the
> blocklist. Then this function does return some status codes that should not
> propagate a client. In fact, that is the thing that makes me confused about
> why such a conversion is needed. So I ask the community if gRPC does have
> clear criteria on when to convert status codes. I believe it can provide
> more guidance developers that use gRPC to do better handling with gRPC
> status codes.
>
> Best regards,
>
> 在2020年8月20日星期四 UTC+8 上午1:20:05<Mark D. Roth> 写道:
>
>> I don't actually see a problem here.  The fact that some of these
>> internal functions may use grpc_status_code to return their intent to the
>> caller doesn't mean that that same code should be returned from gRPC to the
>> application.  As a hypothetical example (I haven't looked at the function
>> and don't know what codes it might actually
>> return), gsec_aead_crypter_decrypt_iovec() might return
>> GRPC_STATUS_INVALID_ARGUMENT if the cryptographic data is invalid, but
>> that's not one of the codes that we're allowed to return from gRPC, as per
>> https://github.com/grpc/grpc/blob/master/doc/statuscodes.md.  I think
>> GRPC_STATUS_INVALID is actually the right thing to return in that
>> situation, because it reflects a clear bug of some sort in the gRPC stack.
>>
>> If you have a specific use-case in which you are using gRPC and believe
>> you are getting the wrong status code, you can file a bug about that.  But
>> so far, what you're pointing out doesn't seem to reflect an actual problem.
>>
>> On Wed, Aug 12, 2020 at 10:23 PM Wensheng Tang <[email protected]> wrote:
>>
>>> Dear gRPC developers,
>>>
>>> We notice that there are many cases gRPC or other status codes are being
>>> converted internally through our code analyzer.
>>>
>>> For example, in
>>> https://github.com/grpc/grpc/issues/23775
>>>
>>> The error code from
>>>
>>> ```c
>>> // may return GRPC_STATUS_INVALID_ARGUMENT
>>> status = gsec_aead_crypter_decrypt_iovec(...);
>>> if (status != GRPC_STATUS_OK) {
>>>     return GRPC_STATUS_INTERNAL;
>>> }
>>> ```
>>> , the original status code is dropped. I believe such a conversion is
>>> not needed. Besides, it may not present the real errors to its client,
>>> which may not follow guidelines in
>>> https://grpc.github.io/grpc/core/md_doc_statuscodes.html.
>>>
>>> On another hand, in the same function, it does keep the original status
>>> codes, some times.
>>>
>>> ```c
>>> grpc_status_code status = verify_frame_header(...);
>>> if (status != GRPC_STATUS_OK) {
>>>     return status;
>>> }
>>> ```
>>>
>>> Your team closes my issues so quickly so I am not sure if here is the
>>> right place to report. Shall we confine a clear and general guideline of
>>> when to convert status codes internally?
>>>
>>> --
>>> 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 [email protected].
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/grpc-io/c0fcae1a-7070-43b0-949e-1b043b8f7233n%40googlegroups.com
>>> <https://groups.google.com/d/msgid/grpc-io/c0fcae1a-7070-43b0-949e-1b043b8f7233n%40googlegroups.com?utm_medium=email&utm_source=footer>
>>> .
>>>
>>
>>
>> --
>> Mark D. Roth <[email protected]>
>> Software Engineer
>> Google, Inc.
>>
> --
> 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 [email protected].
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/grpc-io/ad2fd35b-8f99-4e6d-944e-81578ba2a0d8n%40googlegroups.com
> <https://groups.google.com/d/msgid/grpc-io/ad2fd35b-8f99-4e6d-944e-81578ba2a0d8n%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>


-- 
Mark D. Roth <[email protected]>
Software Engineer
Google, Inc.

-- 
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 [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/grpc-io/CAJgPXp5GHMZW_T_VqQJFEwJNW5GUOJ4q_vLR9ngMOsE0ceYx6w%40mail.gmail.com.

Reply via email to