Skip to content

Commit 6f111b9

Browse files
authored
Merge pull request #1537 from PRX/fix/stream_recording_errors
Handle stream recording errors
2 parents d8f7010 + 66d9db2 commit 6f111b9

4 files changed

Lines changed: 42 additions & 8 deletions

File tree

app/helpers/stream_recordings_helper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def stream_status_class(resource)
4646
"warning"
4747
elsif resource.status_complete?
4848
"info"
49-
elsif resource.recording?
49+
elsif resource.recording? || resource.status_error?
5050
"danger"
5151
else
5252
"primary"

app/models/stream_resource.rb

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ class StreamResource < ApplicationRecord
1616

1717
validates :start_at, presence: true
1818
validates :end_at, presence: true, comparison: {greater_than: :start_at}
19-
validates :actual_start_at, presence: true, if: :done_recording?
20-
validates :actual_end_at, presence: true, comparison: {greater_than: :actual_start_at}, if: :done_recording?
21-
validates :original_url, presence: true, if: :done_recording?
19+
validates :actual_start_at, presence: true, if: :has_recording?
20+
validates :actual_end_at, presence: true, comparison: {greater_than: :actual_start_at}, if: :has_recording?
21+
validates :original_url, presence: true, if: :has_recording?
2222

2323
after_initialize :set_defaults
2424
before_validation :set_defaults
@@ -43,15 +43,15 @@ def copy_media(force = false)
4343
def needs_copy?
4444
if status_complete?
4545
false
46-
elsif done_recording? && copy_task
46+
elsif has_recording? && copy_task
4747
false
4848
else
49-
done_recording?
49+
has_recording?
5050
end
5151
end
5252

53-
def done_recording?
54-
%w[created started recording].exclude?(status)
53+
def has_recording?
54+
%w[created started recording error].exclude?(status)
5555
end
5656

5757
def file_name

app/models/tasks/record_stream_task.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ def update_owner
6767
stream_resource.tasks.fix_media.update_all(status: :cancelled)
6868
stream_resource.copy_media
6969
end
70+
rescue => err
71+
Rails.logger.error("RecordStreamTask update_owner error", error: err)
72+
NewRelic::Agent.notice_error(err)
7073
end
7174

7275
# parsing data from the job_id

test/models/tasks/record_stream_task_test.rb

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,37 @@
166166
end
167167
end
168168
end
169+
170+
it "handles recording errors" do
171+
new_res = StreamResource.new(start_at: "2026-09-09T10:00:00Z", end_at: "2026-09-09T11:00:00Z")
172+
task.owner = new_res
173+
task.update(status: "error")
174+
175+
assert new_res.persisted?
176+
assert_equal new_res.status, "error"
177+
assert_nil new_res.actual_start_at
178+
assert_nil new_res.actual_end_at
179+
end
180+
181+
it "logs validation errors" do
182+
mock_log = Minitest::Mock.new.expect(:call, nil) { true }
183+
mock_notice = Minitest::Mock.new.expect(:call, nil) { true }
184+
185+
Rails.logger.stub(:error, mock_log) do
186+
NewRelic::Agent.stub(:notice_error, mock_notice) do
187+
new_res = StreamResource.new(start_at: "2026-09-09T10:00:00Z", end_at: nil)
188+
task.owner = new_res
189+
task.update(status: "error")
190+
191+
assert_equal "error", task.status
192+
refute task.changed?
193+
refute new_res.persisted?
194+
end
195+
end
196+
197+
mock_log.verify
198+
mock_notice.verify
199+
end
169200
end
170201

171202
describe "#job_id_parts" do

0 commit comments

Comments
 (0)