@@ -10,6 +10,7 @@ defmodule Pinchflat.Downloading.MediaDownloader do
1010 alias Pinchflat.Repo
1111 alias Pinchflat.Media
1212 alias Pinchflat.Media.MediaItem
13+ alias Pinchflat.Utils.StringUtils
1314 alias Pinchflat.Metadata.NfoBuilder
1415 alias Pinchflat.Metadata.MetadataParser
1516 alias Pinchflat.Metadata.MetadataFileHelpers
@@ -20,16 +21,53 @@ defmodule Pinchflat.Downloading.MediaDownloader do
2021
2122 @ doc """
2223 Downloads media for a media item, updating the media item based on the metadata
23- returned by yt-dlp. Also saves the entire metadata response to the associated
24- media_metadata record.
24+ returned by yt-dlp. Encountered errors are saved to the Media Item record. Saves
25+ the entire metadata response to the associated media_metadata record.
2526
26- NOTE: related methods (like the download worker) won't download if the media item's source
27+ NOTE: related methods (like the download worker) won't download if Pthe media item's source
2728 is set to not download media. However, I'm not enforcing that here since I need this for testing.
2829 This may change in the future but I'm not stressed.
2930
30- Returns {:ok, %MediaItem{}} | {:error, any, ...any }
31+ Returns {:ok, %MediaItem{}} | {:error, atom(), String.t()} | {:recovered, %MediaItem{}, String.t() }
3132 """
3233 def download_for_media_item ( % MediaItem { } = media_item , override_opts \\ [ ] ) do
34+ case attempt_download_and_update_for_media_item ( media_item , override_opts ) do
35+ { :ok , media_item } ->
36+ # Returns {:ok, %MediaItem{}}
37+ Media . update_media_item ( media_item , % { last_error: nil } )
38+
39+ { :error , error_atom , message } ->
40+ Media . update_media_item ( media_item , % { last_error: StringUtils . wrap_string ( message ) } )
41+
42+ { :error , error_atom , message }
43+
44+ { :recovered , media_item , message } ->
45+ { :ok , updated_media_item } = Media . update_media_item ( media_item , % { last_error: StringUtils . wrap_string ( message ) } )
46+
47+ { :recovered , updated_media_item , message }
48+ end
49+ end
50+
51+ # Looks complicated, but here's the key points:
52+ # - download_with_options runs a pre-check to see if the media item is suitable for download.
53+ # - If the media item fails the precheck, it returns {:error, :unsuitable_for_download, message}
54+ # - If the precheck passes but the download fails, it normally returns {:error, :download_failed, message}
55+ # - However, there are some errors we can recover from (eg: failure to communicate with SponsorBlock).
56+ # In this case, we attempt the download anyway and update the media item with what details we do have.
57+ # This case returns {:recovered, updated_media_item, message}
58+ # - If we attempt a retry but it fails, we return {:error, :unrecoverable, message}
59+ # - If there is an unknown error unrelated to the above, we return {:error, :unknown, message}
60+ # - Finally, if there is no error, we update the media item with the parsed JSON and return {:ok, updated_media_item}
61+ #
62+ # Restated, here are the return values for each case:
63+ # - On success: {:ok, updated_media_item}
64+ # - On initial failure but successfully recovered: {:recovered, updated_media_item, message}
65+ # - On error: {:error, error_atom, message} where error_atom is one of:
66+ # - `:unsuitable_for_download` if the media item fails the precheck
67+ # - `:unrecoverable` if there was an initial failure and the recovery attempt failed
68+ # - `:download_failed` for all other yt-dlp-related downloading errors
69+ # - `:unknown` for any other errors, including those not related to yt-dlp
70+ defp attempt_download_and_update_for_media_item ( media_item , override_opts ) do
3371 output_filepath = FilesystemUtils . generate_metadata_tmpfile ( :json )
3472 media_with_preloads = Repo . preload ( media_item , [ :metadata , source: :media_profile ] )
3573
@@ -38,31 +76,30 @@ defmodule Pinchflat.Downloading.MediaDownloader do
3876 update_media_item_from_parsed_json ( media_with_preloads , parsed_json )
3977
4078 { :error , :unsuitable_for_download } ->
41- Logger . warning (
79+ message =
4280 "Media item ##{ media_with_preloads . id }
43- )
4481
45- { :error , :unsuitable_for_download }
82+ Logger . warning ( message )
83+
84+ { :error , :unsuitable_for_download , message }
4685
4786 { :error , message , _exit_code } ->
4887 Logger . error ( "yt-dlp download error for media item ##{ media_with_preloads . id } #{ inspect ( message ) } )
4988
5089 if String . contains? ( to_string ( message ) , recoverable_errors ( ) ) do
51- attempt_update_media_item ( media_with_preloads , output_filepath )
52-
53- { :recovered , message }
90+ attempt_recovery_from_error ( media_with_preloads , output_filepath , message )
5491 else
55- { :error , message }
92+ { :error , :download_failed , message }
5693 end
5794
5895 err ->
5996 Logger . error ( "Unknown error downloading media item ##{ media_with_preloads . id } #{ inspect ( err ) } )
6097
61- { :error , "Unknown error: #{ inspect ( err ) } }
98+ { :error , :unknown , "Unknown error: #{ inspect ( err ) } }
6299 end
63100 end
64101
65- defp attempt_update_media_item ( media_with_preloads , output_filepath ) do
102+ defp attempt_recovery_from_error ( media_with_preloads , output_filepath , error_message ) do
66103 with { :ok , contents } <- File . read ( output_filepath ) ,
67104 { :ok , parsed_json } <- Phoenix . json_library ( ) . decode ( contents ) do
68105 Logger . info ( """
@@ -71,12 +108,13 @@ defmodule Pinchflat.Downloading.MediaDownloader do
71108 anyway
72109 """ )
73110
74- update_media_item_from_parsed_json ( media_with_preloads , parsed_json )
111+ { :ok , updated_media_item } = update_media_item_from_parsed_json ( media_with_preloads , parsed_json )
112+ { :recovered , updated_media_item , error_message }
75113 else
76114 err ->
77115 Logger . error ( "Unable to recover error for media item ##{ media_with_preloads . id } #{ inspect ( err ) } )
78116
79- { :error , :retry_failed }
117+ { :error , :unrecoverable , error_message }
80118 end
81119 end
82120
0 commit comments