fail sooner when PR creation data is incomplete (#9888)

updater/lib/dependabot/service.rb

@@ -8,6 +8,7 @@
 require "dependabot/api_client"
 require "dependabot/errors"
 require "dependabot/opentelemetry"
+require "dependabot/experiments"
 
 # This class provides an output adapter for the Dependabot Service which manages
 # communication with the private API as well as consolidated error handling.
@@ -20,6 +21,21 @@ class Service
     extend T::Sig
     extend Forwardable
 
+    class InvalidUpdatedDependencies < DependabotError
+      extend T::Sig
+
+      sig { params(deps_no_previous_version: T::Array[String], deps_no_change: T::Array[String]).void }
+      def initialize(deps_no_previous_version:, deps_no_change:)
+        msg = ""
+        if deps_no_previous_version.any?
+          msg += "Previous version was not provided for: '#{deps_no_previous_version.join(', ')}' "
+        end
+        msg += "No requirements change for: '#{deps_no_change.join(', ')}'" if deps_no_change.any?
+
+        super(msg)
+      end
+    end
+
     sig { returns(T::Array[T.untyped]) }
     attr_reader :pull_requests
 
@@ -48,6 +64,10 @@ def wait_for_calls_to_finish
 
     sig { params(dependency_change: Dependabot::DependencyChange, base_commit_sha: String).void }
     def create_pull_request(dependency_change, base_commit_sha)
+      if Experiments.enabled?("dependency_change_validation")
+        check_dependencies_have_previous_version(dependency_change.updated_dependencies)
+      end
+
       if Experiments.enabled?("threaded_metadata")
         @threads << Thread.new { client.create_pull_request(dependency_change, base_commit_sha) }
       else
@@ -228,5 +248,23 @@ def truncate(string, max: 120)
       snip = max - 3
       string.length > max ? "#{string[0...snip]}..." : string
     end
+
+    sig { params(updated_dependencies: T::Array[Dependabot::Dependency]).void }
+    def check_dependencies_have_previous_version(updated_dependencies)
+      return if updated_dependencies.all? { |d| requirements_changed?(d) }
+      return if updated_dependencies.all?(&:previous_version)
+
+      deps_no_previous_version = updated_dependencies.reject(&:previous_version)
+      deps_no_change = updated_dependencies.reject { |d| requirements_changed?(d) }
+      raise InvalidUpdatedDependencies.new(
+        deps_no_previous_version: deps_no_previous_version.map(&:name),
+        deps_no_change: deps_no_change.map(&:name)
+      )
+    end
+
+    sig { params(dependency: Dependabot::Dependency).returns(T::Boolean) }
+    def requirements_changed?(dependency)
+      (dependency.requirements - T.must(dependency.previous_requirements)).any?
+    end
   end
 end

updater/spec/dependabot/service_spec.rb

@@ -10,6 +10,7 @@
 require "dependabot/errors"
 require "dependabot/pull_request_creator"
 require "dependabot/service"
+require "dependabot/experiments"
 
 RSpec.describe Dependabot::Service do
   subject(:service) { described_class.new(client: mock_client) }
@@ -94,8 +95,6 @@
             commit_message: "Commit message"
           )
         )
-
-      service.create_pull_request(dependency_change, base_sha)
     end
   end
 
@@ -221,15 +220,46 @@
   describe "#create_pull_request" do
     include_context :a_pr_was_created
 
+    before do
+      Dependabot::Experiments.register("dependency_change_validation", true)
+    end
+
     it "delegates to @client" do
+      service.create_pull_request(dependency_change, base_sha)
+
       expect(mock_client)
         .to have_received(:create_pull_request).with(dependency_change, base_sha)
     end
 
     it "memoizes a shorthand summary of the PR" do
+      service.create_pull_request(dependency_change, base_sha)
+
       expect(service.pull_requests)
         .to eql([["dependabot-fortran ( from 1.7.0 to 1.8.0 ), dependabot-pascal ( from 2.7.0 to 2.8.0 )", :created]])
     end
+
+    context "when the change is missing a previous version and there's no change in requirements" do
+      let(:dependencies) do
+        [
+          Dependabot::Dependency.new(
+            name: "dependabot-fortran",
+            package_manager: "bundler",
+            version: "1.8.0",
+            requirements: [
+              { file: "Gemfile", requirement: "~> 1.8.0", groups: [], source: nil }
+            ],
+            previous_requirements: [
+              { file: "Gemfile", requirement: "~> 1.8.0", groups: [], source: nil }
+            ]
+          )
+        ]
+      end
+
+      it "raises an InvalidUpdatedDependencies error" do
+        expect { service.create_pull_request(dependency_change, base_sha) }
+          .to raise_error(Dependabot::Service::InvalidUpdatedDependencies)
+      end
+    end
   end
 
   describe "#update_pull_request" do
@@ -488,6 +518,8 @@
       include_context :a_pr_was_created
 
       it "includes the summary of the created PR" do
+        service.create_pull_request(dependency_change, base_sha)
+
         expect(service.summary)
           .to include("created",
                       "dependabot-fortran ( from 1.7.0 to 1.8.0 ), dependabot-pascal ( from 2.7.0 to 2.8.0 )")
@@ -563,6 +595,10 @@
       include_context :an_error_was_reported
       include_context :a_dependency_error_was_reported
 
+      before do
+        service.create_pull_request(dependency_change, base_sha)
+      end
+
       it "includes the summary of the created PR" do
         expect(service.summary)
           .to include("created",