Correctly handle job-level errors

updater/lib/dependabot/updater/error_handler.rb

@@ -33,133 +33,171 @@ def initialize(service:, job:)
         @job = job
       end
 
-      # rubocop:disable Metrics/AbcSize
-      # rubocop:disable Metrics/MethodLength
-      def handle_dependabot_error(error:, dependency:)
+      # This method handles errors where there is a dependency in the current
+      # context. This should be used by preference where possible.
+      def handle_dependency_error(error:, dependency:)
         # If the error is fatal for the run, we should re-raise it rather than
         # pass it back to the service.
         raise error if RUN_HALTING_ERRORS.keys.any? { |err| error.is_a?(err) }
 
-        error_details =
-          case error
-          when Dependabot::DependencyFileNotResolvable
-            {
-              "error-type": "dependency_file_not_resolvable",
-              "error-detail": { message: error.message }
-            }
-          when Dependabot::DependencyFileNotEvaluatable
-            {
-              "error-type": "dependency_file_not_evaluatable",
-              "error-detail": { message: error.message }
-            }
-          when Dependabot::GitDependenciesNotReachable
-            {
-              "error-type": "git_dependencies_not_reachable",
-              "error-detail": { "dependency-urls": error.dependency_urls }
-            }
-          when Dependabot::GitDependencyReferenceNotFound
-            {
-              "error-type": "git_dependency_reference_not_found",
-              "error-detail": { dependency: error.dependency }
-            }
-          when Dependabot::PrivateSourceAuthenticationFailure
-            {
-              "error-type": "private_source_authentication_failure",
-              "error-detail": { source: error.source }
-            }
-          when Dependabot::PrivateSourceTimedOut
-            {
-              "error-type": "private_source_timed_out",
-              "error-detail": { source: error.source }
-            }
-          when Dependabot::PrivateSourceCertificateFailure
-            {
-              "error-type": "private_source_certificate_failure",
-              "error-detail": { source: error.source }
-            }
-          when Dependabot::MissingEnvironmentVariable
-            {
-              "error-type": "missing_environment_variable",
-              "error-detail": {
-                "environment-variable": error.environment_variable
-              }
-            }
-          when Dependabot::GoModulePathMismatch
-            {
-              "error-type": "go_module_path_mismatch",
-              "error-detail": {
-                "declared-path": error.declared_path,
-                "discovered-path": error.discovered_path,
-                "go-mod": error.go_mod
-              }
-            }
-          when Dependabot::NotImplemented
-            {
-              "error-type": "not_implemented",
-              "error-detail": {
-                message: error.message
-              }
-            }
-          when Dependabot::SharedHelpers::HelperSubprocessFailed
-            # If a helper subprocess has failed the error may include sensitive
-            # info such as file contents or paths. This information is already
-            # in the job logs, so we send a breadcrumb to Sentry to retrieve those
-            # instead.
-            msg = "Subprocess #{error.raven_context[:fingerprint]} failed to run. Check the job logs for error messages"
-            sanitized_error = SubprocessFailed.new(msg, raven_context: error.raven_context)
-            sanitized_error.set_backtrace(error.backtrace)
-            service.capture_exception(error: sanitized_error, job: job)
-
-            { "error-type": "unknown_error" }
-          when *Octokit::RATE_LIMITED_ERRORS
-            # If we get a rate-limited error we let dependabot-api handle the
-            # retry by re-enqueing the update job after the reset
-            {
-              "error-type": "octokit_rate_limited",
-              "error-detail": {
-                "rate-limit-reset": error.response_headers["X-RateLimit-Reset"]
-              }
-            }
-          else
-            service.capture_exception(
-              error: error,
-              job: job,
-              dependency: dependency
-            )
-            { "error-type": "unknown_error" }
-          end
-
+        error_details = error_details_for(error, dependency: dependency)
         service.record_update_job_error(
           error_type: error_details.fetch(:"error-type"),
           error_details: error_details[:"error-detail"],
           dependency: dependency
         )
 
-        log_error(
+        log_dependency_error(
           dependency: dependency,
           error: error,
           error_type: error_details.fetch(:"error-type"),
           error_detail: error_details.fetch(:"error-detail", nil)
         )
       end
-      # rubocop:enable Metrics/MethodLength
-      # rubocop:enable Metrics/AbcSize
 
-      def log_error(dependency:, error:, error_type:, error_detail: nil)
+      # Provides logging for errors that occur when processing a dependency
+      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 }
+          log_unknown_error_with_backtrace(error)
         else
           Dependabot.logger.info(
             "Handled error whilst updating #{dependency.name}: #{error_type} #{error_detail}"
           )
         end
       end
 
