Commit 2d35e2c0 authored by Kassio Borges's avatar Kassio Borges

Do not raise on some github exceptions

To reduce sidekiq error rates due to client errors from GitHub imports,
user mistakes on inputed data, like invalid credentials or URLs, won't
raise exceptions anymore. Instead, the error will be logged.

Other exceptions will continue to be raise.
parent 9766ca6f
...@@ -10,15 +10,26 @@ module Import ...@@ -10,15 +10,26 @@ module Import
return error(_('This namespace has already been taken! Please choose another one.'), :unprocessable_entity) return error(_('This namespace has already been taken! Please choose another one.'), :unprocessable_entity)
end end
project = Gitlab::LegacyGithubImport::ProjectCreator project = create_project(access_params, provider)
.new(repo, project_name, target_namespace, current_user, access_params, type: provider)
.execute(extra_project_attrs)
if project.persisted? if project.persisted?
success(project) success(project)
else else
error(project_save_error(project), :unprocessable_entity) error(project_save_error(project), :unprocessable_entity)
end 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 end
def repo def repo
...@@ -44,6 +55,18 @@ module Import ...@@ -44,6 +55,18 @@ module Import
def authorized? def authorized?
can?(current_user, :create_projects, target_namespace) can?(current_user, :create_projects, target_namespace)
end 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
end end
......
...@@ -11622,6 +11622,9 @@ msgstr "" ...@@ -11622,6 +11622,9 @@ msgstr ""
msgid "Import an exported GitLab project" msgid "Import an exported GitLab project"
msgstr "" msgstr ""
msgid "Import failed due to a GitHub error: %{original}"
msgstr ""
msgid "Import from" msgid "Import from"
msgstr "" 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