Merge pull request #3493 from GiriB/azure_pr_updater_error_handling

Add error handling for failure while updating Pull request in azure

common/lib/dependabot/clients/azure.rb

@@ -201,9 +201,11 @@ def update_ref(branch_name, old_commit, new_commit)
           }
         ]
 
-        post(source.api_endpoint + source.organization + "/" + source.project +
-          "/_apis/git/repositories/" + source.unscoped_repo +
-          "/refs?api-version=5.0", content.to_json)
+        response = post(source.api_endpoint + source.organization + "/" + source.project +
+                        "/_apis/git/repositories/" + source.unscoped_repo +
+                        "/refs?api-version=5.0", content.to_json)
+
+        JSON.parse(response.body).fetch("value").first
       end
       # rubocop:enable Metrics/ParameterLists
 

common/lib/dependabot/pull_request_updater/azure.rb

@@ -6,6 +6,8 @@
 module Dependabot
   class PullRequestUpdater
     class Azure
+      class PullRequestUpdateFailed < Dependabot::DependabotError; end
+
       OBJECT_ID_FOR_BRANCH_DELETE = "0000000000000000000000000000000000000000"
 
       attr_reader :source, :files, :base_commit, :old_commit, :credentials,
@@ -55,9 +57,11 @@ def update_source_branch
         # 1) Push the file changes to a newly created temporary branch (from base commit)
         new_commit = create_temp_branch
         # 2) Update PR source branch to point to the temp branch head commit.
-        update_branch(source_branch_name, old_source_branch_commit, new_commit)
+        response = update_branch(source_branch_name, old_source_branch_commit, new_commit)
         # 3) Delete temp branch
         update_branch(temp_branch_name, new_commit, OBJECT_ID_FOR_BRANCH_DELETE)
+
+        raise PullRequestUpdateFailed, response.fetch("customMessage", nil) unless response.fetch("success", false)
       end
 
       def pull_request

common/spec/dependabot/clients/azure_spec.rb

@@ -188,7 +188,7 @@
     it "sends update branch request with old and new commit id" do
       stub_request(:post, update_ref_url).
         with(basic_auth: [username, password]).
-        to_return(status: 200)
+        to_return(status: 200, body: fixture("azure", "update_ref.json"))
 
       update_ref
 

common/spec/dependabot/pull_request_updater/azure_spec.rb

@@ -113,57 +113,71 @@
       end
     end
 
-    it "updates source branch head commit in AzureDevOps" do
-      stub_request(:get, source_branch_commits_url).
-        to_return(status: 200,
-                  body: fixture("azure", "commits.json"),
-                  headers: json_header)
-      stub_request(:get, repo_contents_tree_url).
-        to_return(status: 200,
-                  body: fixture("azure", "repo_contents_treeroot.json"),
-                  headers: json_header)
-      stub_request(:get, repo_contents_url).
-        to_return(status: 200,
-                  body: fixture("azure", "repo_contents.json"),
-                  headers: json_header)
-      stub_request(:post, create_commit_url).
-        with(body: {
-               refUpdates: [
-                 { name: "refs/heads/#{temp_branch}", oldObjectId: base_commit }
-               ],
-               commits: [
-                 {
-                   comment: commit_message,
-                   changes: files.map do |file|
-                     {
-                       changeType: "edit",
-                       item: { path: file.path },
-                       newContent: {
-                         content: Base64.encode64(file.content),
-                         contentType: "base64encoded"
+    context "tries updating source branch head commit in AzureDevOps" do
+      before do
+        stub_request(:get, source_branch_commits_url).
+          to_return(status: 200,
+                    body: fixture("azure", "commits.json"),
+                    headers: json_header)
+        stub_request(:get, repo_contents_tree_url).
+          to_return(status: 200,
+                    body: fixture("azure", "repo_contents_treeroot.json"),
+                    headers: json_header)
+        stub_request(:get, repo_contents_url).
+          to_return(status: 200,
+                    body: fixture("azure", "repo_contents.json"),
+                    headers: json_header)
+        stub_request(:post, create_commit_url).
+          with(body: {
+                 refUpdates: [
+                   { name: "refs/heads/#{temp_branch}", oldObjectId: base_commit }
+                 ],
+                 commits: [
+                   {
+                     comment: commit_message,
+                     changes: files.map do |file|
+                       {
+                         changeType: "edit",
+                         item: { path: file.path },
+                         newContent: {
+                           content: Base64.encode64(file.content),
+                           contentType: "base64encoded"
+                         }
                        }
-                     }
-                   end
-                 }
-               ]
-             }).
-        to_return(status: 201,
-                  body: fixture("azure", "create_new_branch.json"),
-                  headers: json_header)
-      stub_request(:post, branch_update_url).
-        to_return(status: 201)
-
-      allow(updater).to receive(:temp_branch_name).and_return(temp_branch)
-      updater.update
-
-      expect(WebMock).
-        to(
-          have_requested(:post, branch_update_url).
-              with(body: [
-                { name: "refs/heads/#{source_branch}", oldObjectId: source_branch_old_commit,
-                  newObjectId: source_branch_new_commit }
-              ].to_json)
-        )
+                     end
+                   }
+                 ]
+               }).
+          to_return(status: 201,
+                    body: fixture("azure", "create_new_branch.json"),
+                    headers: json_header)
+
+        allow(updater).to receive(:temp_branch_name).and_return(temp_branch)
+      end
+
+      it "sends request to AzureDevOps to update source branch head commit" do
+        stub_request(:post, branch_update_url).
+          to_return(status: 201, body: fixture("azure", "update_ref.json"))
+
+        allow(updater).to receive(:temp_branch_name).and_return(temp_branch)
+        updater.update
+
+        expect(WebMock).
+          to(
+            have_requested(:post, branch_update_url).
+                with(body: [
+                  { name: "refs/heads/#{source_branch}", oldObjectId: source_branch_old_commit,
+                    newObjectId: source_branch_new_commit }
+                ].to_json)
+          )
+      end
+
+      it "raises a helpful error when source branch update is unsuccessful" do
+        stub_request(:post, branch_update_url).
+          to_return(status: 200, body: fixture("azure", "update_ref_failed.json"))
+
+        expect { updater.update }.to raise_error(Dependabot::PullRequestUpdater::Azure::PullRequestUpdateFailed)
+      end
     end
   end
 end

common/spec/fixtures/azure/update_ref.json

@@ -0,0 +1,14 @@
+{
+    "value": [
+        {
+            "repositoryId": "d3d1760b-311c-4175-a726-20dfc6a7f885",
+            "name": "refs/heads/vsts-api-sample/answer-woman-flame",
+            "oldObjectId": "ggf0dcb632e11e8e71f433956183349746fec562",
+            "newObjectId": "ffe9cba521f00d7f60e322845072238635edb451",
+            "isLocked": false,
+            "updateStatus": "succeeded",
+            "success": true
+        }
+    ],
+    "count": 1
+}

common/spec/fixtures/azure/update_ref_failed.json

@@ -0,0 +1,14 @@
+{
+    "value": [
+        {
+            "repositoryId": "d3d1760b-311c-4175-a726-20dfc6a7f885",
+            "name": "refs/heads/vsts-api-sample/answer-woman-flame",
+            "oldObjectId": "ggf0dcb632e11e8e71f433956183349746fec562",
+            "customMessage": "The user does not have force push permissions",
+            "isLocked": false,
+            "updateStatus": "failed",
+            "success": false
+        }
+    ],
+    "count": 1
+}