Added record update job error api back to capture unknown errors (#8144)

* added update job error api back to file fetcher

* added update job error api back to update_files_commands

* added update job error api back to error_handler

* added update job error api back to base command

Co-authored-by: Ankit Kumar Honey <honeyankit@github.com>

* Fixed the lint issue by removing unused method argument

---------

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>

updater/lib/dependabot/base_command.rb

@@ -64,6 +64,13 @@ def run
     def handle_exception(err)
       Dependabot.logger.error(err.message)
       err.backtrace.each { |line| Dependabot.logger.error(line) }
+      service.capture_exception(error: err, job: job)
+      service.record_update_job_error(error_type: "unknown_error", error_details: { message: err.message })
+      # We don't set this flag in GHES because there older GHES version does not support reporting unknown errors.
+      handle_unknown_error(err) if Experiments.enabled?(:record_update_job_unknown_error)
+    end
+
+    def handle_unknown_error(err)
       error_details = {
         "error-class" => err.class.to_s,
         "error-message" => err.message,
@@ -73,8 +80,6 @@ def handle_exception(err)
         "job-dependencies" => job.dependencies,
         "job-dependency_group" => job.dependency_groups
       }.compact
-
-      service.capture_exception(error: err, job: job)
       service.record_update_job_unknown_error(error_type: "updater_error", error_details: error_details)
       service.increment_metric("updater.update_job_unknown_error", tags: {
         package_manager: job.package_manager,

updater/lib/dependabot/file_fetcher_command.rb

@@ -217,17 +217,19 @@ def log_error(error)
     end
 
     def record_error(error_details)
-      if error_details[:"error-type"] == "file_fetcher_error"
-        service.record_update_job_unknown_error(
-          error_type: error_details.fetch(:"error-type"),
-          error_details: error_details[:"error-detail"]
-        )
-      else
-        service.record_update_job_error(
-          error_type: error_details.fetch(:"error-type"),
-          error_details: error_details[:"error-detail"]
-        )
-      end
+      service.record_update_job_error(
+        error_type: error_details.fetch(:"error-type"),
+        error_details: error_details[:"error-detail"]
+      )
+
+      # We don't set this flag in GHES because there older GHES version does not support reporting unknown errors.
+      return unless Experiments.enabled?(:record_update_job_unknown_error)
+      return unless error_details.fetch(:"error-type") == "file_fetcher_error"
+
+      service.record_update_job_unknown_error(
+        error_type: error_details.fetch(:"error-type"),
+        error_details: error_details[:"error-detail"]
+      )
     end
 
     # Perform a debug check of connectivity to GitHub/GHES. This also ensures

updater/lib/dependabot/service.rb

@@ -49,8 +49,7 @@ def record_update_job_error(error_type:, error_details:, dependency: nil)
       client.record_update_job_error(error_type: error_type, error_details: error_details)
     end
 
-    def record_update_job_unknown_error(error_type:, error_details:, dependency: nil)
-      @errors << [error_type.to_s, dependency]
+    def record_update_job_unknown_error(error_type:, error_details:)
       client.record_update_job_unknown_error(error_type: error_type, error_details: error_details)
     end
 

updater/lib/dependabot/update_files_command.rb

@@ -58,8 +58,9 @@ def base_commit_sha
       Environment.job_definition["base_commit_sha"]
     end
 
-    # rubocop:disable Metrics/MethodLength
     # rubocop:disable Metrics/AbcSize
+    # rubocop:disable Metrics/CyclomaticComplexity
+    # rubocop:disable Metrics/MethodLength
     def handle_parser_error(error)
       # This happens if the repo gets removed after a job gets kicked off.
       # The service will handle the removal without any prompt from the updater,
@@ -152,19 +153,21 @@ def handle_parser_error(error)
           end
         end
 
-      if error_details.fetch(:"error-type") == "update_files_error"
-        service.record_update_job_unknown_error(
-          error_type: "update_files_error",
-          error_details: error_details[:"error-detail"]
-        )
-      else
-        service.record_update_job_error(
-          error_type: error_details.fetch(:"error-type"),
-          error_details: error_details[:"error-detail"]
-        )
-      end
+      service.record_update_job_error(
+        error_type: error_details.fetch(:"error-type"),
+        error_details: error_details[:"error-detail"]
+      )
+      # We don't set this flag in GHES because there older GHES version does not support reporting unknown errors.
+      return unless Experiments.enabled?(:record_update_job_unknown_error)
+      return unless error_details.fetch(:"error-type") == "update_files_error"
+
+      service.record_update_job_unknown_error(
+        error_type: error_details.fetch(:"error-type"),
+        error_details: error_details[:"error-detail"]
+      )
     end
-    # rubocop:enable Metrics/MethodLength
     # rubocop:enable Metrics/AbcSize
+    # rubocop:enable Metrics/CyclomaticComplexity
+    # rubocop:enable Metrics/MethodLength
   end
 end

updater/lib/dependabot/updater/error_handler.rb

@@ -44,14 +44,15 @@ def handle_dependency_error(error:, dependency:, dependency_group: nil)
         raise error if RUN_HALTING_ERRORS.keys.any? { |err| error.is_a?(err) }
 
         error_details = error_details_for(error, dependency: dependency, dependency_group: dependency_group)
-        if error_details.fetch(:"error-type") == "unknown_error"
-          log_unknown_error_with_backtrace(error, dependency)
-        else
-          service.record_update_job_error(
-            error_type: error_details.fetch(:"error-type"),
-            error_details: error_details[:"error-detail"],
-            dependency: dependency
-          )
+        service.record_update_job_error(
+          error_type: error_details.fetch(:"error-type"),
+          error_details: error_details[:"error-detail"],
+          dependency: dependency
+        )
+        # We don't set this flag in GHES because there older GHES version does not support reporting unknown errors.
+        if Experiments.enabled?(:record_update_job_unknown_error) &&
+           error_details.fetch(:"error-type") == "unknown_error"
+          log_unknown_error_with_backtrace(error)
         end
 
         log_dependency_error(
@@ -66,6 +67,8 @@ def handle_dependency_error(error:, dependency:, dependency_group: nil)
       def log_dependency_error(dependency:, error:, error_type:, error_detail: nil)
         if error_type == "unknown_error"
           Dependabot.logger.error "Error processing #{dependency.name} (#{error.class.name})"
+          Dependabot.logger.error error.message
+          error.backtrace.each { |line| Dependabot.logger.error line }
         else
           Dependabot.logger.info(
             "Handled error whilst updating #{dependency.name}: #{error_type} #{error_detail}"
@@ -81,13 +84,14 @@ def handle_job_error(error:, dependency_group: nil)
         raise error if RUN_HALTING_ERRORS.keys.any? { |err| error.is_a?(err) }
 
         error_details = error_details_for(error, dependency_group: dependency_group)
-        if error_details.fetch(:"error-type") == "unknown_error"
-          log_unknown_error_with_backtrace(error, dependency_group)
-        else
-          service.record_update_job_error(
-            error_type: error_details.fetch(:"error-type"),
-            error_details: error_details[:"error-detail"]
-          )
+        service.record_update_job_error(
+          error_type: error_details.fetch(:"error-type"),
+          error_details: error_details[:"error-detail"]
+        )
+        # We don't set this flag in GHES because there older GHES version does not support reporting unknown errors.
+        if Experiments.enabled?(:record_update_job_unknown_error) &&
+           error_details.fetch(:"error-type") == "unknown_error"
+          log_unknown_error_with_backtrace(error)
         end
 
         log_job_error(
@@ -101,6 +105,8 @@ def handle_job_error(error:, dependency_group: nil)
       def log_job_error(error:, error_type:, error_detail: nil)
         if error_type == "unknown_error"
           Dependabot.logger.error "Error processing job (#{error.class.name})"
+          Dependabot.logger.error error.message
+          error.backtrace.each { |line| Dependabot.logger.error line }
         else
           Dependabot.logger.info(
             "Handled error whilst processing job: #{error_type} #{error_detail}"
@@ -209,10 +215,7 @@ def error_details_for(error, dependency: nil, dependency_group: nil) # rubocop:d
         end
       end
 
-      def log_unknown_error_with_backtrace(error, dependency = nil, _dependency_group = nil)
-        Dependabot.logger.error error.message
-        error.backtrace.each { |line| Dependabot.logger.error line }
-
+      def log_unknown_error_with_backtrace(error)
         error_details = {
           "error-class" => error.class.to_s,
           "error-message" => error.message,
@@ -227,8 +230,7 @@ def log_unknown_error_with_backtrace(error, dependency = nil, _dependency_group
           package_manager: job.package_manager,
           class_name: error.class.name
         })
-        service.record_update_job_unknown_error(error_type: "unknown_error", error_details: error_details,
-                                                dependency: dependency)
+        service.record_update_job_unknown_error(error_type: "unknown_error", error_details: error_details)
       end
     end
   end

updater/spec/dependabot/file_fetcher_command_spec.rb

@@ -124,14 +124,33 @@
       end
     end
 
-    context "when the fetcher raises a file fetcher error", vcr: true do
+    context "when the fetcher raises a file fetcher error (cloud) ", vcr: true do
       before do
         allow_any_instance_of(Dependabot::Bundler::FileFetcher)
           .to receive(:commit)
           .and_raise(StandardError, "my_branch")
+        allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(true)
       end
 
-      it "tells the backend about the error (and doesn't re-raise it)" do
+      it "tells the backend about the error via update job error api (and doesn't re-raise it)" do
+        expect(api_client).to receive(:record_update_job_error).with(
+          error_type: "file_fetcher_error",
+          error_details: {
+            "error-backtrace" => an_instance_of(String),
+            "error-message" => "my_branch",
+            "error-class" => "StandardError",
+            "package-manager" => "bundler",
+            "job-id" => "123123",
+            "job-dependency_group" => []
+          }
+        )
+        expect(api_client).to receive(:record_update_job_unknown_error)
+        expect(api_client).to receive(:mark_job_as_processed)
+
+        expect { perform_job }.to output(/Error during file fetching; aborting/).to_stdout_from_any_process
+      end
+
+      it "tells the backend about the error via update job unknown error (and doesn't re-raise it)" do
         expect(api_client).to receive(:record_update_job_unknown_error).with(
           error_type: "file_fetcher_error",
           error_details: {
@@ -149,6 +168,39 @@
       end
     end
 
+    context "when the fetcher raises a file fetcher error (ghes) ", vcr: true do
+      before do
+        allow_any_instance_of(Dependabot::Bundler::FileFetcher)
+          .to receive(:commit)
+          .and_raise(StandardError, "my_branch")
+        allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(false)
+      end
+
+      it "tells the backend about the error via update job error api (and doesn't re-raise it)" do
+        expect(api_client).to receive(:record_update_job_error).with(
+          error_type: "file_fetcher_error",
+          error_details: {
+            "error-backtrace" => an_instance_of(String),
+            "error-message" => "my_branch",
+            "error-class" => "StandardError",
+            "package-manager" => "bundler",
+            "job-id" => "123123",
+            "job-dependency_group" => []
+          }
+        )
+        expect(api_client).to receive(:mark_job_as_processed)
+
+        expect { perform_job }.to output(/Error during file fetching; aborting/).to_stdout_from_any_process
+      end
+
+      it "do not tells the backend about the error" do
+        expect(api_client).to_not receive(:record_update_job_unknown_error)
+        expect(api_client).to receive(:mark_job_as_processed)
+
+        expect { perform_job }.to output(/Error during file fetching; aborting/).to_stdout_from_any_process
+      end
+    end
+
     context "when the fetcher raises a rate limited error" do
       let(:reset_at) { Time.now + 30 }
 

updater/spec/dependabot/integration_spec.rb

@@ -82,6 +82,7 @@
     allow(Dependabot::ApiClient).to receive(:new).and_return(api_client)
     allow(Dependabot::FileFetchers).to receive_message_chain(:for_package_manager, :new).and_return(file_fetcher)
     allow(Dependabot::PullRequestCreator::MessageBuilder).to receive(:new).and_return(message_builder)
+    allow(file_fetcher).to receive(:ecosystem_versions).and_return(nil)
 
     allow(Dependabot.logger).to receive(:info).and_call_original
   end
@@ -203,12 +204,17 @@
       run_job
     end
 
-    context "when there is an exception that blocks PR creation" do
+    context "when there is an exception that blocks PR creation (cloud)" do
       before do
         allow(api_client).to receive(:create_pull_request).and_raise(StandardError, "oh no!")
+        allow(Dependabot::Experiments).to receive(:enabled?).with(:record_ecosystem_versions).and_return(true)
+        allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(true)
       end
 
       it "notifies Dependabot API of the problem" do
+        expect(api_client).to receive(:record_update_job_error)
+          .with({ error_type: "unknown_error", error_details: nil })
+
         expect(api_client).to receive(:record_update_job_unknown_error)
           .with(
             { error_type: "unknown_error",
@@ -248,6 +254,44 @@
       end
     end
 
+    context "when there is an exception that blocks PR creation (ghes)" do
+      before do
+        allow(Dependabot::Experiments).to receive(:enabled?).with(:record_ecosystem_versions).and_return(false)
+        allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(false)
+        allow(api_client).to receive(:create_pull_request).and_raise(StandardError, "oh no!")
+      end
+
+      it "notifies Dependabot API of the problem" do
+        expect(api_client).to receive(:record_update_job_error)
+          .with({ error_type: "unknown_error", error_details: nil })
+
+        expect(api_client).to_not receive(:record_update_job_unknown_error)
+        expect { run_job }.to output(/oh no!/).to_stdout_from_any_process
+      end
+
+      it "indicates there was an error in the summary" do
+        expect(Dependabot.logger).not_to receive(:info).with(/Changes to Dependabot Pull Requests/)
+        expect(Dependabot.logger).to receive(:info).with(/Dependabot encountered '1' error/)
+
+        expect { run_job }.to output(/oh no!/).to_stdout_from_any_process
+      end
+
+      it "does not raise an exception" do
+        expect { run_job }.to output(/oh no!/).to_stdout_from_any_process
+      end
+
+      context "when GITHUB_ACTIONS is set" do
+        before do
+          allow(Dependabot::Environment).to receive(:github_actions?) { "true" }
+        end
+
+        it "raises an exception" do
+          expect { run_job }.to raise_error(Dependabot::RunFailure)
+            .and output(/oh no!/).to_stdout_from_any_process
+        end
+      end
+    end
+
     context "when there is an exception that does not block PR creation" do
       before do
         # Pre-populate an updater error