Log errors for failed pipeline creation in PostReceive

When a pipeline fails to create in `PostReceive`, the error is silently
discarded, making it difficult to understand why a pipeline was not
created. We now add a Sidekiq warning message for this. Adding a Sentry
exception when this happens would generate a lot of noise for invalid CI
files.

Relates to https://gitlab.com/gitlab-org/gitlab-ee/issues/14720
This commit is contained in:
Stan Hu 2019-09-03 23:34:52 -07:00
parent 13227500f2
commit c34240d26f
3 changed files with 46 additions and 1 deletions

View file

@ -57,7 +57,9 @@ module Git
Ci::CreatePipelineService
.new(project, current_user, pipeline_params)
.execute(:push, pipeline_options)
.execute!(:push, pipeline_options)
rescue Ci::CreatePipelineService::CreateError => ex
log_pipeline_errors(ex)
end
def execute_project_hooks
@ -125,5 +127,29 @@ module Git
project.mark_stuck_remote_mirrors_as_failed!
project.update_remote_mirrors
end
def log_pipeline_errors(exception)
data = {
class: self.class.name,
correlation_id: Labkit::Correlation::CorrelationId.current_id.to_s,
project_id: project.id,
project_path: project.full_path,
message: "Error creating pipeline",
errors: exception.to_s,
pipeline_params: pipeline_params
}
logger.warn(data)
end
def logger
if Sidekiq.server?
Sidekiq.logger
else
# This service runs in Sidekiq, so this shouldn't ever be
# called, but this is included just in case.
Gitlab::ProjectServiceLogger
end
end
end
end

View file

@ -0,0 +1,5 @@
---
title: Log errors for failed pipeline creation in PostReceive
merge_request: 32633
author:
type: other

View file

@ -99,6 +99,20 @@ describe Git::BranchPushService, services: true do
expect(pipeline).to be_push
expect(Gitlab::Git::BRANCH_REF_PREFIX + pipeline.ref).to eq(ref)
end
context 'when pipeline has errors' do
before do
config = YAML.dump({ test: { script: 'ls', only: ['feature'] } })
stub_ci_pipeline_yaml_file(config)
end
it 'reports an error' do
allow(Sidekiq).to receive(:server?).and_return(true)
expect(Sidekiq.logger).to receive(:warn)
expect { subject }.not_to change { Ci::Pipeline.count }
end
end
end
describe "Updates merge requests" do