Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add status_code property on http error handling #1185

Merged
merged 6 commits into from Mar 17, 2021

Conversation

@wmarquardt
Copy link
Contributor

@wmarquardt wmarquardt commented Feb 12, 2021

Fixes #1183 馃
Fixes #1255 馃

  • Add status code property in http error handler class.
  • Resolve issue where error_details is not populated.
Copy link
Contributor

@parthea parthea left a comment

Hi @wmarquardt ,
Thanks for submitting a PR! Could you please also update the getting started documentation here with an example?

https://github.com/googleapis/google-api-python-client/blob/master/docs/start.md#execution-and-response

We could replace the existing example with

try:
    response = request.execute()
except HttpError as e:
    print('Error response status code : {0}, reason : {1}'.format(e.status_code, e.error_details))

@parthea
Copy link
Contributor

@parthea parthea commented Mar 3, 2021

Unrelated to your changes, when I ran the example code I found that e.error_details is empty in my test code. To fix it, I had to make a change in errors.py. The fix was to add a call to self._get_reason() in the __init__ function here.

Change

    @util.positional(3)
    def __init__(self, resp, content, uri=None):
        self.resp = resp
        if not isinstance(content, bytes):
            raise TypeError("HTTP content should be bytes")
        self.content = content
        self.uri = uri
        self.error_details = ""

to

    @util.positional(3)
    def __init__(self, resp, content, uri=None):
        self.resp = resp
        if not isinstance(content, bytes):
            raise TypeError("HTTP content should be bytes")
        self.content = content
        self.uri = uri
        self.error_details = ""
        self._get_reason()

Copy link
Contributor

@parthea parthea left a comment

Hi @wmarquardt ,

I missed this in the last review. Please could you also add a docstring in the new function?

googleapiclient/errors.py Show resolved Hide resolved
@wmarquardt
Copy link
Contributor Author

@wmarquardt wmarquardt commented Mar 3, 2021

Hi @parthea,

Thank's for your review. I'll make these changes as soon as possible

@parthea
Copy link
Contributor

@parthea parthea commented Mar 17, 2021

@busunkim96 , Please can you take a look? I was able to re-create the issue in #1255 where error_details isn't populated by removing the fix that I added here.

(errors) partheniou@partheniou-vm-1:~/git/google-api-python-client-upstream$ pytest tests/test_errors.py::Error::test_json_body
============================================================================================= test session starts =============================================================================================
...
tests/test_errors.py F                                                                                                                                                                                  [100%]

================================================================================================== FAILURES ===================================================================================================
____________________________________________________________________________________________ Error.test_json_body _____________________________________________________________________________________________

self = <tests.test_errors.Error testMethod=test_json_body>

    def test_json_body(self):
        """Test a nicely formed, expected error response."""
        resp, content = fake_response(
            JSON_ERROR_CONTENT,
            {"status": "400", "content-type": "application/json"},
            reason="Failed",
        )
        error = HttpError(resp, content, uri="http://example.org")
>       self.assertEqual(error.error_details, "error details")
E       AssertionError: '' != 'error details'
E       + error details

tests/test_errors.py:86: AssertionError

Copy link
Contributor

@busunkim96 busunkim96 left a comment

This looks OK to me @parthea but I am not very familiar with the code in errors.py. Is there something in particular you're concerned about?

@gcf-merge-on-green gcf-merge-on-green bot merged commit db2a766 into googleapis:master Mar 17, 2021
3 checks passed
gcf-merge-on-green bot pushed a commit that referenced this issue Mar 31, 2021
馃 I have created a release \*beep\* \*boop\*
---
## [2.1.0](https://www.github.com/googleapis/google-api-python-client/compare/v2.0.2...v2.1.0) (2021-03-31)


### Features

* add status_code property on http error handling ([#1185](https://www.github.com/googleapis/google-api-python-client/issues/1185)) ([db2a766](https://www.github.com/googleapis/google-api-python-client/commit/db2a766bbd976742f6ef10d721d8423c8ac9246d))


### Bug Fixes

* Change default of `static_discovery` when `discoveryServiceUrl` set ([#1261](https://www.github.com/googleapis/google-api-python-client/issues/1261)) ([3b4f2e2](https://www.github.com/googleapis/google-api-python-client/commit/3b4f2e243709132b5ca41a3c23853d5067dfb0ab))
* correct api version in oauth-installed.md ([#1258](https://www.github.com/googleapis/google-api-python-client/issues/1258)) ([d1a255f](https://www.github.com/googleapis/google-api-python-client/commit/d1a255fcbeaa36f615cede720692fea2b9f894db))
* fix .close() ([#1231](https://www.github.com/googleapis/google-api-python-client/issues/1231)) ([a9583f7](https://www.github.com/googleapis/google-api-python-client/commit/a9583f712d13c67aa282d14cd30e00999b530d7c))
* Resolve issue where num_retries would have no effect ([#1244](https://www.github.com/googleapis/google-api-python-client/issues/1244)) ([c518472](https://www.github.com/googleapis/google-api-python-client/commit/c518472e836c32ba2ff5e8480ab5a7643f722d46))


### Documentation

* Distinguish between public/private docs in 2.0 guide ([#1226](https://www.github.com/googleapis/google-api-python-client/issues/1226)) ([a6f1706](https://www.github.com/googleapis/google-api-python-client/commit/a6f17066caf6e911b7e94e8feab52fa3af2def1b))
* Update README to promote cloud client libraries ([#1252](https://www.github.com/googleapis/google-api-python-client/issues/1252)) ([22807c9](https://www.github.com/googleapis/google-api-python-client/commit/22807c92ce754ff3d60f240ec5c38de50c5b654b))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
gcf-merge-on-green bot pushed a commit that referenced this issue Sep 2, 2021
self._get_reason is being called in \_\_init\_\_ (#1185)  , so why not save it then?
also in the \_\_repr\_\_ function we got the reason by calling the _get_reason function right in the beginning, but was then called again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants