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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exception handling to docs #1082

Closed
mm326 opened this issue Nov 6, 2020 · 2 comments
Closed

Add exception handling to docs #1082

mm326 opened this issue Nov 6, 2020 · 2 comments
Assignees

Comments

@mm326
Copy link
Contributor

@mm326 mm326 commented Nov 6, 2020

Hi :)

I was reading the docs looking for an example to handle exceptions from when request.execute() goes wrong e.g. a 403 due to
Exceeding qouta limits.

I would like for the docs to be updated with a try: and except: like this

    try:
        response = request.execute()
    except HttpError as e:
        logger.error('Error response status code %d, reason %s:', e.resp.status, e.content)
        return {'error': 403, 'body' : 'YouTube API Data v3 qouta limit exceeded'}

or something else in the except block

If you're happy with this I'd like to contribute this as a first timer to open source?

@parthea
Copy link
Contributor

@parthea parthea commented Nov 7, 2020

Hey @mm326 ,

Thanks for suggesting an update to the docs! A pull request for the docs would be very welcome! My initial thought is to modify the except block to be more generic and just let the specific quota error message bubble up. I'd suggest to use e.error_details instead of e.content as the output of e.content seems overwhelming.

For example,

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

I tried sample code from YouTube API Data v3 and sent a bunch of requests to exceed my YouTube quota limit to test the output of the proposed docs update. After the quota was exceeded, I ran the above code and found that e.error_details is empty. We would need to make an improvement to the library to improve how error_details attribute is populated. Are you by any chance interested in making changes to the error parsing in the library? The code that does the error parsing is in function _get_reason() function of HttpError located here .

Looking at the the error parsing code, the code is only looking for either details or detail as the keyword for looking for detailed error information which isn't part of the response that we get so the error_details attribute in HttpError isn't populated. My initial thought is that it would be better to at least populate the error_details with whatever is in message rather than have it blank.

>>> data = json.loads(self.content.decode("utf-8"))
>>> data['error']
{'code': 403, 'message': 'The request cannot be completed because you have exceeded your <a href="/youtube/v3/getting-started#quota">quota</a>.', 'errors': [{'message': 'The request cannot be completed because you have exceeded your <a href="/youtube/v3/getting-started#quota">quota</a>.', 'domain': 'youtube.quota', 'reason': 'quotaExceeded'}]}

I managed to hack together some code to determine the best error detail keyword to use and then populate the error_details attribute.

Existing code

    def _get_reason(self):
        """Calculate the reason for the error from the response content."""
        reason = self.resp.reason
        try:
            data = json.loads(self.content.decode("utf-8"))
            if isinstance(data, dict):
                reason = data["error"]["message"]
                if "details" in data["error"]:
                    self.error_details = data["error"]["details"]
                elif "detail" in data["error"]:
                    self.error_details = data["error"]["detail"]

Improved code

    def _get_reason(self):
        """Calculate the reason for the error from the response content."""
        reason = self.resp.reason
        try:
            data = json.loads(self.content.decode("utf-8"))
            if isinstance(data, dict):
                reason = data["error"]["message"]
                # APIs are not consistent in the error detail keyword
                # so determine the appropriate error detail keyword
                error_detail_keyword = next((kw for kw in ['detail', 'details', 'message'] if kw in data['error']), '')
                if error_detail_keyword:
                    self.error_details = data["error"][error_detail_keyword]

Please let me know if you are intersted in opening a PR to improve the error parsing in the library! You would also need to add a unit test here.

@mm326
Copy link
Contributor Author

@mm326 mm326 commented Nov 7, 2020

I've submitted a PR let me know what you think :)

gcf-merge-on-green bot pushed a commit that referenced this issue Nov 8, 2020
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [x] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/google-api-python-client/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [x] Ensure the tests and linter pass
- [x] Code coverage does not decrease (if any source code was changed)
- [x] Appropriate docs were updated (if necessary)

Fixes #1082 🦕
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants