summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-09-03 23:34:52 -0700
committerStan Hu <stanhu@gmail.com>2019-09-05 21:42:14 -0700
commitc34240d26fdcf447ee86172a81c0fc56fcaf9cbc (patch)
treef1d74946d5725d2df09c5845f76debc5851adc36
parent13227500f29d8a74c77cba23b7dfdb4169222821 (diff)
downloadgitlab-ce-sh-add-sidekiq-logging-for-bad-ci.tar.gz
Log errors for failed pipeline creation in PostReceivesh-add-sidekiq-logging-for-bad-ci
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
-rw-r--r--app/services/git/base_hooks_service.rb28
-rw-r--r--changelogs/unreleased/sh-add-sidekiq-logging-for-bad-ci.yml5
-rw-r--r--spec/services/git/branch_push_service_spec.rb14
3 files changed, 46 insertions, 1 deletions
diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb
index 47c308c8280..35a4d2015fa 100644
--- a/app/services/git/base_hooks_service.rb
+++ b/app/services/git/base_hooks_service.rb
@@ -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
diff --git a/changelogs/unreleased/sh-add-sidekiq-logging-for-bad-ci.yml b/changelogs/unreleased/sh-add-sidekiq-logging-for-bad-ci.yml
new file mode 100644
index 00000000000..b334355cab6
--- /dev/null
+++ b/changelogs/unreleased/sh-add-sidekiq-logging-for-bad-ci.yml
@@ -0,0 +1,5 @@
+---
+title: Log errors for failed pipeline creation in PostReceive
+merge_request: 32633
+author:
+type: other
diff --git a/spec/services/git/branch_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb
index d9e607cd251..c3a4f3dbe3f 100644
--- a/spec/services/git/branch_push_service_spec.rb
+++ b/spec/services/git/branch_push_service_spec.rb
@@ -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