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

2 files changed, 13 insertions, 8 deletions

diff --git a/app/services/base_service.rb b/app/services/base_service.rb
index bb51795df7c..52ab29f1492 100644
--- a/app/services/base_service.rb
+++ b/app/services/base_service.rb
@@ -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
diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb
index b4986e1c5c6..bcf0e7f3cee 100644
--- a/app/services/files/update_service.rb
+++ b/app/services/files/update_service.rb
@@ -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