Raise the exception from #execute instead of #run_hook. #1156 #3069issue_1156

2 files changed, 15 insertions, 13 deletions

diff --git a/app/services/git_hooks_service.rb b/app/services/git_hooks_service.rb
index b7804ed472f..8f5c3393dfc 100644
--- a/app/services/git_hooks_service.rb
+++ b/app/services/git_hooks_service.rb
@@ -8,23 +8,21 @@ class GitHooksService
     @newrev     = newrev
     @ref        = ref
 
-    if run_hook('pre-receive') && run_hook('update')
-      yield
-
-      run_hook('post-receive')
+    %w(pre-receive update).each do |hook_name|
+      unless run_hook(hook_name)
+        raise PreReceiveError.new("Git operation was rejected by #{hook_name} hook")
+      end
     end
+
+    yield
+
+    run_hook('post-receive')
   end
 
   private
 
   def run_hook(name)
     hook = Gitlab::Git::Hook.new(name, @repo_path)
-    status = hook.trigger(@user, @oldrev, @newrev, @ref)
-
-    if !status && (name != 'post-receive')
-      raise PreReceiveError.new("Git operation was rejected by #{name} hook")
-    end
-
-    status
+    hook.trigger(@user, @oldrev, @newrev, @ref)
   end
 end
diff --git a/spec/services/git_hooks_service_spec.rb b/spec/services/git_hooks_service_spec.rb
index bb639a5ae23..7e018d3c9fe 100644
--- a/spec/services/git_hooks_service_spec.rb
+++ b/spec/services/git_hooks_service_spec.rb
@@ -31,7 +31,9 @@ describe GitHooksService do
         expect(service).to receive(:run_hook).with('pre-receive').and_return(false)
         expect(service).not_to receive(:run_hook).with('post-receive')
 
-        service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+        expect do
+          service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+        end.to raise_error(GitHooksService::PreReceiveError)
       end
     end
 
@@ -41,7 +43,9 @@ describe GitHooksService do
         expect(service).to receive(:run_hook).with('update').and_return(false)
         expect(service).not_to receive(:run_hook).with('post-receive')
 
-        service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+        expect do
+          service.execute(user, @repo_path, @blankrev, @newrev, @ref)
+        end.to raise_error(GitHooksService::PreReceiveError)
       end
     end