Make audio_file#convert raise an exception on failure (#656)

app/models/audio_file.rb

@@ -15,6 +15,8 @@
 #
 
 class AudioFile < ApplicationRecord
+  class FailedTranscode < StandardError; end
+
   belongs_to :location
   belongs_to :codec
   has_one :track, dependent: :nullify
@@ -37,7 +39,7 @@ def check_self
 
   def convert(codec_conversion, out_file_name)
     parameters = codec_conversion.ffmpeg_params.split
-    Open3.popen2(
+    _stdout, status = Open3.capture2(
       'ffmpeg',
       '-i', full_path,
       '-f', codec_conversion.resulting_codec.extension,
@@ -46,10 +48,8 @@ def convert(codec_conversion, out_file_name)
       '-map', 'a',
       out_file_name,
       err: [Rails.configuration.ffmpeg_log_location, 'a']
-    ) do |_stdin, _stdout, _wait_thr|
-      # We call `Open3.popen2` with an empty block, so the process finishes before we return
-      # This returns `Process::Status`
-    end
+    )
+    raise FailedTranscode, "ffmpeg exited with #{status.to_i}" unless status.success?
   end
 
   def full_path

test/models/audio_file_test.rb

@@ -15,6 +15,15 @@
 require 'test_helper'
 
 class AudioFileTest < ActiveSupport::TestCase
+  class ProcessStub
+    def initialize(exit_status)
+      @exit_status = exit_status
+    end
+
+    def success? = @exit_status.zero?
+    def to_i = @exit_status
+  end
+
   def setup
     location = Location.create(path: Rails.root.join('test/files'))
     codec = Codec.create(extension: 'flac', mimetype: 'audio/flac')
@@ -26,10 +35,20 @@ def setup
 
     path = TranscodedItem.path_for(codec_conversion, SecureRandom.uuid)
 
-    Open3.stubs(:popen2).once.with do |*args, **kwargs|
+    Open3.stubs(:capture2).once.returns(['', ProcessStub.new(0)]).with do |*args, **kwargs|
       args.include?('ffmpeg') && args.include?(@audio_file.full_path) && args[-1] == path && kwargs.key?(:err)
     end
 
     @audio_file.convert(codec_conversion, path)
   end
+
+  test 'convert should raise if underlying process fails' do
+    codec_conversion = create(:codec_conversion)
+
+    Open3.stubs(:capture2).once.returns(['', ProcessStub.new(9)])
+
+    assert_raises AudioFile::FailedTranscode do
+      @audio_file.convert(codec_conversion, TranscodedItem.path_for(codec_conversion, SecureRandom.uuid))
+    end
+  end
 end