Skip to content

feat: add status_code property on http error handling#1185

Merged
gcf-merge-on-green[bot] merged 6 commits intogoogleapis:masterfrom
wmarquardt:http-error-status
Mar 17, 2021
Merged

feat: add status_code property on http error handling#1185
gcf-merge-on-green[bot] merged 6 commits intogoogleapis:masterfrom
wmarquardt:http-error-status

Conversation

@wmarquardt
Copy link
Copy Markdown
Contributor

@wmarquardt wmarquardt commented Feb 12, 2021 •
edited by parthea
Loading

Fixes #1183 🦕
Fixes #1255 🦕

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

@wmarquardt wmarquardt requested a review from a team as a code owner February 12, 2021 21:46
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 12, 2021
@wmarquardt wmarquardt mentioned this pull request Feb 12, 2021
Closed
@parthea parthea added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 15, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 15, 2021
parthea
parthea reviewed Mar 3, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@parthea parthea left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy Markdown
Contributor

parthea commented Mar 3, 2021 •
edited
Loading

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()

parthea
parthea reviewed Mar 3, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@parthea parthea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @wmarquardt ,

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

@wmarquardt
Copy link
Copy Markdown
Contributor Author

wmarquardt commented Mar 3, 2021

Hi @parthea,

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

@parthea parthea requested a review from busunkim96 March 17, 2021 16:53
@parthea parthea added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 17, 2021
@parthea parthea self-assigned this Mar 17, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 17, 2021
@parthea
Copy link
Copy Markdown
Contributor

parthea commented Mar 17, 2021 •
edited
Loading

@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

busunkim96
busunkim96 reviewed Mar 17, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@busunkim96 busunkim96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@parthea parthea added the automerge Merge the pull request once unit tests and other checks pass. label Mar 17, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit db2a766 into googleapis:master Mar 17, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Mar 17, 2021
gcf-merge-on-green bot pushed a commit that referenced this pull request Mar 31, 2021
@release-please
chore: release 2.1.0 (#1256)
cc717a1 
🤖 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).
@dermasmid dermasmid mentioned this pull request Sep 2, 2021
Merged
gcf-merge-on-green bot pushed a commit that referenced this pull request Sep 2, 2021
@dermasmid
fix: remove repeated calls to self._get_reason (#1513)
d5cf4e0 
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

Reviewers

@parthea parthea parthea approved these changes

+1 more reviewer

@busunkim96 busunkim96 busunkim96 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@parthea parthea

@wmarquardt wmarquardt

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

HttpError error_details isn't populated unless __repr__ is called first Get HTTP status code error handling

4 participants

@wmarquardt @parthea @busunkim96 @yoshi-kokoro