This PR adds a rescue to catch Octokit::Forbidden.
What is going on is we have a chron rake task that runs hourly which refreshes GitHub users via RefreshGithubUserJob, which calls the modified method in this PR. Certain users within our db have been deleted (or at least I cannot find or access them using either the GitHub UI or API calls), and when this job tries to call those users, we see an influx of this error as the job continues to retry and then is enqueued again an hour later. These users link to deactivated Slack accounts as well.
Other options I considered were deleting these users from our db when we see this response code, but I don't think that is the call here as a Forbidden can also come from things like hitting the secondary rate limit (the reason for this PR is not that case though).
In our case, there are only a set number of users that cause this, so it would be easy enough to also just delete them from the db manually, but I figured there will probably continue to be situations where users end up in this state over time and it was better to just rescue and log what was happening, with the possibility of deleting later too.
What:
This PR adds a rescue to catch
Octokit::Forbidden.What is going on is we have a chron rake task that runs hourly which refreshes GitHub users via
RefreshGithubUserJob, which calls the modified method in this PR. Certain users within our db have been deleted (or at least I cannot find or access them using either the GitHub UI or API calls), and when this job tries to call those users, we see an influx of this error as the job continues to retry and then is enqueued again an hour later. These users link to deactivated Slack accounts as well.Other options I considered were deleting these users from our db when we see this response code, but I don't think that is the call here as a
Forbiddencan also come from things like hitting the secondary rate limit (the reason for this PR is not that case though).In our case, there are only a set number of users that cause this, so it would be easy enough to also just delete them from the db manually, but I figured there will probably continue to be situations where users end up in this state over time and it was better to just rescue and log what was happening, with the possibility of deleting later too.
Does anyone disagree with this approach?