Merge branch 'rescue-ci-minutes-email-notification-errors' into 'master'

Gracefully track errors raised by sending CI minutes notifications

See merge request gitlab-org/gitlab!71815

ee/app/services/ci/minutes/update_project_and_namespace_usage_service.rb

@@ -37,10 +37,20 @@ module Ci
 
       private
 
+      # We want to rescue and track exceptions to ensure the service
+      # remains idempotent. Sending email notifications is not as critical
+      # as this service idempotency.
+      # TODO: we should move this to be an async job.
+      # https://gitlab.com/gitlab-org/gitlab/-/issues/335885
       def send_minutes_email_notification
         # `perform reset` on `project` because `Namespace#namespace_statistics` will otherwise return stale data.
         # TODO(issue 335885): Remove @project
         ::Ci::Minutes::EmailNotificationService.new(@project.reset).execute if ::Gitlab.com?
+      rescue StandardError => e
+        ::Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e,
+          project_id: @project_id,
+          namespace_id: @namespace_id,
+          build_id: @build_id)
       end
 
       def legacy_track_usage_of_monthly_minutes(consumption)

ee/app/workers/ci/minutes/update_project_and_namespace_usage_worker.rb

@@ -9,6 +9,10 @@ module Ci
       urgency :low
       data_consistency :always # primarily performs writes
 
+      # Ensure with retries we stay within the IDEMPOTENCY_CACHE_TTL
+      # used by the service object.
+      sidekiq_options retry: 3
+
       def perform(consumption, project_id, namespace_id, build_id)
         ::Ci::Minutes::UpdateProjectAndNamespaceUsageService
           .new(project_id, namespace_id, build_id)

ee/spec/services/ci/minutes/update_project_and_namespace_usage_service_spec.rb

@@ -96,6 +96,22 @@ RSpec.describe Ci::Minutes::UpdateProjectAndNamespaceUsageService do
 
           subject
         end
+
+        context 'when an error is raised by the email notification' do
+          before do
+            allow_next_instance_of(Ci::Minutes::EmailNotificationService) do |service|
+              allow(service).to receive(:execute).and_raise(StandardError)
+            end
+          end
+
+          it 'rescues and tracks the exception' do
+            expect(::Gitlab::ErrorTracking)
+              .to receive(:track_and_raise_for_dev_exception)
+              .with(an_instance_of(StandardError), project_id: project.id, namespace_id: namespace.id, build_id: build.id)
+
+            subject
+          end
+        end
       end
 
       context 'when not on .com' do

spec/workers/every_sidekiq_worker_spec.rb

@@ -161,6 +161,7 @@ RSpec.describe 'Every Sidekiq worker' do
         'Ci::DropPipelineWorker' => 3,
         'Ci::InitialPipelineProcessWorker' => 3,
         'Ci::MergeRequests::AddTodoWhenBuildFailsWorker' => 3,
+        'Ci::Minutes::UpdateProjectAndNamespaceUsageWorker' => 3,
         'Ci::PipelineArtifacts::CoverageReportWorker' => 3,
         'Ci::PipelineArtifacts::CreateQualityReportWorker' => 3,
         'Ci::PipelineBridgeStatusWorker' => 3,