+      # This method handles errors where there is no dependency in the current
+      # context.
+      def handle_job_error(error:)
+        # If the error is fatal for the run, we should re-raise it rather than
+        # pass it back to the service.
+        raise error if RUN_HALTING_ERRORS.keys.any? { |err| error.is_a?(err) }
+
+        error_details = error_details_for(error)
+        service.record_update_job_error(
+          error_type: error_details.fetch(:"error-type"),
+          error_details: error_details[:"error-detail"]
+        )
+        log_job_error(
+          error: error,
+          error_type: error_details.fetch(:"error-type"),
+          error_detail: error_details.fetch(:"error-detail", nil)
+        )
+      end
+
+      # Provides logging for errors that occur outside of a dependency context
+      def log_job_error(error:, error_type:, error_detail: nil)
+        if error_type == "unknown_error"
+          Dependabot.logger.error "Error processing job (#{error.class.name})"
+          log_unknown_error_with_backtrace(error)
+        else
+          Dependabot.logger.info(
+            "Handled error whilst processing job: #{error_type} #{error_detail}"
+          )
+        end
+      end
+
       private
 
       attr_reader :service, :job
+
+      # This method accepts an error class and returns an appropriate `error_details` hash
+      # to be reported to the backend service.
+      def error_details_for(error, dependency: nil) # rubocop:disable Metrics/MethodLength
+        case error
+        when Dependabot::DependencyFileNotResolvable
+          {
+            "error-type": "dependency_file_not_resolvable",
+            "error-detail": { message: error.message }
+          }
+        when Dependabot::DependencyFileNotEvaluatable
+          {
+            "error-type": "dependency_file_not_evaluatable",
+            "error-detail": { message: error.message }
+          }
+        when Dependabot::GitDependenciesNotReachable
+          {
+            "error-type": "git_dependencies_not_reachable",
+            "error-detail": { "dependency-urls": error.dependency_urls }
+          }
+        when Dependabot::GitDependencyReferenceNotFound
+          {
+            "error-type": "git_dependency_reference_not_found",
+            "error-detail": { dependency: error.dependency }
+          }
+        when Dependabot::PrivateSourceAuthenticationFailure
+          {
+            "error-type": "private_source_authentication_failure",
+            "error-detail": { source: error.source }
+          }
+        when Dependabot::PrivateSourceTimedOut
+          {
+            "error-type": "private_source_timed_out",
+            "error-detail": { source: error.source }
+          }
+        when Dependabot::PrivateSourceCertificateFailure
+          {
+            "error-type": "private_source_certificate_failure",
+            "error-detail": { source: error.source }
+          }
+        when Dependabot::MissingEnvironmentVariable
+          {
+            "error-type": "missing_environment_variable",
+            "error-detail": {
+              "environment-variable": error.environment_variable
+            }
+          }
+        when Dependabot::GoModulePathMismatch
+          {
+            "error-type": "go_module_path_mismatch",
+            "error-detail": {
+              "declared-path": error.declared_path,
+              "discovered-path": error.discovered_path,
+              "go-mod": error.go_mod
+            }
+          }
+        when Dependabot::NotImplemented
+          {
+            "error-type": "not_implemented",
+            "error-detail": {
+              message: error.message
+            }
+          }
+        when Dependabot::SharedHelpers::HelperSubprocessFailed
+          # If a helper subprocess has failed the error may include sensitive
+          # info such as file contents or paths. This information is already
+          # in the job logs, so we send a breadcrumb to Sentry to retrieve those
+          # instead.
+          msg = "Subprocess #{error.raven_context[:fingerprint]} failed to run. Check the job logs for error messages"
+          sanitized_error = SubprocessFailed.new(msg, raven_context: error.raven_context)
+          sanitized_error.set_backtrace(error.backtrace)
+          service.capture_exception(error: sanitized_error, job: job)
+
+          { "error-type": "unknown_error" }
+        when *Octokit::RATE_LIMITED_ERRORS
+          # If we get a rate-limited error we let dependabot-api handle the
+          # retry by re-enqueing the update job after the reset
+          {
+            "error-type": "octokit_rate_limited",
+            "error-detail": {
+              "rate-limit-reset": error.response_headers["X-RateLimit-Reset"]
+            }
+          }
+        else
+          service.capture_exception(
+            error: error,
+            job: job,
+            dependency: dependency
+          )
+          { "error-type": "unknown_error" }
+        end
+      end
+
+      def log_unknown_error_with_backtrace(error)
+        Dependabot.logger.error error.message
+        error.backtrace.each { |line| Dependabot.logger.error line }
+      end
     end
   end
 end

updater/lib/dependabot/updater/group_update_creation.rb

@@ -85,7 +85,7 @@ def create_change_for(lead_dependency, updated_dependencies, dependency_files, d
           change_source: dependency_group
         )
       rescue Dependabot::InconsistentRegistryResponse => e
