Video transcoding deletes original file before new one saved

[mattrogowski]

Affected version

2.3.2

XFMG\Service\Media\Transcoder::finalizeTranscode():

File::deleteFromAbstractedPath($queueData['fileName']);

clearstatcache();
$updates = [
    'file_hash' => md5_file($outputFile),
    'file_size' => filesize($outputFile),
];

$data = $attachment->Data;
$data->bulkSet($updates);
if (!$data->save(false, false))
{
    $this->transcodeException($queueData);
}
$finalPath = $data->getAbstractedDataPath();

File::copyFileToAbstractedPath($outputFile, $finalPath);

The original path is deleted first, but if anything errors during this step the transcoding will just fail and the original file is lost. It throws a transcodeException but by this point the process can't be recovered as the file has been deleted. I suppose as the queue item was deleted it doesn't matter in itself as it never gets re-tried anyway, but the process can be recoverable. An example of what can error here is a table lock when saving the attachment - if this happens the save won't work but there's no reason that couldn't be caught and re-tried - however it definitely can't if the file has been deleted.

I've extended this to wrap the logic in a transaction, run it inside a try/catch block, catch any database exceptions, and restore/update the queue item to be pending again. It also only deletes the file after the new one is written. This way it'll just try the transcoding again, the file hasn't been deleted, and it more than likely wouldn't hit another database exception a second time. Currently it throws the baby out with the bathwater if there's a temporary database error.