Rely on error types to determine when a repo should be removed on the RepoSyncWorker (#10131)

* Registers the Github AcocuntSuspended & RepositoryUnavailable errors

Adds the above-mentioned errors to the list of known errors raised by
Octokit as to make possible for the rest of the application to
specifically trap those.

* Makes the RepoSyncWorker error handling rely on types

The current implementation attempted to parse the error message to
determine if the error reported by Github's API can/should be handled,
this commit changes the implementation so that the exception type is
used instead.
This commit is contained in:
Diogo Osório 2020-09-01 20:01:23 +01:00 committed by GitHub
parent 0b849341e0
commit a5e7b1cf60
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 45 additions and 8 deletions

View file

@ -17,5 +17,11 @@ module Github
class InvalidRepository < ArgumentError
end
class AccountSuspended < ClientError
end
class RepositoryUnavailable < ClientError
end
end
end

View file

@ -27,12 +27,10 @@ module GithubRepos
info_hash: fetched_repo.to_hash,
)
repo.user&.touch(:github_repos_updated_at)
rescue Github::Errors::NotFound, Github::Errors::Unauthorized
repo.destroy
rescue Github::Errors::ClientError => e
msg = e.message
raise e unless msg.include?("Your account was suspended") || msg.include?("Repository access blocked")
rescue Github::Errors::NotFound,
Github::Errors::Unauthorized,
Github::Errors::AccountSuspended,
Github::Errors::RepositoryUnavailable
repo.destroy
end
end

View file

@ -26,6 +26,28 @@ RSpec.describe Github::OauthClient, type: :service, vcr: true do
end
end
describe ".repository" do
subject(:client) { described_class.new(client_id: "value", client_secret: "value") }
let(:repo_id) { 100 }
context "when the Github account to which the repo belongs to is suspended" do
it "returns a Github::Errors::AccountSuspended error" do
stub_request(:get, %r{repositories/#{repo_id}}).to_return(body: "account was suspended", status: 403)
expect { client.repository(repo_id) }.to raise_error(Github::Errors::AccountSuspended)
end
end
context "when the repo is unavailable" do
it "returns a Github::Errors::RepositoryUnavailable error" do
stub_request(:get, %r{repositories/#{repo_id}}).to_return(body: "repository access blocked", status: 403)
expect { client.repository(repo_id) }.to raise_error(Github::Errors::RepositoryUnavailable)
end
end
end
describe ".issue" do
it "returns a an issue using the client_id/client_secret" do
VCR.use_cassette("github_client_issue") do

View file

@ -52,7 +52,9 @@ RSpec.describe GithubRepos::RepoSyncWorker, type: :worker do
it "destroys suspended account repos" do
repo_id = repo.id
client_error = Github::Errors::ClientError.new(message: "GET https:// 403 - Sorry. Your account was suspended.")
client_error = Github::Errors::AccountSuspended.new(
message: "GET https:// 403 - Sorry. Your account was suspended.",
)
allow(github_client).to receive(:repository).and_raise(client_error)
worker.perform(repo.id)
@ -61,11 +63,20 @@ RSpec.describe GithubRepos::RepoSyncWorker, type: :worker do
it "destroys blocked access repos" do
repo_id = repo.id
client_error = Github::Errors::ClientError.new(message: "GET https:// 451 - Repository access blocked.")
client_error = Github::Errors::RepositoryUnavailable.new(message: "GET https:// 451 - Repository access blocked.")
allow(github_client).to receive(:repository).and_raise(client_error)
worker.perform(repo.id)
expect(GithubRepo.find_by(id: repo_id)).to be_nil
end
it "retains the repo on an unexpected Github client error" do
repo_id = repo.id
client_error = Github::Errors::ServerError.new(message: "GET https:// 500 - Internal Server Error")
allow(github_client).to receive(:repository).and_raise(client_error)
expect { worker.perform(repo.id) }.to raise_error(client_error)
expect(GithubRepo.find_by(id: repo_id)).to be_present
end
end
end