Handle internal API errors when getting merge request URLsonly-parse-mr-urls-on-success

3 files changed, 29 insertions, 7 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f83bdf6..2ad4116 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,6 @@
+v5.3.1
+  - Gracefully handle internal API errors when getting merge request URLs
+
 v5.3.0
   - Add ability to have git-receive-pack and git-upload-pack to go over Gitaly
 
diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb
index fad06d6..4c938b1 100644
--- a/lib/gitlab_net.rb
+++ b/lib/gitlab_net.rb
@@ -73,7 +73,14 @@ class GitlabNet
     url = "#{host}/merge_request_urls?project=#{URI.escape(repo_path)}&changes=#{URI.escape(changes)}"
     url += "&gl_repository=#{URI.escape(gl_repository)}" if gl_repository
     resp = get(url)
-    JSON.parse(resp.body) rescue []
+
+    if resp.code == '200'
+      JSON.parse(resp.body)
+    else
+      []
+    end
+  rescue
+    []
   end
 
   def check
diff --git a/spec/gitlab_net_spec.rb b/spec/gitlab_net_spec.rb
index f3c67fd..3960c96 100644
--- a/spec/gitlab_net_spec.rb
+++ b/spec/gitlab_net_spec.rb
@@ -2,7 +2,6 @@ require_relative 'spec_helper'
 require_relative '../lib/gitlab_net'
 require_relative '../lib/gitlab_access_status'
 
-
 describe GitlabNet, vcr: true do
   let(:gitlab_net) { GitlabNet.new }
   let(:changes) { ['0000000000000000000000000000000000000000 92d0970eefd7acb6d548878925ce2208cfe2d2ec refs/heads/branch4'] }
@@ -97,20 +96,33 @@ describe GitlabNet, vcr: true do
 
   describe :merge_request_urls do
     let(:gl_repository) { "project-1" }
-    let(:repo_path) { "/path/to/my/repo.git" }
     let(:changes) { "123456 789012 refs/heads/test\n654321 210987 refs/tags/tag" }
     let(:encoded_changes) { "123456%20789012%20refs/heads/test%0A654321%20210987%20refs/tags/tag" }
 
     it "sends the given arguments as encoded URL parameters" do
-      gitlab_net.should_receive(:get).with("#{host}/merge_request_urls?project=#{repo_path}&changes=#{encoded_changes}&gl_repository=#{gl_repository}")
+      gitlab_net.should_receive(:get).with("#{host}/merge_request_urls?project=#{project}&changes=#{encoded_changes}&gl_repository=#{gl_repository}")
 
-      gitlab_net.merge_request_urls(gl_repository, repo_path, changes)
+      gitlab_net.merge_request_urls(gl_repository, project, changes)
     end
 
     it "omits the gl_repository parameter if it's nil" do
-      gitlab_net.should_receive(:get).with("#{host}/merge_request_urls?project=#{repo_path}&changes=#{encoded_changes}")
+      gitlab_net.should_receive(:get).with("#{host}/merge_request_urls?project=#{project}&changes=#{encoded_changes}")
+
+      gitlab_net.merge_request_urls(nil, project, changes)
+    end
+
+    it "returns an empty array when the result cannot be parsed as JSON" do
+      response = double(:response, code: '200', body: '')
+      allow(gitlab_net).to receive(:get).and_return(response)
+
+      expect(gitlab_net.merge_request_urls(gl_repository, project, changes)).to eq([])
+    end
+
+    it "returns an empty array when the result's status is not 200" do
+      response = double(:response, code: '500', body: '[{}]')
+      allow(gitlab_net).to receive(:get).and_return(response)
 
-      gitlab_net.merge_request_urls(nil, repo_path, changes)
+      expect(gitlab_net.merge_request_urls(gl_repository, project, changes)).to eq([])
     end
   end