report to users server errors over REST

[vchudnov-g]

Right now, it looks like for at least some server errors, we do not handle or report them to the user and instead (if we're lucky?) encounter an exception later. Case in point: Showcase does not recognize capitalized booleans over rest (the issue in #1407) and returns an error. I can reproduce the error with curl, and the HTTP body is

{"error":{"code":400,"message":"error reading query params: terminal field \"unknownEnum\" of field path \"unknownEnum\" is of type \"bool\" with value string \"False\", which could not be parsed: could not parse \"False\" as a bool","details":null,"Body":"","Header":null,"Errors":null}}

Using the Python GAPIC, I simply get an exception

TypeError: 'NoneType' object is not iterable

If I don't catch the exception, the stack trace is (full filepaths redacted):

Traceback (most recent call last):
  File "HOME/APPDIR/vchudnov_showcase_get_enum.py", line 70, in <module>
    sample_get_enum()
  File "HOME/APPDIR/vchudnov_showcase_get_enum.py", line 64, in sample_get_enum
    response = client.get_enum(request=request)
  File "HOME/APPDIR/showcase/google/showcase_v1beta1/services/compliance/client.py", line 983, in get_enum
    response = rpc(
  File "HOME/.pyenv/versions/3.9.5/lib/python3.9/site-packages/google/api_core/gapic_v1/method.py", line 154, in __call__
    return wrapped_func(*args, **kwargs)
  File "HOME/.pyenv/versions/3.9.5/lib/python3.9/site-packages/google/api_core/grpc_helpers.py", line 50, in error_remapped_callable
    return callable_(*args, **kwargs)
  File "HOME/GAPICDIR/google/showcase_v1beta1/services/compliance/transports/rest.py", line 475, in __call__
    raise core_exceptions.from_http_response(response)
  File "HOME/.pyenv/versions/3.9.5/lib/python3.9/site-packages/google/api_core/exceptions.py", line 484, in from_http_response
    filter(

Reporting errors correctly should be a blocker for launching REGAPIC.

[vam-google]

This seems related to https://github.com/googleapis/gapic-generator-python/issues/1407. I'm still fixing multiple issues found by running unit tests (via py_test target). This PR https://github.com/googleapis/python-api-core/pull/428 will have a great impact on how we transcode and serialize our request messages (it is also expected to fix many similar issues).

[vchudnov-g]

To be clear: yes, this issue was sparked by my running into the issue in #1407, but this itself is a separate issue: that when we get an error from the server, we don't report it to the user, but merely get a type error and a stack dump. We should be reporting the error information (in this case, the message field).

In that sense, this might be another manifestation of python-compute#279, although there it did not manifest as an exception but as a non-informative error message.

[hkdevandla]

@atulep , can you take a look at this?

[atulep]

The error occurs because error.details field of the status.proto is set to None in showcase response.

The payload we receive is:

{'error': {'code': 400, 'message': 'error reading query params: terminal field "unknownEnum" of field path "unknownEnum" is of type "bool" with value string "True", which could not be parsed: could not parse "True" as a bool', 'details': None, 'Body': '', 'Header': None, 'Errors': None}}

It looks to me like response.json method that deserializes bytes from Showcase sets default value for missing fields.

I think most APIs now send details in the payload so this issue should be quite rare in practice. To solve it in general, we can add additional check for null value in the api-core logic, or showcase can start sending details.

[atulep]

This issue affects only REST which we don't support yet. Additionally, it's been discovered only with synthetic Showcase test case, which may not be doing all the things real APIs do - hence the error may only affect Showcase. As such, lowering priority to P2 sounds reasonable here.

[vchudnov-g]

A couple of points here:

• we should indeed have Showcase fill in details for most errors (see Showcase #1257)
• the Python GAPIC should be more robust when encountering a missing detail; we should not exacerbate the error client-side if the service does not provide that field
• to test this behavior, Showcase should explicitly NOT fill details for some errors that we expect to encounter in tests