From 3f74f199dc11ee5f4b31bfc406a7e577a9d80939 Mon Sep 17 00:00:00 2001
From: Kieran <kieran.eglin@gmail.com>
Date: Mon, 29 Apr 2024 14:06:12 -0700
Subject: [PATCH] Stopped worker from retrying if doing so wouldn't improve
 things (#210)

---
 .../downloading/media_download_worker.ex       | 18 ++++++++++++++++--
 .../downloading/media_download_worker_test.exs | 12 ++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/lib/pinchflat/downloading/media_download_worker.ex b/lib/pinchflat/downloading/media_download_worker.ex
index 0e12067..b977132 100644
--- a/lib/pinchflat/downloading/media_download_worker.ex
+++ b/lib/pinchflat/downloading/media_download_worker.ex
@@ -67,8 +67,8 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
       {:recovered, _} ->
         {:error, :retry}
 
-      {:error, _message} ->
-        {:error, :download_failed}
+      {:error, message} ->
+        action_on_error(message)
     end
   end
 
@@ -89,4 +89,18 @@ defmodule Pinchflat.Downloading.MediaDownloadWorker do
       nil
     end
   end
+
+  defp action_on_error(message) do
+    # This will attempt re-download at the next indexing, but it won't be retried
+    # immediately as part of job failure logic
+    non_retryable_errors = ["Video unavailable"]
+
+    if String.contains?(to_string(message), non_retryable_errors) do
+      Logger.error("yt-dlp download will not be retried: #{inspect(message)}")
+
+      {:ok, :non_retry}
+    else
+      {:error, :download_failed}
+    end
+  end
 end
diff --git a/test/pinchflat/downloading/media_download_worker_test.exs b/test/pinchflat/downloading/media_download_worker_test.exs
index 7d0e616..e9155fa 100644
--- a/test/pinchflat/downloading/media_download_worker_test.exs
+++ b/test/pinchflat/downloading/media_download_worker_test.exs
@@ -106,6 +106,18 @@ defmodule Pinchflat.Downloading.MediaDownloadWorkerTest do
       end)
     end
 
+    test "does not set the job to retryable if retrying wouldn't fix the issue", %{media_item: media_item} do
+      expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl ->
+        {:error, "Something something Video unavailable something something", 1}
+      end)
+
+      Oban.Testing.with_testing_mode(:inline, fn ->
+        {:ok, job} = Oban.insert(MediaDownloadWorker.new(%{id: media_item.id, redownload?: true}))
+
+        assert job.state == "completed"
+      end)
+    end
+
     test "it ensures error are returned in a 2-item tuple", %{media_item: media_item} do
       expect(YtDlpRunnerMock, :run, fn _url, _opts, _ot, _addl -> {:error, "error", 1} end)