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

Update handling of commit data requests to account for API failure #317

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

dkbennett
Copy link
Member

Summary of the pull request

This improves handling and logging of failures while processing pull requests if the checks or checksuite API fails from Octokit. The intent here is to allow pull requests to continue to process even if we cannot obtain their checks information, and to clean up any data they may have so we do not get state tear or data corruption.
 

References and relevant issues

Closes #316

Detailed description of the pull request / Additional comments

We started seeing some errors coming from a small set of pull requests originating in Octokit. These errors were related to the Checks data requests. These are new and did not occur previously and appear to be tied to specific pull requests, but not all pull requests.

For most data we process by querying a repository once and then processing each item in the result. In the case of pull requests, we query the data multiple times for Checks information for each pull request. If one of the checks information fails it can cause the entire data sync to fail. @guimafelipe put in a quick fix for this for failures observed in the Pull Requests widget, but they were also occurring in the developer pull requests (used for notifications).

This fix addresses that problem more thoroughly. In addition to handling both types of checks API calls, the data deletion was also moved up in the code to happen first, before we attempt to query the new data. This ensures we have removed all existing data before we hit an exception in querying the new data, which keeps the data in a good state. The logging was also tweaked so we keep error messages short and not dump the entire stack (which can be very large in the case of Octokit failures). Instead we will log the error and then put the detailed stack in the Debug output.

Specific changes:

  • Moved deletion of checks information for pull request processing to happen before querying new check data in case of check query failure.
  • Included all checks API calls in the try-catch for API failure handling.
  • Tweaked logging messages and comments.

Also added a user loginid of the GitHub client logging of the Repository query. This is unrelated to this change but in the area and useful now that we support multiple GitHub accounts so it is clear which login is being used to make the query for troubleshooting.

Validation steps performed

  • Verified pull request widget works for repositories which have pull requests that fail the API checks call.
  • Verified the database records for queries which fail checks have correct data and are set to "None" for checks conclusions, since we cannot make any conclusion about them when we do not have the information.
  • Verified the rest of the data looks correct for pull request processing in this scenario.
  • Created several widgets of different repositories.

PR checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated

@dkbennett dkbennett merged commit 5f3ef13 into main Jan 12, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Octokit failures in checks API can cause Pull Request data updating to fail.
3 participants