-        error_handler.log_error(
+        error_handler.log_dependency_error(
           dependency: lead_dependency,
           error: e,
           error_type: "inconsistent_registry_response",
@@ -94,9 +94,7 @@ def create_change_for(lead_dependency, updated_dependencies, dependency_files, d
 
         false
       rescue StandardError => e
-        raise if ErrorHandler::RUN_HALTING_ERRORS.keys.any? { |err| e.is_a?(err) }
-
-        error_handler.handle_dependabot_error(error: e, dependency: lead_dependency)
+        error_handler.handle_dependency_error(error: e, dependency: lead_dependency)
 
         false
       end
@@ -144,15 +142,15 @@ def compile_updates_for(dependency, dependency_files) # rubocop:disable Metrics/
 
         updated_deps
       rescue Dependabot::InconsistentRegistryResponse => e
-        error_handler.log_error(
+        error_handler.log_dependency_error(
           dependency: dependency,
           error: e,
           error_type: "inconsistent_registry_response",
           error_detail: e.message
         )
         [] # return an empty set
       rescue StandardError => e
-        error_handler.handle_dependabot_error(error: e, dependency: dependency)
+        error_handler.handle_dependency_error(error: e, dependency: dependency)
         [] # return an empty set
       end
 

updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb

@@ -45,13 +45,7 @@ def perform
             begin
               service.create_pull_request(dependency_change, dependency_snapshot.base_commit_sha)
             rescue StandardError => e
-              raise if ErrorHandler::RUN_HALTING_ERRORS.keys.any? { |err| e.is_a?(err) }
-
-              # FIXME: This will result in us reporting a the group name as a dependency name
-              #
-              # In future we should modify this method to accept both dependency and group
-              # so the downstream error handling can tag things appropriately.
-              error_handler.handle_dependabot_error(error: e, dependency: group)
+              error_handler.handle_job_error(error: e)
             end
           else
             Dependabot.logger.info("Nothing to update for Dependency Group: '#{group.name}'")

updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb

@@ -55,14 +55,14 @@ def perform
         def check_and_create_pr_with_error_handling(dependency)
           check_and_create_pull_request(dependency)
         rescue Dependabot::InconsistentRegistryResponse => e
-          error_handler.log_error(
+          error_handler.log_dependency_error(
             dependency: dependency,
             error: e,
             error_type: "inconsistent_registry_response",
             error_detail: e.message
           )
         rescue StandardError => e
-          error_handler.handle_dependabot_error(error: e, dependency: dependency)
+          error_handler.handle_dependency_error(error: e, dependency: dependency)
         end
 
         # rubocop:disable Metrics/AbcSize

updater/lib/dependabot/updater/operations/refresh_group_update_pull_request.rb

@@ -84,13 +84,7 @@ def upsert_pull_request_with_error_handling(dependency_change)
             close_pull_request(reason: :up_to_date)
           end
         rescue StandardError => e
-          raise if ErrorHandler::RUN_HALTING_ERRORS.keys.any? { |err| e.is_a?(err) }
-
-          # FIXME: This will result in us reporting a the group name as a dependency name
-          #
-          # In future we should modify this method to accept both dependency and group
-          # so the downstream error handling can tag things appropriately.
-          error_handler.handle_dependabot_error(error: e, dependency: dependency_change.dependency_group)
+          error_handler.handle_job_error(error: e)
         end
 
         # Having created the dependency_change, we need to determine the right strategy to apply it to the project:

updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb

@@ -39,7 +39,7 @@ def perform
           dependency = dependencies.last
           check_and_update_pull_request(dependencies)
         rescue StandardError => e
-          error_handler.handle_dependabot_error(error: e, dependency: dependency)
+          error_handler.handle_dependency_error(error: e, dependency: dependency)
         end
 
         private

updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb

@@ -36,7 +36,7 @@ def perform
           dependency = dependencies.last
           check_and_update_pull_request(dependencies)
         rescue StandardError => e
-          error_handler.handle_dependabot_error(error: e, dependency: dependency)
+          error_handler.handle_dependency_error(error: e, dependency: dependency)
         end
 
         private

updater/lib/dependabot/updater/operations/update_all_versions.rb

@@ -58,14 +58,14 @@ def dependencies
         def check_and_create_pr_with_error_handling(dependency)
           check_and_create_pull_request(dependency)
         rescue Dependabot::InconsistentRegistryResponse => e
-          error_handler.log_error(
+          error_handler.log_dependency_error(
             dependency: dependency,
             error: e,
             error_type: "inconsistent_registry_response",
             error_detail: e.message
           )
         rescue StandardError => e
-          error_handler.handle_dependabot_error(error: e, dependency: dependency)
+          error_handler.handle_dependency_error(error: e, dependency: dependency)
         end
 
         # rubocop:disable Metrics/AbcSize