raise exceptions when PR creation fails

common/lib/dependabot/pull_request_creator/github.rb

@@ -42,8 +42,8 @@ def initialize(source:, branch_name:, base_commit:, credentials:,
       end
 
       def create
-        return if branch_exists?(branch_name) && unmerged_pull_request_exists?
-        return if require_up_to_date_base? && !base_commit_is_up_to_date?
+        raise "Unmerged PR exists" if branch_exists?(branch_name) && unmerged_pull_request_exists?
+        raise "Base commit is not up to date" if require_up_to_date_base? && !base_commit_is_up_to_date?
 
         create_annotated_pull_request
       rescue AnnotationError, Octokit::Error => e
@@ -111,10 +111,10 @@ def base_commit_is_up_to_date?
       def create_annotated_pull_request
         commit = create_commit
         branch = create_or_update_branch(commit)
-        return unless branch
+        raise "Unexpected PR branch error" unless branch
 
         pull_request = create_pull_request
-        return unless pull_request
+        raise "Unexpected PR creation error" unless pull_request
 
         begin
           annotate_pull_request(pull_request)
@@ -219,10 +219,7 @@ def create_or_update_branch(commit)
         # A race condition may cause GitHub to fail here, in which case we retry
         retry_count ||= 0
         retry_count += 1
-        if retry_count > 10
-          raise "Repeatedly failed to create or update branch #{branch_name} " \
-                "with commit #{commit.sha}."
-        end
+        raise if retry_count > 10
 
         sleep(rand(1..1.99))
         retry
@@ -358,9 +355,7 @@ def create_pull_request
           pr_description,
           headers: custom_headers || {}
         )
-      rescue Octokit::UnprocessableEntity => e
-        return handle_pr_creation_error(e) if e.message.include? "Error summary"
-
+      rescue Octokit::UnprocessableEntity
         # Sometimes PR creation fails with no details (presumably because the
         # details are internal). It doesn't hurt to retry in these cases, in
         # case the cause is a race.
@@ -371,18 +366,6 @@ def create_pull_request
         retry
       end
 
-      def handle_pr_creation_error(error)
-        # Ignore races that we lose
-        return if error.message.include?("pull request already exists")
-
-        # Ignore cases where the target branch has been deleted
-        return if error.message.include?("field: base") &&
-                  source.branch &&
-                  !branch_exists?(source.branch)
-
-        raise
-      end
-
       def target_branch
         source.branch || default_branch
       end

common/spec/dependabot/pull_request_creator/github_spec.rb

@@ -508,8 +508,9 @@
             .to_return(status: 200, body: "[{}]", headers: json_header)
         end
 
-        it "returns nil" do
-          expect(creator.create).to be_nil
+        it "raises a helpful error" do
+          expect { creator.create }
+            .to raise_error(StandardError, /Unmerged PR exists/)
           expect(WebMock).to_not have_requested(:post, "#{repo_api_url}/pulls")
         end
 
@@ -535,9 +536,9 @@
               )
           end
 
-          it "returns nil" do
-            expect(creator.create).to be_nil
-            expect(WebMock).to have_requested(:post, "#{repo_api_url}/pulls")
+          it "raises the error" do
+            expect { creator.create }
+              .to raise_error(Octokit::UnprocessableEntity)
           end
         end
 
@@ -579,8 +580,9 @@
           context "when `require_up_to_date_base` is true" do
             let(:require_up_to_date_base) { true }
 
-            it "does not create a PR" do
-              expect(creator.create).to be_nil
+            it "raises a helpful error" do
+              expect { creator.create }
+                .to raise_error(StandardError, /Base commit is not up to date/)
               expect(WebMock)
                 .to_not have_requested(:post, "#{repo_api_url}/pulls")
             end
@@ -795,8 +797,8 @@
                        headers: json_header)
         end
 
-        it "quietly ignores the failure" do
-          expect { creator.create }.to_not raise_error
+        it "raises the error" do
+          expect { creator.create }.to raise_error(Octokit::UnprocessableEntity)
         end
       end
     end