diff options
author | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-02-24 05:17:23 +0300 |
---|---|---|
committer | Dmitriy Zaporozhets <dzaporozhets@gitlab.com> | 2015-02-24 05:17:23 +0300 |
commit | c87c1cb3b9def05d29f3fab65af9474d7a11b24f (patch) | |
tree | a76cc43b59abcb19a615b804b7744ad78b8d2e1d /lib | |
parent | 8601c502e8cabc70b95a5d4cfb6649889fca83c5 (diff) | |
parent | 5f232b5687b447e7eac40f58c56628da22580de6 (diff) |
Merge branch 'api-empty-commit' into 'master'
Improve error messages when file editing fails
Give more specific errors in API responses and web UI flash messages when a file update fails. See #1479.
Instead of returning false from `Gitlab::Satellite::Files::EditFileAction#commit!` when a `Grit::Git::CommandFailed` error is raised, now `#commit!` raises a different error depending on whether the failure happened during checkout, commit, or push.
@dzaporozhets Please let me know if you want to change the HTTP status codes or the error messages in `Files::UpdateService`
cc @sytse
See merge request !1569
Diffstat (limited to 'lib')
-rw-r--r-- | lib/api/files.rb | 3 | ||||
-rw-r--r-- | lib/gitlab/satellite/files/edit_file_action.rb | 28 | ||||
-rw-r--r-- | lib/gitlab/satellite/satellite.rb | 4 |
3 files changed, 28 insertions, 7 deletions
diff --git a/lib/api/files.rb b/lib/api/files.rb index e6e71bac367..3176ef0e256 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -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 diff --git a/lib/gitlab/satellite/files/edit_file_action.rb b/lib/gitlab/satellite/files/edit_file_action.rb index 2834b722b27..82d71ab9906 100644 --- a/lib/gitlab/satellite/files/edit_file_action.rb +++ b/lib/gitlab/satellite/files/edit_file_action.rb @@ -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 diff --git a/lib/gitlab/satellite/satellite.rb b/lib/gitlab/satellite/satellite.rb index 62d1bb364d3..70125d539da 100644 --- a/lib/gitlab/satellite/satellite.rb +++ b/lib/gitlab/satellite/satellite.rb @@ -1,5 +1,9 @@ module Gitlab module Satellite + class CheckoutFailed < StandardError; end + class CommitFailed < StandardError; end + class PushFailed < StandardError; end + class Satellite include Gitlab::Popen |