GitLab steht Mittwoch, den 23. September, zwischen 10:00 und 12:00 Uhr aufgrund von Wartungsarbeiten nicht zur Verfügung.

Commit 5f232b56 authored by Vinnie Okada's avatar Vinnie Okada

Improve error messages when file editing fails

Give more specific errors in API responses and web UI flash messages
when a file update fails.
parent ebe0d341
v 7.9.0 (unreleased)
- Move labels/milestones tabs to sidebar
- Upgrade Rails gem to version 4.1.9.
- Improve error messages for file edit failures
- Improve UI for commits, issues and merge request lists
- Fix commit comments on first line of diff not rendering in Merge Request Discussion view.
......
......@@ -37,11 +37,14 @@ class BaseService
private
def error(message)
{
def error(message, http_status = nil)
result = {
message: message,
status: :error
}
result[:http_status] = http_status if http_status
result
end
def success
......
......@@ -20,17 +20,19 @@ module Files
end
edit_file_action = Gitlab::Satellite::EditFileAction.new(current_user, project, ref, path)
created_successfully = edit_file_action.commit!(
edit_file_action.commit!(
params[:content],
params[:commit_message],
params[:encoding]
)
if created_successfully
success
else
error("Your changes could not be committed. Maybe the file was changed by another process or there was nothing to commit?")
end
success
rescue Gitlab::Satellite::CheckoutFailed => ex
error("Your changes could not be committed because ref '#{ref}' could not be checked out", 400)
rescue Gitlab::Satellite::CommitFailed => ex
error("Your changes could not be committed. Maybe there was nothing to commit?", 409)
rescue Gitlab::Satellite::PushFailed => ex
error("Your changes could not be committed. Maybe the file was changed by another process?", 409)
end
end
end
......@@ -117,7 +117,8 @@ module API
branch_name: branch_name
}
else
render_api_error!(result[:message], 400)
http_status = result[:http_status] || 400
render_api_error!(result[:message], http_status)
end
end
......
......@@ -15,7 +15,11 @@ module Gitlab
prepare_satellite!(repo)
# create target branch in satellite at the corresponding commit from bare repo
repo.git.checkout({ raise: true, timeout: true, b: true }, ref, "origin/#{ref}")
begin
repo.git.checkout({ raise: true, timeout: true, b: true }, ref, "origin/#{ref}")
rescue Grit::Git::CommandFailed => ex
log_and_raise(CheckoutFailed, ex.message)
end
# update the file in the satellite's working dir
file_path_in_satellite = File.join(repo.working_dir, file_path)
......@@ -31,19 +35,31 @@ module Gitlab
# commit the changes
# will raise CommandFailed when commit fails
repo.git.commit(raise: true, timeout: true, a: true, m: commit_message)
begin
repo.git.commit(raise: true, timeout: true, a: true, m: commit_message)
rescue Grit::Git::CommandFailed => ex
log_and_raise(CommitFailed, ex.message)
end
# push commit back to bare repo
# will raise CommandFailed when push fails
repo.git.push({ raise: true, timeout: true }, :origin, ref)
begin
repo.git.push({ raise: true, timeout: true }, :origin, ref)
rescue Grit::Git::CommandFailed => ex
log_and_raise(PushFailed, ex.message)
end
# everything worked
true
end
rescue Grit::Git::CommandFailed => ex
Gitlab::GitLogger.error(ex.message)
false
end
private
def log_and_raise(errorClass, message)
Gitlab::GitLogger.error(message)
raise(errorClass, message)
end
end
end
......
module Gitlab
module Satellite
class CheckoutFailed < StandardError; end
class CommitFailed < StandardError; end
class PushFailed < StandardError; end
class Satellite
include Gitlab::Popen
......
......@@ -98,13 +98,33 @@ describe API::API, api: true do
expect(response.status).to eq(400)
end
it "should return a 400 if satellite fails to create file" do
Gitlab::Satellite::EditFileAction.any_instance.stub(
commit!: false,
)
it 'should return a 400 if the checkout fails' do
Gitlab::Satellite::EditFileAction.any_instance.stub(:commit!)
.and_raise(Gitlab::Satellite::CheckoutFailed)
put api("/projects/#{project.id}/repository/files", user), valid_params
expect(response.status).to eq(400)
ref = valid_params[:branch_name]
expect(response.body).to match("ref '#{ref}' could not be checked out")
end
it 'should return a 409 if the file was not modified' do
Gitlab::Satellite::EditFileAction.any_instance.stub(:commit!)
.and_raise(Gitlab::Satellite::CommitFailed)
put api("/projects/#{project.id}/repository/files", user), valid_params
expect(response.status).to eq(409)
expect(response.body).to match("Maybe there was nothing to commit?")
end
it 'should return a 409 if the push fails' do
Gitlab::Satellite::EditFileAction.any_instance.stub(:commit!)
.and_raise(Gitlab::Satellite::PushFailed)
put api("/projects/#{project.id}/repository/files", user), valid_params
expect(response.status).to eq(409)
expect(response.body).to match("Maybe the file was changed by another process?")
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