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

Improve and error handling for update and autoupdate features. #3161

Merged
merged 2 commits into from
Jul 3, 2022

Conversation

Jerrie-Aries
Copy link
Contributor

Main issue:

  • Fix rare issue with autoupdate feature where the bot raises KeyError while trying to get the commit data. See picture below:
    unknown
    The cause of the issue is probably because of the forked repo name is other than modmail, so the url cannot be found with the response status 404. The response data would be like:
{'message': 'Not Found', 'documentation_url': 'https://docs.github.com/rest/reference/repos#merge-a-branch'} 

Other:

  • Add error handling for update and autoupdate features.
  • Start autoupdate and post_metadata loops on on_ready event.
  • Add read_before_return (type: bool) parameter to GitHub.request method. If set to True, ClientResponse.read() method will be performed first before returning the ClientResponse object. Useful if we want to do other method like .json() or .text() after exiting the ClientSession context manager.
  • Fix obsolete references.

- Return only if hosting method is HEROKU.
- Try to get the response error message first if any.
@fourjr fourjr added the staged Staged for next version label Jul 3, 2022
@fourjr fourjr merged commit b446861 into modmail-dev:development Jul 3, 2022
@Jerrie-Aries Jerrie-Aries deleted the autoupdate branch July 3, 2022 15:01
headers: dict = None,
) -> Union[ClientResponse, dict, str]:
return_response: bool = False,
read_before_return: bool = True,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at the PR again and just noticed the minor mistake here, the read_before_return should default to False.

fourjr added a commit that referenced this pull request Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
staged Staged for next version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants