Close tempfiles earlier in case of exceptions

app/services/audio_processor/ffmpeg.rb

@@ -40,22 +40,18 @@ def concat(new_path, other_paths)
       assert_directory(new_path)
       assert_same_codecs(other_paths)
       list_file = Tempfile.new('list')
-      begin
-        create_list_file(list_file, [audio.path, *other_paths])
-        concat_audio(new_path, list_file)
-      ensure
-        list_file.close!
-      end
+      create_list_file(list_file, [audio.path, *other_paths])
+      concat_audio(new_path, list_file)
+    ensure
+      list_file&.close!
     end
 
     def tag(tags)
       work_file = Tempfile.new(['tagged', File.extname(file)])
-      begin
-        transcode_preserving(work_file.path, custom: metadata_args(tags))
-        FileUtils.mv(work_file.path, file, force: true)
-      ensure
-        work_file.close!
-      end
+      transcode_preserving(work_file.path, custom: metadata_args(tags))
+      FileUtils.mv(work_file.path, file, force: true)
+    ensure
+      work_file&.close!
     end
 
     def bitrate

app/services/import/recording/composer.rb

@@ -146,7 +146,7 @@ def trim_available(recording, start, duration)
 
       def trim(file, start, duration)
         inform("Trimming #{file} from #{start.round}s to #{(start + duration).round}s")
-        new_tempfile(::File.extname(file)).tap do |target_file|
+        new_tempfile(::File.extname(file)) do |target_file|
           proc = AudioProcessor.new(file)
           proc.trim(target_file.path, start, duration)
         end
@@ -156,7 +156,7 @@ def concat(list)
         return list.first if list.size <= 1
 
         with_same_format(list) do |unified|
-          new_tempfile(::File.extname(unified[0])).tap do |target_file|
+          new_tempfile(::File.extname(unified[0])) do |target_file|
             proc = AudioProcessor.new(unified[0])
             proc.concat(target_file.path, unified[1..])
           end
@@ -187,16 +187,15 @@ def convert_all_to_same_format(list)
       # times before raising the exception.
       def convert_list_to_flac(list, format)
         frame_size ||= AudioProcessor::COMMON_FLAC_FRAME_SIZE
-        converted = list.map { |file| convert_to_flac(file, format, frame_size) }
+        convert_file_list(list) { |file| convert_to_flac(file, format, frame_size) }
       rescue AudioProcessor::FailingFrameSizeError
-        close_files(converted) if converted
         frame_size += 1
         max_retry_frame_size = AudioProcessor::COMMON_FLAC_FRAME_SIZE + MAX_TRANSCODE_RETRIES
         frame_size <= max_retry_frame_size ? retry : raise
       end
 
       def convert_list_to_format(list, format)
-        list.map do |file|
+        convert_file_list(list) do |file|
           if ::File.extname(file.path) == ".#{format.file_extension}"
             file
           else
@@ -207,24 +206,42 @@ def convert_list_to_format(list, format)
 
       def convert_to_format(file, format)
         processor = AudioProcessor.new(file.path)
-        new_tempfile(".#{format.file_extension}").tap do |target_file|
+        new_tempfile(".#{format.file_extension}") do |target_file|
           processor.transcode(target_file.path, format)
         end
       end
 
       def convert_to_flac(file, format, frame_size)
         processor = AudioProcessor.new(file.path)
-        new_tempfile(".#{format.file_extension}").tap do |target_file|
+        new_tempfile(".#{format.file_extension}") do |target_file|
           processor.transcode_flac(target_file.path, format, frame_size)
         end
       end
 
+      def convert_file_list(list)
+        converted = []
+        # use `each` instead of `map` to be able to close previously converted files
+        # if an error is raised in the middle of the list.
+        list.each { |file| converted << yield(file) }
+        converted
+      rescue StandardError
+        close_files(converted)
+        raise
+      end
+
       def close_files(list)
         list.each { |file| file.close! if file.respond_to?(:close!) }
       end
 
+      # Create a new tempfile, generate its content and then return the file.
+      # If generating content fails, remove the tempfile and raise the original error.
       def new_tempfile(extension)
-        Tempfile.new(['master', extension])
+        file = Tempfile.new(['master', extension])
+        yield file # generate content
+        file
+      rescue StandardError
+        file&.close!
+        raise
       end
 
     end