From 5d111d335029e6659fd5a9a6d8e04663b36fbbfd Mon Sep 17 00:00:00 2001 From: Alex Kriuchykhin Date: Fri, 25 Mar 2022 15:38:15 +0100 Subject: [PATCH] Improve error reporting in task status transition [SCI-6611] (#3952) --- app/jobs/my_module_status_consequences_job.rb | 15 +++++++++++---- app/models/my_module.rb | 13 ++++--------- app/models/my_module_status.rb | 9 +++++++++ .../repository_snapshot.rb | 8 +++++++- 4 files changed, 31 insertions(+), 14 deletions(-) diff --git a/app/jobs/my_module_status_consequences_job.rb b/app/jobs/my_module_status_consequences_job.rb index a27c547e9..cc5a78086 100644 --- a/app/jobs/my_module_status_consequences_job.rb +++ b/app/jobs/my_module_status_consequences_job.rb @@ -4,19 +4,26 @@ class MyModuleStatusConsequencesJob < ApplicationJob queue_as :high_priority def perform(my_module, my_module_status_consequences, status_changing_direction) - error_raised = false - my_module.transaction do + error = nil + my_module.transaction(requires_new: true) do my_module_status_consequences.each do |consequence| consequence.public_send(status_changing_direction, my_module) end my_module.update!(status_changing: false) + my_module.update!(last_transition_error: nil) rescue StandardError => e Rails.logger.error(e.message) Rails.logger.error(e.backtrace.join("\n")) - error_raised = true + error = if e.is_a?(MyModuleStatus::MyModuleStatusTransitionError) + e.error + else + { type: :general, message: e.message } + end + raise ActiveRecord::Rollback end - if error_raised + if error.present? my_module.my_module_status = my_module.changing_from_my_module_status + my_module.last_transition_error = error my_module.status_changing = false my_module.save! end diff --git a/app/models/my_module.rb b/app/models/my_module.rb index 4f1943d09..022b84e58 100644 --- a/app/models/my_module.rb +++ b/app/models/my_module.rb @@ -478,16 +478,11 @@ class MyModule < ApplicationRecord yield if my_module_status.my_module_status_consequences.any?(&:runs_in_background?) - MyModuleStatusConsequencesJob.perform_later( - self, - my_module_status.my_module_status_consequences.to_a, - status_changing_direction - ) + MyModuleStatusConsequencesJob + .perform_later(self, my_module_status.my_module_status_consequences.to_a, status_changing_direction) else - my_module_status.my_module_status_consequences.each do |consequence| - consequence.public_send(status_changing_direction, self) - end - update!(status_changing: false) + MyModuleStatusConsequencesJob + .perform_now(self, my_module_status.my_module_status_consequences.to_a, status_changing_direction) end end diff --git a/app/models/my_module_status.rb b/app/models/my_module_status.rb index 3315dd7d2..64e13c237 100644 --- a/app/models/my_module_status.rb +++ b/app/models/my_module_status.rb @@ -1,6 +1,15 @@ # frozen_string_literal: true class MyModuleStatus < ApplicationRecord + class MyModuleStatusTransitionError < StandardError + attr_reader :error + + def initialize(error) + @error = error + super + end + end + has_many :my_modules, dependent: :nullify has_many :my_module_status_conditions, dependent: :destroy has_many :my_module_status_consequences, dependent: :destroy diff --git a/app/models/my_module_status_consequences/repository_snapshot.rb b/app/models/my_module_status_consequences/repository_snapshot.rb index 4760d9102..01badc36d 100644 --- a/app/models/my_module_status_consequences/repository_snapshot.rb +++ b/app/models/my_module_status_consequences/repository_snapshot.rb @@ -13,7 +13,13 @@ module MyModuleStatusConsequences unless service.succeed? repository_snapshot.failed! - raise StandardError, service.errors + raise MyModuleStatus::MyModuleStatusTransitionError.new( + { + type: :repository_snapshot, + repository_id: repository_snapshot.parent_id, + message: service.errors.values.join("\n") + } + ) end snapshot = service.repository_snapshot