Commit 4944d897 authored by Ash McKenzie's avatar Ash McKenzie

Merge branch 'kborges/github-service-do-not-raise-any-errors' into 'master'

Do not raise on some GitHub exceptions

See merge request gitlab-org/gitlab!32998
parents 9ce58eed 2d35e2c0
......@@ -10,15 +10,26 @@ module Import
return error(_('This namespace has already been taken! Please choose another one.'), :unprocessable_entity)
end
project = Gitlab::LegacyGithubImport::ProjectCreator
.new(repo, project_name, target_namespace, current_user, access_params, type: provider)
.execute(extra_project_attrs)
project = create_project(access_params, provider)
if project.persisted?
success(project)
else
error(project_save_error(project), :unprocessable_entity)
end
rescue Octokit::Error => e
log_error(e)
end
def create_project(access_params, provider)
Gitlab::LegacyGithubImport::ProjectCreator.new(
repo,
project_name,
target_namespace,
current_user,
access_params,
type: provider
).execute(extra_project_attrs)
end
def repo
......@@ -44,6 +55,18 @@ module Import
def authorized?
can?(current_user, :create_projects, target_namespace)
end
private
def log_error(exception)
Gitlab::Import::Logger.error(
message: 'Import failed due to a GitHub error',
status: exception.response_status,
error: exception.response_body
)
error(_('Import failed due to a GitHub error: %{original}') % { original: exception.response_body }, :unprocessable_entity)
end
end
end
......
......@@ -11616,6 +11616,9 @@ msgstr ""
msgid "Import an exported GitLab project"
msgstr ""
msgid "Import failed due to a GitHub error: %{original}"
msgstr ""
msgid "Import from"
msgstr ""
......
# frozen_string_literal: true
require 'spec_helper'
describe Import::GithubService do
let_it_be(:user) { create(:user) }
let_it_be(:token) { 'complex-token' }
let_it_be(:access_params) { { github_access_token: 'github-complex-token' } }
let_it_be(:client) { Gitlab::LegacyGithubImport::Client.new(token) }
let_it_be(:params) { { repo_id: 123, new_name: 'new_repo', target_namespace: 'root' } }
let(:subject) { described_class.new(client, user, params) }
before do
allow(subject).to receive(:authorized?).and_return(true)
end
context 'do not raise an exception on input error' do
let(:exception) { Octokit::ClientError.new(status: 404, body: 'Not Found') }
before do
expect(client).to receive(:repo).and_raise(exception)
end
it 'logs the original error' do
expect(Gitlab::Import::Logger).to receive(:error).with({
message: 'Import failed due to a GitHub error',
status: 404,
error: 'Not Found'
}).and_call_original
subject.execute(access_params, :github)
end
it 'returns an error' do
result = subject.execute(access_params, :github)
expect(result).to include(
message: 'Import failed due to a GitHub error: Not Found',
status: :error,
http_status: :unprocessable_entity
)
end
end
it 'raises an exception for unknown error causes' do
exception = StandardError.new('Not Implemented')
expect(client).to receive(:repo).and_raise(exception)
expect(Gitlab::Import::Logger).not_to receive(:error)
expect { subject.execute(access_params, :github) }.to raise_error(exception)
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment