diff --git a/app/jobs/publish_feed_job.rb b/app/jobs/publish_feed_job.rb index 2157b9d95..3545fe50d 100644 --- a/app/jobs/publish_feed_job.rb +++ b/app/jobs/publish_feed_job.rb @@ -19,32 +19,51 @@ def perform(podcast, pub_item) podcast.feeds.each { |feed| publish_rss(podcast, feed) } PublishingPipelineState.complete!(podcast) rescue Apple::AssetStateTimeoutError => e - PublishingPipelineState.error!(podcast) - Rails.logger.error("Asset processing timeout", {podcast_id: podcast.id, error: e.message, backtrace: e.backtrace.join("\n")}) + handle_apple_timeout_error(podcast, e) rescue => e - PublishingPipelineState.error!(podcast) - # TODO, we can remove this once we have a better way to track errors - Rails.logger.error("Error publishing podcast", {podcast_id: podcast.id, error: e.message, backtrace: e.backtrace.join("\n")}) + handle_error(podcast, e) ensure PublishingPipelineState.settle_remaining!(podcast) end def publish_apple(podcast, feed) return unless feed.publish_to_apple? - res = PublishAppleJob.do_perform(feed.apple_config) - PublishingPipelineState.publish_apple!(podcast) - res - rescue => e - NewRelic::Agent.notice_error(e) - res = PublishingPipelineState.error_apple!(podcast) - raise e if feed.apple_config.sync_blocks_rss? - res + + begin + res = PublishAppleJob.do_perform(podcast.apple_config) + PublishingPipelineState.publish_apple!(podcast) + res + rescue => e + PublishingPipelineState.error_apple!(podcast) + NewRelic::Agent.notice_error(e) + raise e + end end def publish_rss(podcast, feed) res = save_file(podcast, feed) PublishingPipelineState.publish_rss!(podcast) res + rescue => e + handle_rss_error(podcast, feed, e) + end + + def handle_apple_timeout_error(podcast, error, raise_error: true) + PublishingPipelineState.error!(podcast) + Rails.logger.error("Asset processing timeout", {podcast_id: podcast.id, error: error.message, backtrace: error&.backtrace&.join("\n")}) + raise error + end + + def handle_rss_error(podcast, feed, error) + PublishingPipelineState.error!(podcast) + Rails.logger.error("Error publishing RSS", {podcast_id: podcast.id, feed_id: feed.id, error: error.message, backtrace: error&.backtrace&.join("\n")}) + raise error + end + + def handle_error(podcast, error) + PublishingPipelineState.error!(podcast) + Rails.logger.error("Error publishing podcast", {podcast_id: podcast.id, error: error.message, backtrace: error&.backtrace&.join("\n")}) + raise error end def save_file(podcast, feed, options = {}) diff --git a/test/jobs/publish_feed_job_test.rb b/test/jobs/publish_feed_job_test.rb index 0f3a11cb5..693d975a6 100644 --- a/test/jobs/publish_feed_job_test.rb +++ b/test/jobs/publish_feed_job_test.rb @@ -4,7 +4,7 @@ let(:episode) { create(:episode, prx_uri: "/api/v1/stories/87683") } let(:podcast) { episode.podcast } let(:feed) { create(:feed, podcast: podcast, slug: "adfree") } - let(:private_feed) { create(:private_feed, podcast: podcast) } + let(:private_feed) { create(:apple_feed, podcast: podcast) } let(:job) { PublishFeedJob.new } @@ -25,71 +25,109 @@ end end - it "can process publishing a podcast" do + it "transitions to the error state upon rss error" do job.stub(:s3_client, stub_client) do - PublishFeedJob.stub(:perform_later, nil) do - PublishingPipelineState.start_pipeline!(podcast) - end - - pub_item = PublishingQueueItem.unfinished_items(podcast).first - - rss = job.perform(podcast, pub_item) - refute_nil rss - refute_nil job.put_object - assert_nil job.copy_object + PublishingPipelineState.start_pipeline!(podcast) + assert_raises(RuntimeError) { job.handle_rss_error(podcast, feed, RuntimeError.new("rss error")) } + assert_equal ["created", "error"], PublishingPipelineState.where(podcast: podcast).latest_pipelines.pluck(:status) end end - it "will skip the publishing if the pub items are mismatched" do + it "transitions to the error state upon general error" do + # Simulate some method blowing up job.stub(:s3_client, stub_client) do - PublishFeedJob.stub(:perform_later, nil) do - PublishingPipelineState.start_pipeline!(podcast) + PublishingPipelineState.start_pipeline!(podcast) + PublishingPipelineState.stub(:publish_rss!, ->(*, **) { raise "some general" }) do + assert_raises(RuntimeError) { job.handle_rss_error(podcast, feed, RuntimeError.new("some general")) } + assert_equal ["created", "error"], PublishingPipelineState.where(podcast: podcast).latest_pipelines.pluck(:status) end - - pub_item = PublishingQueueItem.create(podcast: podcast) - assert job.mismatched_publishing_item?(podcast, pub_item) - assert_equal :mismatched, job.perform(podcast, pub_item) end end - it "will skip the publishing if the pub items are null" do - job.stub(:s3_client, stub_client) do - assert PublishingQueueItem.unfinished_items(podcast).empty? + describe "validations of the publishing pipeline" do + it "can process publishing a podcast" do + job.stub(:s3_client, stub_client) do + PublishFeedJob.stub(:perform_later, nil) do + PublishingPipelineState.start_pipeline!(podcast) + end - assert job.null_publishing_item?(podcast, nil) - assert_equal :null, job.perform(podcast, nil) + pub_item = PublishingQueueItem.unfinished_items(podcast).first - # There is no currently running publishing pipeline - pub_item = PublishingQueueItem.create(podcast: podcast) - assert job.null_publishing_item?(podcast, pub_item) - assert_equal :null, job.perform(podcast, pub_item) + rss = job.perform(podcast, pub_item) + refute_nil rss + refute_nil job.put_object + assert_nil job.copy_object + end + end - # `settle_remaining` is called at the end of the publishing job - # This means pub_item has been picked up and scheduled - assert_equal "created", pub_item.reload.last_pipeline_state - PublishingPipelineState.complete!(podcast) - assert_equal "complete", pub_item.reload.last_pipeline_state + it "will skip the publishing if the pub items are mismatched" do + job.stub(:s3_client, stub_client) do + PublishFeedJob.stub(:perform_later, nil) do + PublishingPipelineState.start_pipeline!(podcast) + end - # Start a pipeline: Create publishing item and transition that item's pipeline to :created - queue_item = PublishingPipelineState.start_pipeline!(podcast) + pub_item = PublishingQueueItem.create(podcast: podcast) + assert job.mismatched_publishing_item?(podcast, pub_item) + assert_equal :mismatched, job.perform(podcast, pub_item) + end + end + + it "will skip the publishing if the pub items are null" do + job.stub(:s3_client, stub_client) do + assert PublishingQueueItem.unfinished_items(podcast).empty? + + assert job.null_publishing_item?(podcast, nil) + assert_equal :null, job.perform(podcast, nil) - refute job.null_publishing_item?(podcast, queue_item) - res = job.perform(podcast, queue_item) - refute_equal :null, res + # There is no currently running publishing pipeline + pub_item = PublishingQueueItem.create(podcast: podcast) + assert job.null_publishing_item?(podcast, pub_item) + + PublishAppleJob.stub(:do_perform, :publishing_apple!) do + assert_equal :null, job.perform(podcast, pub_item) + end + + # `settle_remaining` is called at the end of the publishing job + # This means pub_item has been picked up and scheduled + assert_equal "created", pub_item.reload.last_pipeline_state + PublishingPipelineState.complete!(podcast) + assert_equal "complete", pub_item.reload.last_pipeline_state + + # Start a pipeline: Create publishing item and transition that item's pipeline to :created + queue_item = PublishingPipelineState.start_pipeline!(podcast) + + refute job.null_publishing_item?(podcast, queue_item) + PublishAppleJob.stub(:do_perform, :publishing_apple!) do + refute_equal :null, job.perform(podcast, queue_item) + end + end end end end describe "publishing to apple" do - it "does not schedule publishing to apple if the feed is non-apple" do - assert_nil job.publish_apple(podcast, private_feed) + let(:apple_feed) { private_feed } + let(:apple_config) { podcast.apple_config } + + before do + assert private_feed.persisted? + assert podcast.reload.apple_config.present? end - describe "when the apple config is present" do - let(:apple_feed) { create(:apple_feed, podcast: podcast) } + it "does not schedule publishing to apple if the apple config prevents it" do + podcast.apple_config.update!(publish_enabled: false) + assert_nil job.publish_apple(podcast, apple_feed) + end + + it "does not schedule publishing to apple if the apple config is disabled" do + apple_config.update!(publish_enabled: false) + assert_nil job.publish_apple(podcast, apple_feed) + end + describe "when the apple config is present" do it "does not schedule publishing to apple if the config is marked as not publishable" do - apple_feed.apple_config.update!(publish_enabled: false) + podcast.apple_config.update!(publish_enabled: false) + assert_nil job.publish_apple(podcast, apple_feed) end @@ -101,31 +139,10 @@ end end - it "Performs the apple publishing job based regardless of sync_blocks_rss flag" do - assert apple_feed.apple_config.present? - assert apple_feed.apple_config.publish_enabled - - # stub the two possible ways the job can be called - # perform_later is not used. - PublishAppleJob.stub(:perform_later, :perform_later) do - PublishAppleJob.stub(:do_perform, :do_perform) do - apple_feed.apple_config.update!(sync_blocks_rss: true) - - assert_equal :do_perform, job.publish_apple(podcast, apple_feed) - - apple_feed.apple_config.update!(sync_blocks_rss: false) - feed.reload - assert_equal :do_perform, job.publish_apple(podcast, apple_feed) - end - end - end - describe "when the apple publishing fails" do before do # Simulate a publishing attempt PublishingQueueItem.create!(podcast: feed.podcast) - PublishingPipelineState.attempt!(feed.podcast) - PublishingPipelineState.start!(feed.podcast) end it "raises an error if the apple publishing fails" do @@ -133,29 +150,21 @@ assert apple_feed.apple_config.publish_enabled PublishAppleJob.stub(:do_perform, ->(*, **) { raise "some apple error" }) do - # it raises - assert_raises(RuntimeError) { job.publish_apple(podcast, apple_feed) } + assert_raises(RuntimeError) { PublishingPipelineState.attempt!(feed.podcast, perform_later: false) } - assert_equal ["created", "started", "error_apple"].sort, PublishingPipelineState.where(podcast: feed.podcast).latest_pipelines.pluck(:status).sort + assert_equal ["created", "started", "error", "error_apple"].sort, PublishingPipelineState.where(podcast: feed.podcast).latest_pipelines.pluck(:status).sort end end - it "does not raise an error if the apple publishing is not blocking RSS" do + it "raises an error if the apple publishing times out" do assert apple_feed.apple_config.present? assert apple_feed.apple_config.publish_enabled - apple_feed.apple_config.update!(sync_blocks_rss: false) - mock = Minitest::Mock.new - mock.expect(:call, nil, [RuntimeError]) + PublishAppleJob.stub(:do_perform, ->(*, **) { raise Apple::AssetStateTimeoutError.new([]) }) do + assert_raises(Apple::AssetStateTimeoutError) { PublishingPipelineState.attempt!(feed.podcast, perform_later: false) } - PublishAppleJob.stub(:do_perform, ->(*, **) { raise "some apple error" }) do - NewRelic::Agent.stub(:notice_error, mock) do - job.publish_apple(podcast, apple_feed) - end + assert_equal ["created", "started", "error", "error_apple"].sort, PublishingPipelineState.where(podcast: feed.podcast).latest_pipelines.pluck(:status).sort end - assert_equal ["created", "started", "error_apple"].sort, PublishingPipelineState.where(podcast: feed.podcast).latest_pipelines.pluck(:status).sort - - mock.verify end end end diff --git a/test/models/publishing_pipeline_state_test.rb b/test/models/publishing_pipeline_state_test.rb index 70db46bd8..820524a6d 100644 --- a/test/models/publishing_pipeline_state_test.rb +++ b/test/models/publishing_pipeline_state_test.rb @@ -255,7 +255,8 @@ PublishFeedJob.stub_any_instance(:save_file, nil) do PublishFeedJob.stub_any_instance(:publish_apple, ->(*args) { raise "error" }) do pqi = PublishingQueueItem.ensure_queued!(podcast) - PublishingPipelineState.attempt!(podcast, perform_later: false) + + assert_raises(RuntimeError) { PublishingPipelineState.attempt!(podcast, perform_later: false) } end end