Commit 2036f9a1 authored by Fabio Pitino's avatar Fabio Pitino

Gracefully track errors raised by sending CI minutes notifications

Sometimes while calculating the notification level to be sent to
the user we may see some errors. For example related to a project
being delete in the meantime.

We are rescuing and track such errors since we need to guarantee
that the service which updates CI minutes remains idempotent.

Changelog: fixed
EE: true
parent 542ca3dc
......@@ -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)
......
......@@ -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)
......
......@@ -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
......
......@@ -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,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment