diff --git a/lib/reencodarr/ab_av1/crf_search.ex b/lib/reencodarr/ab_av1/crf_search.ex index 46739561..65bfd1a0 100644 --- a/lib/reencodarr/ab_av1/crf_search.ex +++ b/lib/reencodarr/ab_av1/crf_search.ex @@ -87,12 +87,14 @@ defmodule Reencodarr.AbAv1.CrfSearch do nil -> false - _pid -> - try do - GenServer.call(__MODULE__, :running?) == :running - catch - :exit, _ -> false + pid when is_pid(pid) -> + case GenServer.call(__MODULE__, :running?, 1000) do + :running -> true + _ -> false end + + _ -> + false end end diff --git a/lib/reencodarr/ab_av1/helper.ex b/lib/reencodarr/ab_av1/helper.ex index 6544f55a..b996689d 100644 --- a/lib/reencodarr/ab_av1/helper.ex +++ b/lib/reencodarr/ab_av1/helper.ex @@ -51,8 +51,15 @@ defmodule Reencodarr.AbAv1.Helper do if File.exists?(temp_dir) do temp_dir else - File.mkdir_p!(temp_dir) - temp_dir + case File.mkdir_p(temp_dir) do + :ok -> + temp_dir + + {:error, reason} -> + Logger.error("Failed to create temp directory #{temp_dir}: #{inspect(reason)}") + # Fallback to system temp directory + System.tmp_dir!() + end end end diff --git a/lib/reencodarr/analyzer/broadway.ex b/lib/reencodarr/analyzer/broadway.ex index c31499ea..f7f978ff 100644 --- a/lib/reencodarr/analyzer/broadway.ex +++ b/lib/reencodarr/analyzer/broadway.ex @@ -131,21 +131,7 @@ defmodule Reencodarr.Analyzer.Broadway do # Process the batch using optimized batch mediainfo fetching # This does ALL the mediainfo gathering first, then database operations at the end - result = - try do - process_batch_with_single_mediainfo(video_infos, context) - rescue - e -> - Logger.error("Broadway: Exception during batch processing: #{inspect(e)}") - Logger.error("Broadway: Exception stacktrace: #{inspect(__STACKTRACE__)}") - :error - end - - # Only log failures - case result do - :error -> Logger.error("Broadway: Batch processing failed") - _ -> :ok - end + _result = process_batch_with_single_mediainfo(video_infos, context) # Log completion and emit telemetry duration = System.monotonic_time(:millisecond) - start_time @@ -156,10 +142,9 @@ defmodule Reencodarr.Analyzer.Broadway do # Get current queue length for progress calculation current_queue_length = - try do - QueueManager.get_count() - catch - _error -> 0 + case QueueManager.get_count() do + count when is_integer(count) and count >= 0 -> count + _ -> 0 end Telemetry.emit_analyzer_throughput(batch_size, current_queue_length) @@ -171,13 +156,8 @@ defmodule Reencodarr.Analyzer.Broadway do {:batch_analysis_completed, batch_size} ) - # Transform successful results to success, all failed to failed for Broadway - Enum.map(messages, fn message -> - case result do - :ok -> message - :error -> Message.failed(message, "batch processing failed") - end - end) + # Return messages as-is since processing always succeeds + messages end @doc """ @@ -207,53 +187,46 @@ defmodule Reencodarr.Analyzer.Broadway do Logger.debug("Video paths in batch: #{inspect(Enum.map(video_infos, & &1.path))}") - try do - # Extract all paths for batch mediainfo command - paths = Enum.map(video_infos, & &1.path) - Logger.debug("Broadway: Extracted #{length(paths)} paths for mediainfo") + # Extract all paths for batch mediainfo command + paths = Enum.map(video_infos, & &1.path) + Logger.debug("Broadway: Extracted #{length(paths)} paths for mediainfo") - mediainfo_start_time = System.monotonic_time(:millisecond) + mediainfo_start_time = System.monotonic_time(:millisecond) - case execute_chunked_mediainfo_command(paths, mediainfo_batch_size) do - {:ok, mediainfo_map} -> - mediainfo_duration = System.monotonic_time(:millisecond) - mediainfo_start_time + case execute_chunked_mediainfo_command(paths, mediainfo_batch_size) do + {:ok, mediainfo_map} -> + mediainfo_duration = System.monotonic_time(:millisecond) - mediainfo_start_time - # Record mediainfo batch performance for tuning - PerformanceMonitor.record_mediainfo_batch(length(paths), mediainfo_duration) + # Record mediainfo batch performance for tuning + PerformanceMonitor.record_mediainfo_batch(length(paths), mediainfo_duration) - Logger.debug( - "Successfully fetched mediainfo for #{length(video_infos)} videos in #{mediainfo_duration}ms" - ) + Logger.debug( + "Successfully fetched mediainfo for #{length(video_infos)} videos in #{mediainfo_duration}ms" + ) - Logger.debug("Mediainfo keys: #{inspect(Map.keys(mediainfo_map))}") - Logger.debug("Broadway: About to process videos with batch mediainfo") - result = process_videos_with_batch_mediainfo(video_infos, mediainfo_map) + Logger.debug("Mediainfo keys: #{inspect(Map.keys(mediainfo_map))}") + Logger.debug("Broadway: About to process videos with batch mediainfo") + result = process_videos_with_batch_mediainfo(video_infos, mediainfo_map) - Logger.debug( - "Broadway: Completed process_videos_with_batch_mediainfo with result: #{inspect(result)}" - ) + Logger.debug( + "Broadway: Completed process_videos_with_batch_mediainfo with result: #{inspect(result)}" + ) - result + result - {:error, reason} -> - Logger.warning( - "Batch mediainfo fetch failed: #{reason}, falling back to individual processing" - ) + {:error, reason} -> + Logger.warning( + "Batch mediainfo fetch failed: #{reason}, falling back to individual processing" + ) - Logger.debug("Broadway: About to process videos individually") - result = process_videos_individually(video_infos) + Logger.debug("Broadway: About to process videos individually") + result = process_videos_individually(video_infos) - Logger.debug( - "Broadway: Completed process_videos_individually with result: #{inspect(result)}" - ) + Logger.debug( + "Broadway: Completed process_videos_individually with result: #{inspect(result)}" + ) - result - end - rescue - e -> - Logger.error("Broadway: Exception in process_batch_with_single_mediainfo: #{inspect(e)}") - Logger.error("Broadway: Stacktrace: #{inspect(__STACKTRACE__)}") - :error + result end end @@ -373,13 +346,9 @@ defmodule Reencodarr.Analyzer.Broadway do handle_upsert_results(successful_data, upsert_results, failed_paths) {:error, reason} -> + Logger.error("Broadway: perform_batch_upsert failed: #{inspect(reason)}") {:error, reason} end - rescue - e -> - Logger.error("Broadway: Exception during batch upsert and transition: #{inspect(e)}") - Logger.error("Broadway: Exception stacktrace: #{inspect(__STACKTRACE__)}") - :error end defp log_batch_operation(batch_size) when batch_size > 5 do @@ -580,20 +549,13 @@ defmodule Reencodarr.Analyzer.Broadway do defp decode_and_parse_single_mediainfo_json(json, path) do Logger.debug("Decoding mediainfo JSON for #{path}") - try do - case Jason.decode(json) do - {:ok, data} -> - handle_decoded_single_mediainfo(data) + case Jason.decode(json) do + {:ok, data} -> + handle_decoded_single_mediainfo(data) - {:error, reason} -> - Logger.error("JSON decode failed: #{inspect(reason)}") - {:error, "JSON decode failed: #{inspect(reason)}"} - end - rescue - e -> - Logger.error("Error parsing mediainfo JSON: #{inspect(e)}") - Logger.error("Stacktrace: #{inspect(__STACKTRACE__)}") - {:error, "error parsing JSON: #{inspect(e)}"} + {:error, reason} -> + Logger.error("JSON decode failed: #{inspect(reason)}") + {:error, "JSON decode failed: #{inspect(reason)}"} end end @@ -699,19 +661,13 @@ defmodule Reencodarr.Analyzer.Broadway do defp decode_and_parse_batch_mediainfo_json(json, paths) do Logger.debug("Decoding batch mediainfo JSON for #{length(paths)} files") - try do - case Jason.decode(json) do - {:ok, data} -> - handle_decoded_mediainfo_data(data, paths) + case Jason.decode(json) do + {:ok, data} -> + handle_decoded_mediainfo_data(data, paths) - {:error, reason} -> - Logger.error("JSON decode failed: #{inspect(reason)}") - {:error, "JSON decode failed: #{inspect(reason)}"} - end - rescue - e -> - Logger.error("Error parsing batch mediainfo JSON: #{inspect(e)}") - {:error, "error parsing JSON: #{inspect(e)}"} + {:error, reason} -> + Logger.error("JSON decode failed: #{inspect(reason)}") + {:error, "JSON decode failed: #{inspect(reason)}"} end end diff --git a/lib/reencodarr/core/parsers.ex b/lib/reencodarr/core/parsers.ex index 1aced7db..4633afbd 100644 --- a/lib/reencodarr/core/parsers.ex +++ b/lib/reencodarr/core/parsers.ex @@ -6,6 +6,8 @@ defmodule Reencodarr.Core.Parsers do for common data transformations needed throughout the application. """ + require Logger + @doc """ Parses duration string in various formats to seconds. @@ -262,15 +264,32 @@ defmodule Reencodarr.Core.Parsers do end # Convert captured string values to appropriate types - defp convert_value(value, :int), do: String.to_integer(value) + @spec convert_value(String.t(), :int) :: integer() + defp convert_value(value, :int) do + case Integer.parse(value) do + {int, ""} -> int + _ -> 0 + end + end + @spec convert_value(String.t(), :float) :: float() defp convert_value(value, :float) do case String.contains?(value, ".") do - true -> String.to_float(value) - false -> String.to_integer(value) |> Kernel.*(1.0) + true -> + case Float.parse(value) do + {float, ""} -> float + _ -> 0.0 + end + + false -> + case Integer.parse(value) do + {int, ""} -> int * 1.0 + _ -> 0.0 + end end end + @spec convert_value(String.t(), :string) :: String.t() defp convert_value(value, :string), do: value @doc """ @@ -323,42 +342,4 @@ defmodule Reencodarr.Core.Parsers do end def parse_float_exact(_), do: {:error, :invalid_input} - - @doc """ - Parses an integer with exact matching, raises on error. - - ## Examples - - iex> Parsers.parse_integer_exact!("123") - 123 - - iex> Parsers.parse_integer_exact!("123abc") - ** (ArgumentError) Invalid integer format: "123abc" - """ - @spec parse_integer_exact!(String.t()) :: integer() - def parse_integer_exact!(value) do - case parse_integer_exact(value) do - {:ok, int} -> int - {:error, _} -> raise ArgumentError, "Invalid integer format: #{inspect(value)}" - end - end - - @doc """ - Parses a float with exact matching, raises on error. - - ## Examples - - iex> Parsers.parse_float_exact!("123.45") - 123.45 - - iex> Parsers.parse_float_exact!("invalid") - ** (ArgumentError) Invalid float format: "invalid" - """ - @spec parse_float_exact!(String.t()) :: float() - def parse_float_exact!(value) do - case parse_float_exact(value) do - {:ok, float} -> float - {:error, _} -> raise ArgumentError, "Invalid float format: #{inspect(value)}" - end - end end diff --git a/lib/reencodarr/dashboard_state.ex b/lib/reencodarr/dashboard_state.ex index 20a6e56b..42f04720 100644 --- a/lib/reencodarr/dashboard_state.ex +++ b/lib/reencodarr/dashboard_state.ex @@ -91,19 +91,25 @@ defmodule Reencodarr.DashboardState do # Check actual status of Broadway pipelines for initial state defp analyzer_running? do - Reencodarr.Analyzer.Broadway.running?() + case Reencodarr.Analyzer.Broadway.running?() do + result when is_boolean(result) -> result + end rescue _ -> false end defp crf_searcher_running? do - Reencodarr.CrfSearcher.Broadway.running?() + case Reencodarr.CrfSearcher.Broadway.running?() do + result when is_boolean(result) -> result + end rescue _ -> false end defp encoder_running? do - Reencodarr.Encoder.Broadway.running?() + case Reencodarr.Encoder.Broadway.running?() do + result when is_boolean(result) -> result + end rescue _ -> false end diff --git a/lib/reencodarr/encoder/broadway/producer.ex b/lib/reencodarr/encoder/broadway/producer.ex index 977cb047..90cac807 100644 --- a/lib/reencodarr/encoder/broadway/producer.ex +++ b/lib/reencodarr/encoder/broadway/producer.ex @@ -249,26 +249,17 @@ defmodule Reencodarr.Encoder.Broadway.Producer do false pid -> - try do - case GenServer.call(pid, :running?, 1000) do - :not_running -> - Logger.debug( - "Producer: encoding_available? - Encode GenServer is :not_running - AVAILABLE" - ) - - true + case GenServer.call(pid, :running?, 1000) do + :not_running -> + Logger.debug( + "Producer: encoding_available? - Encode GenServer is :not_running - AVAILABLE" + ) - status -> - Logger.debug( - "Producer: encoding_available? - Encode GenServer status: #{inspect(status)} - NOT AVAILABLE" - ) + true - false - end - catch - :exit, reason -> + status when status != :not_running -> Logger.debug( - "Producer: encoding_available? - Encode GenServer call failed: #{inspect(reason)}" + "Producer: encoding_available? - Encode GenServer status: #{inspect(status)} - NOT AVAILABLE" ) false diff --git a/lib/reencodarr/media.ex b/lib/reencodarr/media.ex index d4882c13..57336196 100644 --- a/lib/reencodarr/media.ex +++ b/lib/reencodarr/media.ex @@ -328,9 +328,6 @@ defmodule Reencodarr.Media do _ -> false end) - rescue - # If there's any error generating args, consider it problematic - _ -> true end @doc """ @@ -1023,7 +1020,13 @@ defmodule Reencodarr.Media do end defp parse_crf(crf) when is_number(crf), do: crf - defp parse_crf(crf) when is_binary(crf), do: Parsers.parse_float_exact!(crf) + + defp parse_crf(crf) when is_binary(crf) do + case Parsers.parse_float_exact(crf) do + {:ok, float} -> float + {:error, _} -> 0.0 + end + end def list_videos_awaiting_crf_search do from(v in Video, diff --git a/lib/reencodarr/media/clean.ex b/lib/reencodarr/media/clean.ex index 92167f44..003aed11 100644 --- a/lib/reencodarr/media/clean.ex +++ b/lib/reencodarr/media/clean.ex @@ -489,6 +489,7 @@ defmodule Reencodarr.Media.Clean do end defp parse_crf(crf) do - Parsers.parse_float_exact!(crf) + {:ok, value} = Parsers.parse_float_exact(crf) + value end end diff --git a/lib/reencodarr/media/codec_mapper.ex b/lib/reencodarr/media/codec_mapper.ex index 8f90f92f..afa2ed63 100644 --- a/lib/reencodarr/media/codec_mapper.ex +++ b/lib/reencodarr/media/codec_mapper.ex @@ -66,9 +66,9 @@ defmodule Reencodarr.Media.CodecMapper do "0" => 0 } - @spec map_codec_id(String.t() | nil) :: String.t() | atom() + @spec map_codec_id(String.t() | nil) :: String.t() | integer() | atom() def map_codec_id(codec) do - Map.fetch!(@codec_id_map, codec) + Map.get(@codec_id_map, codec, :unknown) end @spec has_av1_codec?(list(String.t()) | nil) :: boolean() diff --git a/lib/reencodarr/media/codecs.ex b/lib/reencodarr/media/codecs.ex index 6098a18e..59951b9b 100644 --- a/lib/reencodarr/media/codecs.ex +++ b/lib/reencodarr/media/codecs.ex @@ -120,9 +120,9 @@ defmodule Reencodarr.Media.Codecs do @doc """ Maps codec identifiers to standardized internal format. """ - @spec map_codec_id(String.t() | nil) :: String.t() | atom() + @spec map_codec_id(String.t() | nil) :: String.t() | integer() | atom() def map_codec_id(codec) do - Map.fetch!(@codec_id_map, codec) + Map.get(@codec_id_map, codec, :unknown) end # === Codec Detection Functions === diff --git a/lib/reencodarr/media/video_upsert.ex b/lib/reencodarr/media/video_upsert.ex index c7212520..dfef2a06 100644 --- a/lib/reencodarr/media/video_upsert.ex +++ b/lib/reencodarr/media/video_upsert.ex @@ -327,7 +327,17 @@ defmodule Reencodarr.Media.VideoUpsert do string_key = to_string(key) Enum.any?(conflict_except, &(to_string(&1) == string_key)) or string_key == "dateAdded" end) - |> Enum.map(fn {key, value} -> {String.to_atom(key), value} end) + |> Enum.map(fn {key, value} -> + case safe_to_existing_atom(key) do + {:ok, atom_key} -> + {atom_key, value} + + :error -> + Logger.warning("Ignoring unknown attribute key: #{inspect(key)}") + nil + end + end) + |> Enum.reject(&is_nil/1) from(v in Video, update: [set: ^update_fields], @@ -367,4 +377,13 @@ defmodule Reencodarr.Media.VideoUpsert do # Fallback for invalid paths - let validation handle the error @spec get_video_metadata_for_comparison(any()) :: nil defp get_video_metadata_for_comparison(_), do: nil + + @spec safe_to_existing_atom(any()) :: {:ok, atom()} | :error + defp safe_to_existing_atom(key) when is_binary(key) do + # Only convert if the atom already exists - let it crash if not + {:ok, String.to_existing_atom(key)} + end + + defp safe_to_existing_atom(key) when is_atom(key), do: {:ok, key} + defp safe_to_existing_atom(_), do: :error end diff --git a/lib/reencodarr/media/video_validator.ex b/lib/reencodarr/media/video_validator.ex index 95098f53..228573ac 100644 --- a/lib/reencodarr/media/video_validator.ex +++ b/lib/reencodarr/media/video_validator.ex @@ -108,6 +108,6 @@ defmodule Reencodarr.Media.VideoValidator do """ @spec get_attr_value(video_attrs(), String.t()) :: any() def get_attr_value(attrs, key) when is_binary(key) do - Map.get(attrs, key) || Map.get(attrs, String.to_atom(key)) + Map.get(attrs, key) || Map.get(attrs, String.to_existing_atom(key)) end end diff --git a/lib/reencodarr/services/radarr.ex b/lib/reencodarr/services/radarr.ex index c2760cb7..922ff877 100644 --- a/lib/reencodarr/services/radarr.ex +++ b/lib/reencodarr/services/radarr.ex @@ -99,44 +99,69 @@ defmodule Reencodarr.Services.Radarr do end end + @spec execute_movie_rename(integer(), list(map())) :: {:ok, map()} | {:error, String.t()} defp execute_movie_rename(movie_id, renameable_files) do # Extract movie file IDs from the renameable files response and ensure they're integers - renameable_file_ids = + file_ids = renameable_files |> Enum.map(fn file -> file["movieFileId"] end) - |> Enum.map(&parse_file_id/1) - - json_payload = %{ - name: "RenameFiles", - files: renameable_file_ids - } - - Logger.info( - "Radarr rename_movie_files request - Movie ID: #{movie_id}, File IDs: #{inspect(renameable_file_ids)}" - ) - - Logger.debug("Radarr rename_movie_files JSON payload: #{inspect(json_payload)}") - case request( - url: "/api/v3/command", - method: :post, - json: json_payload - ) do - {:ok, response} = result -> - Logger.debug("Radarr rename_movie_files response: #{inspect(response.body)}") - result - - {:error, reason} = error -> - Logger.error("Radarr rename_movie_files error: #{inspect(reason)}") - error + # Parse all file IDs, collecting successes and failures + {successes, failures} = + file_ids + |> Enum.map(&parse_file_id/1) + |> Enum.split_with(fn + {:ok, _} -> true + {:error, _} -> false + end) + + # If any parsing failed, return early with error + if Enum.empty?(failures) do + # Extract successful IDs + renameable_file_ids = Enum.map(successes, fn {:ok, id} -> id end) + + json_payload = %{ + name: "RenameFiles", + files: renameable_file_ids + } + + Logger.info( + "Radarr rename_movie_files request - Movie ID: #{movie_id}, File IDs: #{inspect(renameable_file_ids)}" + ) + + Logger.debug("Radarr rename_movie_files JSON payload: #{inspect(json_payload)}") + + case request( + url: "/api/v3/command", + method: :post, + json: json_payload + ) do + {:ok, response} = result -> + Logger.debug("Radarr rename_movie_files response: #{inspect(response.body)}") + result + + {:error, reason} = error -> + Logger.error("Radarr rename_movie_files error: #{inspect(reason)}") + error + end + else + error_msgs = Enum.map(failures, fn {:error, msg} -> msg end) + {:error, "Failed to parse movie file IDs: #{Enum.join(error_msgs, ", ")}"} end end # Helper function to parse file IDs from various formats to integers - defp parse_file_id(value) when is_integer(value), do: value - defp parse_file_id(value) when is_binary(value), do: Parsers.parse_integer_exact!(value) + @spec parse_file_id(any()) :: {:ok, integer()} | {:error, String.t()} + defp parse_file_id(value) when is_integer(value), do: {:ok, value} + + defp parse_file_id(value) when is_binary(value) do + case Parsers.parse_integer_exact(value) do + {:ok, int} -> {:ok, int} + {:error, _} -> {:error, "invalid integer format"} + end + end defp parse_file_id(value) do - raise ArgumentError, "Expected integer or string, got: #{inspect(value)}" + {:error, "expected integer or string, got: #{inspect(value)}"} end end diff --git a/lib/reencodarr/services/sonarr.ex b/lib/reencodarr/services/sonarr.ex index 85f06a4f..eefa8ef8 100644 --- a/lib/reencodarr/services/sonarr.ex +++ b/lib/reencodarr/services/sonarr.ex @@ -132,19 +132,52 @@ defmodule Reencodarr.Services.Sonarr do end # Execute rename command and return result + @spec execute_rename_request(integer(), list(), list(map())) :: + {:ok, map()} | {:error, String.t()} defp execute_rename_request(series_id, file_ids, renameable_files) do - renameable_file_ids = + with {:ok, renameable_file_ids} <- parse_renameable_files(renameable_files), + {:ok, files_to_rename} <- determine_files_to_rename(file_ids, renameable_file_ids) do + execute_rename_api_request(series_id, files_to_rename) + end + end + + @spec parse_renameable_files(list(map())) :: {:ok, list(integer())} | {:error, String.t()} + defp parse_renameable_files(renameable_files) do + # Parse renameable file IDs from API response + parsed_renameable = renameable_files |> Enum.map(& &1["episodeFileId"]) |> Enum.map(&parse_file_id/1) - files_to_rename = - if file_ids == [] do - renameable_file_ids - else - file_ids |> Enum.map(&parse_file_id/1) - end + # Check for parsing errors in renameable files + {renameable_successes, renameable_failures} = + Enum.split_with(parsed_renameable, fn + {:ok, _} -> true + {:error, _} -> false + end) + + if Enum.empty?(renameable_failures) do + renameable_file_ids = Enum.map(renameable_successes, fn {:ok, id} -> id end) + {:ok, renameable_file_ids} + else + error_msgs = Enum.map(renameable_failures, fn {:error, msg} -> msg end) + {:error, "Failed to parse renameable file IDs: #{Enum.join(error_msgs, ", ")}"} + end + end + + @spec determine_files_to_rename(list(), list(integer())) :: + {:ok, list(integer())} | {:error, String.t()} + defp determine_files_to_rename(file_ids, renameable_file_ids) do + if file_ids == [] do + {:ok, renameable_file_ids} + else + parse_explicit_file_ids(file_ids) + end + end + @spec execute_rename_api_request(integer(), list(integer())) :: + {:ok, map()} | {:error, String.t()} + defp execute_rename_api_request(series_id, files_to_rename) do json_payload = %{name: "RenameFiles", seriesId: series_id, files: files_to_rename} Logger.info( @@ -169,10 +202,36 @@ defmodule Reencodarr.Services.Sonarr do end # Helper function to parse file IDs from various formats to integers - defp parse_file_id(value) when is_integer(value), do: value - defp parse_file_id(value) when is_binary(value), do: Parsers.parse_integer_exact!(value) + @spec parse_file_id(any()) :: {:ok, integer()} | {:error, String.t()} + defp parse_file_id(value) when is_integer(value), do: {:ok, value} + + defp parse_file_id(value) when is_binary(value) do + case Parsers.parse_integer_exact(value) do + {:ok, int} -> {:ok, int} + {:error, _} -> {:error, "invalid integer format"} + end + end defp parse_file_id(value) do - raise ArgumentError, "Expected integer or string, got: #{inspect(value)}" + {:error, "expected integer or string, got: #{inspect(value)}"} + end + + @spec parse_explicit_file_ids(list()) :: {:ok, list(integer())} | {:error, String.t()} + defp parse_explicit_file_ids(file_ids) do + parsed_explicit = file_ids |> Enum.map(&parse_file_id/1) + + {explicit_successes, explicit_failures} = + Enum.split_with(parsed_explicit, fn + {:ok, _} -> true + {:error, _} -> false + end) + + if Enum.empty?(explicit_failures) do + parsed_ids = Enum.map(explicit_successes, fn {:ok, id} -> id end) + {:ok, parsed_ids} + else + error_msgs = Enum.map(explicit_failures, fn {:error, msg} -> msg end) + {:error, "Failed to parse explicit file IDs: #{Enum.join(error_msgs, ", ")}"} + end end end diff --git a/lib/reencodarr_web/dashboard/presenter.ex b/lib/reencodarr_web/dashboard/presenter.ex index f4e05773..cd576510 100644 --- a/lib/reencodarr_web/dashboard/presenter.ex +++ b/lib/reencodarr_web/dashboard/presenter.ex @@ -24,10 +24,11 @@ defmodule ReencodarrWeb.Dashboard.Presenter do @cache_table :presenter_cache def start_cache do - :ets.new(@cache_table, [:set, :public, :named_table]) - rescue - # Table already exists - ArgumentError -> :ok + case :ets.whereis(@cache_table) do + :undefined -> :ets.new(@cache_table, [:set, :public, :named_table]) + # Table already exists + _ -> :ok + end end def present(dashboard_state), do: present(dashboard_state, "UTC") diff --git a/lib/reencodarr_web/live/components/manual_scan_component.ex b/lib/reencodarr_web/live/components/manual_scan_component.ex index 718a5264..a150ecff 100644 --- a/lib/reencodarr_web/live/components/manual_scan_component.ex +++ b/lib/reencodarr_web/live/components/manual_scan_component.ex @@ -85,13 +85,7 @@ defmodule ReencodarrWeb.ManualScanComponent do parent_pid = self() Task.start(fn -> - result = - try do - Reencodarr.ManualScanner.scan(path) - :ok - rescue - error -> {:error, error} - end + result = Reencodarr.ManualScanner.scan(path) # Send result to parent LiveView send(parent_pid, {:manual_scan_completed, result}) diff --git a/lib/reencodarr_web/live/dashboard_live.ex b/lib/reencodarr_web/live/dashboard_live.ex index 227bab6b..646e2bee 100644 --- a/lib/reencodarr_web/live/dashboard_live.ex +++ b/lib/reencodarr_web/live/dashboard_live.ex @@ -248,10 +248,6 @@ defmodule ReencodarrWeb.DashboardLive do |> stream(:crf_search_queue, crf_search_items, reset: true) |> stream(:encoding_queue, encoding_items, reset: true) |> stream(:analyzer_queue, analyzer_items, reset: true) - rescue - error -> - Logger.warning("Failed to update queue streams: #{inspect(error)}") - socket end defp generate_stream_items(files, prefix) when is_list(files) do @@ -265,12 +261,15 @@ defmodule ReencodarrWeb.DashboardLive do defp generate_stream_items(_, _), do: [] - # Safe state retrieval functions - defp get_safe_full_state, do: safe_call(&DashboardLiveHelpers.get_initial_state/0) - defp present_state(state, timezone), do: safe_call(fn -> Presenter.present(state, timezone) end) + # State retrieval functions + defp get_safe_full_state do + {:ok, DashboardLiveHelpers.get_initial_state()} + rescue + error -> {:error, error} + end - defp safe_call(func) do - {:ok, func.()} + defp present_state(state, timezone) do + {:ok, Presenter.present(state, timezone)} rescue error -> {:error, error} end diff --git a/lib/reencodarr_web/live/failures_live.ex b/lib/reencodarr_web/live/failures_live.ex index e45c9f45..7c756b98 100644 --- a/lib/reencodarr_web/live/failures_live.ex +++ b/lib/reencodarr_web/live/failures_live.ex @@ -889,78 +889,103 @@ defmodule ReencodarrWeb.FailuresLive do defp get_failed_videos_paginated(page, per_page, stage_filter, category_filter, search_term) do import Ecto.Query - base_query = - from v in Reencodarr.Media.Video, - where: v.state == :failed - - # Apply stage and category filters by joining with failures table - filtered_query = - if stage_filter != "all" or category_filter != "all" do - query = - from v in base_query, - join: f in Reencodarr.Media.VideoFailure, - on: f.video_id == v.id, - where: f.resolved == false, - distinct: true - - query = - if stage_filter != "all" do - stage_atom = String.to_atom(stage_filter) - from [v, f] in query, where: f.failure_stage == ^stage_atom - else - query - end - - query = - if category_filter != "all" do - category_atom = String.to_atom(category_filter) - from [v, f] in query, where: f.failure_category == ^category_atom - else - query - end - - query - else - base_query - end + base_query = from(v in Reencodarr.Media.Video, where: v.state == :failed) - # Apply search - searched_query = - if search_term != "" do - search_pattern = "%#{search_term}%" + base_query + |> apply_failure_filters(stage_filter, category_filter) + |> apply_search_filter(search_term) + |> apply_ordering() + |> get_paginated_results(page, per_page) + end - case_insensitive_like_condition = - SharedQueries.case_insensitive_like(:path, search_pattern) + defp apply_failure_filters(base_query, stage_filter, category_filter) do + if stage_filter != "all" or category_filter != "all" do + query = + from v in base_query, + join: f in Reencodarr.Media.VideoFailure, + on: f.video_id == v.id, + where: f.resolved == false, + distinct: true + + query + |> apply_stage_filter(stage_filter) + |> apply_category_filter(category_filter) + else + base_query + end + end - from v in filtered_query, where: ^case_insensitive_like_condition - else - filtered_query - end + defp apply_stage_filter(query, "all"), do: query - # Order by most recent first - ordered_query = from v in searched_query, order_by: [desc: v.inserted_at] - - # Get total count - for SQLite compatibility, we need to handle GROUP BY queries differently - total_count = - case has_group_by?(searched_query) do - true -> - # When we have GROUP BY, we need to count the grouped results - # instead of using aggregate which tries to add DISTINCT - subquery = from v in searched_query, select: v.id - Repo.all(subquery) |> length() - - false -> - # No GROUP BY, safe to use aggregate - Repo.aggregate(ordered_query, :count, :id) - end + defp apply_stage_filter(query, stage_filter) do + case parse_stage_filter(stage_filter) do + {:ok, stage_atom} -> + from [v, f] in query, where: f.failure_stage == ^stage_atom - # Get paginated results - offset = (page - 1) * per_page - videos = Repo.all(from v in ordered_query, limit: ^per_page, offset: ^offset) + {:error, _reason} -> + # Invalid stage filter, return no results + from [v, f] in query, where: false + end + end + + defp apply_category_filter(query, "all"), do: query + defp apply_category_filter(query, category_filter) do + case parse_category_filter(category_filter) do + {:ok, category_atom} -> + from [v, f] in query, where: f.failure_category == ^category_atom + + {:error, _reason} -> + # Invalid category filter, return no results + from [v, f] in query, where: false + end + end + + defp parse_stage_filter("analysis"), do: {:ok, :analysis} + defp parse_stage_filter("crf_search"), do: {:ok, :crf_search} + defp parse_stage_filter("encoding"), do: {:ok, :encoding} + defp parse_stage_filter(invalid), do: {:error, "Invalid stage filter: #{inspect(invalid)}"} + + defp parse_category_filter("system"), do: {:ok, :system} + defp parse_category_filter("media"), do: {:ok, :media} + defp parse_category_filter("network"), do: {:ok, :network} + defp parse_category_filter("configuration"), do: {:ok, :configuration} + + defp parse_category_filter(invalid), + do: {:error, "Invalid category filter: #{inspect(invalid)}"} + + defp apply_search_filter(query, ""), do: query + + defp apply_search_filter(query, search_term) do + search_pattern = "%#{search_term}%" + case_insensitive_like_condition = SharedQueries.case_insensitive_like(:path, search_pattern) + from v in query, where: ^case_insensitive_like_condition + end + + defp apply_ordering(query) do + from v in query, order_by: [desc: v.inserted_at] + end + + defp get_paginated_results(query, page, per_page) do + total_count = get_total_count(query) + offset = (page - 1) * per_page + videos = Repo.all(from v in query, limit: ^per_page, offset: ^offset) {videos, total_count} end + defp get_total_count(query) do + case has_group_by?(query) do + true -> + # When we have GROUP BY, we need to count the grouped results + subquery = from v in query, select: v.id + Repo.all(subquery) |> length() + + false -> + # No GROUP BY, safe to use aggregate + Repo.aggregate(query, :count, :id) + end + end + # Helper function to check if a query has GROUP BY clause defp has_group_by?(%Ecto.Query{group_bys: group_bys}), do: length(group_bys) > 0 diff --git a/test/reencodarr/ab_av1/crf_search_retry_test.exs b/test/reencodarr/ab_av1/crf_search_retry_test.exs index 1ef9b5b6..4cd83f17 100644 --- a/test/reencodarr/ab_av1/crf_search_retry_test.exs +++ b/test/reencodarr/ab_av1/crf_search_retry_test.exs @@ -12,9 +12,8 @@ defmodule Reencodarr.AbAv1.CrfSearchRetryTest do describe "CRF search retry mechanism" do setup do # Clean up any existing mocks - try do - :meck.unload() - catch + case :meck.unload() do + :ok -> :ok _ -> :ok end diff --git a/test/reencodarr/analyzer/broadway/error_handling_test.exs b/test/reencodarr/analyzer/broadway/error_handling_test.exs index e868f56e..8eb9b426 100644 --- a/test/reencodarr/analyzer/broadway/error_handling_test.exs +++ b/test/reencodarr/analyzer/broadway/error_handling_test.exs @@ -144,12 +144,8 @@ defmodule Reencodarr.Analyzer.Broadway.ErrorHandlingTest do log = capture_log(fn -> Enum.each(video_infos, fn _video_info -> - try do - # Instead of the removed process_path/1, test Broadway dispatch - Broadway.dispatch_available() - rescue - _ -> :ok - end + # Instead of the removed process_path/1, test Broadway dispatch + Broadway.dispatch_available() end) # Give time for processing @@ -173,23 +169,19 @@ defmodule Reencodarr.Analyzer.Broadway.ErrorHandlingTest do log = capture_log(fn -> - try do - Broadway.pause() - Process.sleep(100) - paused_running = Broadway.running?() - - Broadway.resume() - Process.sleep(100) - resumed_running = Broadway.running?() - - # The exact values depend on whether the pipeline is actually running - # in the test environment, but the calls should not crash - assert is_boolean(initial_running) - assert is_boolean(paused_running) - assert is_boolean(resumed_running) - rescue - _ -> :ok - end + Broadway.pause() + Process.sleep(100) + paused_running = Broadway.running?() + + Broadway.resume() + Process.sleep(100) + resumed_running = Broadway.running?() + + # The exact values depend on whether the pipeline is actually running + # in the test environment, but the calls should not crash + assert is_boolean(initial_running) + assert is_boolean(paused_running) + assert is_boolean(resumed_running) end) assert is_binary(log) diff --git a/test/reencodarr/sync_integration_test.exs b/test/reencodarr/sync_integration_test.exs index 828050df..7f07720d 100644 --- a/test/reencodarr/sync_integration_test.exs +++ b/test/reencodarr/sync_integration_test.exs @@ -272,28 +272,18 @@ defmodule Reencodarr.SyncIntegrationTest do log = capture_log(fn -> # Test Sonarr refresh - try do - result = Sync.refresh_operations("123", :sonarr) - # Should attempt the operation (may fail without actual service) - assert result == {:error, :econnrefused} or - match?({:ok, _}, result) or - match?({:error, _}, result) - rescue - # Expected in test environment - _ -> :ok - end + result = Sync.refresh_operations("123", :sonarr) + # Should attempt the operation (may fail without actual service) + assert result == {:error, :econnrefused} or + match?({:ok, _}, result) or + match?({:error, _}, result) # Test Radarr refresh - try do - result = Sync.refresh_operations("456", :radarr) - # Should attempt the operation (may fail without actual service) - assert result == {:error, :econnrefused} or - match?({:ok, _}, result) or - match?({:error, _}, result) - rescue - # Expected in test environment - _ -> :ok - end + result = Sync.refresh_operations("456", :radarr) + # Should attempt the operation (may fail without actual service) + assert result == {:error, :econnrefused} or + match?({:ok, _}, result) or + match?({:error, _}, result) end) # Should log the refresh attempts or connection errors @@ -359,16 +349,11 @@ defmodule Reencodarr.SyncIntegrationTest do test "rescan_and_rename_series delegates correctly" do log = capture_log(fn -> - try do - result = Sync.rescan_and_rename_series("789") - # Should delegate to refresh_operations - assert result == {:error, :econnrefused} or - match?({:ok, _}, result) or - match?({:error, _}, result) - rescue - # Expected in test environment - _ -> :ok - end + result = Sync.rescan_and_rename_series("789") + # Should delegate to refresh_operations + assert result == {:error, :econnrefused} or + match?({:ok, _}, result) or + match?({:error, _}, result) end) # Should log the operation attempt or connection errors diff --git a/test/reencodarr/video_processing_pipeline_test.exs b/test/reencodarr/video_processing_pipeline_test.exs index 1689cfe3..3e9217b6 100644 --- a/test/reencodarr/video_processing_pipeline_test.exs +++ b/test/reencodarr/video_processing_pipeline_test.exs @@ -191,22 +191,22 @@ defmodule Reencodarr.VideoProcessingPipelineTest do # Mock FileOperations to simulate cross-device scenario with proper cleanup :meck.new(FileOperations, [:passthrough]) - try do - :meck.expect(FileOperations, :move_file, fn src, dest, context, _video -> - case context do - "IntermediateMove" -> - # Simulate cross-device move requiring copy+delete - File.cp!(src, dest) - File.rm!(src) - :ok - - "FinalRename" -> - # Normal rename - File.rename!(src, dest) - :ok - end - end) + :meck.expect(FileOperations, :move_file, fn src, dest, context, _video -> + case context do + "IntermediateMove" -> + # Simulate cross-device move requiring copy+delete + File.cp!(src, dest) + File.rm!(src) + :ok + + "FinalRename" -> + # Normal rename + File.rename!(src, dest) + :ok + end + end) + try do capture_log(fn -> result = PostProcessor.process_encoding_success(video, encoded_output) assert {:ok, :success} = result @@ -322,30 +322,28 @@ defmodule Reencodarr.VideoProcessingPipelineTest do # Test database transaction rollback on failure with proper mock cleanup :meck.new(Media, [:passthrough]) - try do - :meck.expect(Media, :mark_as_reencoded, fn _video -> - {:error, :database_connection_lost} - end) - - log = - capture_log(fn -> - result = PostProcessor.process_encoding_success(video, encoded_output) - # Should still succeed overall - assert {:ok, :success} = result - end) + :meck.expect(Media, :mark_as_reencoded, fn _video -> + {:error, :database_connection_lost} + end) - # Video should remain in original state due to transaction handling - unchanged_video = Media.get_video!(video.id) - # The mock prevents the state change, so the video should remain unchanged - # In the state machine approach, reencoded is only true when state is :encoded - assert unchanged_video.state != :encoded - assert unchanged_video.state == :needs_analysis - assert unchanged_video.state != :failed + log = + capture_log(fn -> + result = PostProcessor.process_encoding_success(video, encoded_output) + # Should still succeed overall + assert {:ok, :success} = result + end) - assert log =~ "Failed to mark video #{video.id} as re-encoded" - after - :meck.unload(Media) - end + # Video should remain in original state due to transaction handling + unchanged_video = Media.get_video!(video.id) + # The mock prevents the state change, so the video should remain unchanged + # In the state machine approach, reencoded is only true when state is :encoded + assert unchanged_video.state != :encoded + assert unchanged_video.state == :needs_analysis + assert unchanged_video.state != :failed + + assert log =~ "Failed to mark video #{video.id} as re-encoded" + # Ensure Media mock is cleaned up + :meck.unload(Media) end defp calculate_savings(original_size, percent) do diff --git a/test/support/test_helpers.ex b/test/support/test_helpers.ex index 182ecc7e..dcb44048 100644 --- a/test/support/test_helpers.ex +++ b/test/support/test_helpers.ex @@ -281,8 +281,9 @@ defmodule Reencodarr.TestHelpers do unique_id = System.unique_integer([:positive]) temp_file = Path.join(temp_dir, "test_file_#{unique_id}#{extension}") + File.write!(temp_file, content) + try do - File.write!(temp_file, content) fun.(temp_file) after File.rm(temp_file) @@ -297,9 +298,5 @@ defmodule Reencodarr.TestHelpers do def test_broadway_error_handling(_broadway_module, _message) do # Trigger Broadway dispatch to test error handling AnalyzerBroadway.dispatch_available() - catch - kind, reason -> - # Broadway should handle errors gracefully, so catching here indicates a problem - flunk("Broadway pipeline crashed: #{inspect(kind)} #{inspect(reason)}") end end