Skip to content

Commit bee3f1b

Browse files
mjcclaude
andcommitted
Fix antipatterns: remove rescue blocks and sleep-based timing in tests
DB Queries: - Removed safe_query_list/2 and safe_chart_query/2 catch-all blocks - Let DB connection pool handle retries via timeouts (existing mechanism) - Added rescue handlers in :refresh_stats, :refresh_queues, :refresh_charts to log and reschedule on failure, preventing GenServer crash - DB errors now logged but don't interrupt service Tests: - Removed :timer.sleep(50) from "sync_sonarr shows error when sync already in progress" - Instead, call render/1 after sending message to process it synchronously - No more timing-dependent flaky tests This leverages the existing DB timeout and connection pooling retry logic instead of catching errors at the GenServer level. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1 parent 4f5cf35 commit bee3f1b

2 files changed

Lines changed: 30 additions & 46 deletions

File tree

lib/reencodarr/dashboard/state.ex

Lines changed: 26 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,11 @@ defmodule Reencodarr.Dashboard.State do
287287
broadcast_state(state)
288288
Process.send_after(self(), :refresh_stats, @stats_refresh_interval)
289289
{:noreply, state}
290+
rescue
291+
error ->
292+
Logger.warning("Dashboard.State refresh_stats failed: #{inspect(error)}")
293+
Process.send_after(self(), :refresh_stats, @stats_refresh_interval)
294+
{:noreply, state}
290295
end
291296

292297
@impl true
@@ -299,6 +304,15 @@ defmodule Reencodarr.Dashboard.State do
299304
end
300305

301306
{:noreply, state}
307+
rescue
308+
error ->
309+
Logger.warning("Dashboard.State refresh_queues failed: #{inspect(error)}")
310+
311+
if queue_refresh_enabled?() do
312+
Process.send_after(self(), :refresh_queues, @queue_refresh_interval)
313+
end
314+
315+
{:noreply, state}
302316
end
303317

304318
@impl true
@@ -315,6 +329,11 @@ defmodule Reencodarr.Dashboard.State do
315329
broadcast_state(state)
316330
Process.send_after(self(), :refresh_charts, @chart_refresh_interval)
317331
{:noreply, state}
332+
rescue
333+
error ->
334+
Logger.warning("Dashboard.State refresh_charts failed: #{inspect(error)}")
335+
Process.send_after(self(), :refresh_charts, @chart_refresh_interval)
336+
{:noreply, state}
318337
end
319338

320339
# Catch-all for unknown messages
@@ -330,42 +349,14 @@ defmodule Reencodarr.Dashboard.State do
330349
Phoenix.PubSub.broadcast(Reencodarr.PubSub, @state_channel, {:dashboard_state_changed, state})
331350
end
332351

333-
defp fetch_queue_items(current_items) do
352+
defp fetch_queue_items(_current_items) do
334353
%{
335-
analyzer:
336-
safe_query_list(
337-
fn ->
338-
VideoQueries.videos_needing_analysis(5, timeout: @query_timeout)
339-
end,
340-
current_items.analyzer
341-
),
342-
crf_searcher:
343-
safe_query_list(
344-
fn ->
345-
VideoQueries.videos_for_crf_search(5, timeout: @query_timeout)
346-
end,
347-
current_items.crf_searcher
348-
),
349-
encoder:
350-
safe_query_list(
351-
fn ->
352-
VideoQueries.videos_ready_for_encoding(5, timeout: @query_timeout)
353-
end,
354-
current_items.encoder
355-
)
354+
analyzer: VideoQueries.videos_needing_analysis(5, timeout: @query_timeout),
355+
crf_searcher: VideoQueries.videos_for_crf_search(5, timeout: @query_timeout),
356+
encoder: VideoQueries.videos_ready_for_encoding(5, timeout: @query_timeout)
356357
}
357358
end
358359

359-
defp safe_query_list(fun, fallback) do
360-
fun.()
361-
rescue
362-
DBConnection.ConnectionError -> fallback
363-
catch
364-
:exit, {:timeout, _} -> fallback
365-
:exit, {%DBConnection.ConnectionError{}, _} -> fallback
366-
:exit, {{%DBConnection.ConnectionError{}, _}, _} -> fallback
367-
end
368-
369360
defp queue_refresh_enabled? do
370361
Application.get_env(:reencodarr, :dashboard_queue_refresh_enabled, true)
371362
end
@@ -382,19 +373,9 @@ defmodule Reencodarr.Dashboard.State do
382373

383374
defp load_chart_data do
384375
%{
385-
vmaf: safe_chart_query(fn -> ChartQueries.vmaf_score_distribution() end, []),
386-
resolution: safe_chart_query(fn -> ChartQueries.resolution_distribution() end, []),
387-
codec: safe_chart_query(fn -> ChartQueries.codec_distribution() end, [])
376+
vmaf: ChartQueries.vmaf_score_distribution(),
377+
resolution: ChartQueries.resolution_distribution(),
378+
codec: ChartQueries.codec_distribution()
388379
}
389380
end
390-
391-
defp safe_chart_query(fun, fallback) do
392-
fun.()
393-
rescue
394-
DBConnection.ConnectionError -> fallback
395-
catch
396-
:exit, {:timeout, _} -> fallback
397-
:exit, {%DBConnection.ConnectionError{}, _} -> fallback
398-
:exit, {{%DBConnection.ConnectionError{}, _}, _} -> fallback
399-
end
400381
end

test/reencodarr_web/live/dashboard_v2_live_test.exs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,8 +287,11 @@ defmodule ReencodarrWeb.DashboardLiveTest do
287287
test "sync_sonarr shows error when sync already in progress", %{conn: conn} do
288288
{:ok, view, _} = live(conn, ~p"/")
289289

290+
# Manually update the view to mark sync as in progress
290291
send(view.pid, {:sync_started, %{service_type: "sonarr"}})
291-
:timer.sleep(50)
292+
293+
# Render to process the message, no sleep needed
294+
_html = render(view)
292295

293296
html = view |> render_click("sync_sonarr", %{})
294297

0 commit comments

Comments
 (0)