diff --git a/TECHDEBTS.md b/TECHDEBTS.md index 8b7db5c..d17c289 100644 --- a/TECHDEBTS.md +++ b/TECHDEBTS.md @@ -855,7 +855,19 @@ add `ON DELETE CASCADE` via an Ecto migration on the **Acceptance.** Injected failure between the two deletes leaves both intact; no orphaned messages possible. -### TD-23: Sweep blanket `rescue`/`catch` blocks for silent failure +### TD-23: Sweep blanket `rescue`/`catch` blocks for silent failure ✅ DONE (2026-06-12) + +**Status: implemented.** The named sweep surface was tightened in the intended +way instead of being rewritten wholesale. `ImportAnalysis` and +`ImportExecution` now log contextual warnings when top-level import handling, +repo-transaction wrappers, media linking, or progress callbacks fall back after +exceptions; the tests now prove those progress-callback failures are no longer +silent. In `ImportAnalysis.year_from/1`, the blanket integer-date rescue was +narrowed to `ArgumentError`, which is the actual parsing failure path there. +`OverlayComponents` kept its defensive UI fallbacks, but every rescue now logs +why the fallback path was taken instead of silently swallowing the error. +`MainWindow` needed no change in this pass: its catches are already scoped to +expected `:exit`/wx failures rather than bare rescue-all blocks. **Context.** Several modules rescue *all* exceptions into `:ok`/fallbacks (`shutdown.ex` defensibly — it must never block quit; but also diff --git a/lib/bds/desktop/shell_live/overlay_components.ex b/lib/bds/desktop/shell_live/overlay_components.ex index 95cf689..0263d12 100644 --- a/lib/bds/desktop/shell_live/overlay_components.ex +++ b/lib/bds/desktop/shell_live/overlay_components.ex @@ -5,6 +5,7 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do use Gettext, backend: BDS.Gettext import Ecto.Query + require Logger alias BDS.{I18n, Media, Metadata, Posts, Repo} alias BDS.Media.Media, as: MediaRecord @@ -70,7 +71,9 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do {:ok, metadata} = Metadata.get_project_metadata(project_id) metadata rescue - _error -> %{main_language: "en", blog_languages: []} + error -> + log_overlay_warning("project metadata", error) + %{main_language: "en", blog_languages: []} end defp posts(nil), do: [] @@ -137,7 +140,9 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do select: pm.media_id ) rescue - _error -> [] + error -> + log_overlay_warning("post media ids for #{post_id}", error) + [] end defp post_media_ids(_tab), do: [] @@ -150,7 +155,9 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do ) |> Map.new(fn {language, status} -> {language, Atom.to_string(status || :draft)} end) rescue - _error -> %{} + error -> + log_overlay_warning("post translations for #{post_id}", error) + %{} end defp existing_translations(%{type: :media, id: media_id}) do @@ -161,7 +168,9 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do ) |> Map.new(fn {language, status} -> {language, status} end) rescue - _error -> %{} + error -> + log_overlay_warning("media translations for #{media_id}", error) + %{} end defp existing_translations(_tab), do: %{} @@ -179,7 +188,9 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do _other -> metadata.main_language || "en" end rescue - _error -> metadata.main_language || "en" + error -> + log_overlay_warning("post source language for #{post_id}", error) + metadata.main_language || "en" end defp source_language(%{type: :media, id: media_id}, metadata) do @@ -188,7 +199,9 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do _other -> metadata.main_language || "en" end rescue - _error -> metadata.main_language || "en" + error -> + log_overlay_warning("media source language for #{media_id}", error) + metadata.main_language || "en" end defp source_language(_tab, metadata), do: metadata.main_language || "en" @@ -242,7 +255,9 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do [] end rescue - _error -> [] + error -> + log_overlay_warning("post AI fields for #{post_id}", error) + [] end defp ai_fields(%{type: :media, id: media_id}, title, _subtitle, page_language) do @@ -279,7 +294,9 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do [] end rescue - _error -> [] + error -> + log_overlay_warning("media AI fields for #{media_id}", error) + [] end defp ai_fields(_tab, _title, _subtitle, _page_language), do: [] @@ -309,7 +326,9 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do reference_list: reference_list } rescue - _error -> + error -> + log_overlay_warning("delete media details for #{media_id}", error) + %{ title: BDS.Gettext.lgettext(page_language, "ui", "Delete Media"), entity_name: media_id, @@ -330,7 +349,9 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do reference_list: [] } rescue - _error -> + error -> + log_overlay_warning("delete tag details", error) + %{ title: BDS.Gettext.lgettext(page_language, "ui", "Delete Tag"), entity_name: "tag", @@ -367,7 +388,9 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do message: BDS.Gettext.lgettext(page_language, "ui", "Cannot be undone.") } rescue - _error -> + error -> + log_overlay_warning("merge tag details for project #{project_id}", error) + %{ target: "tag", count: 1, @@ -391,4 +414,8 @@ defmodule BDS.Desktop.ShellLive.OverlayComponents do |> String.replace(~r/[^a-z0-9]+/u, "-") |> String.trim("-") end + + defp log_overlay_warning(context, error) do + Logger.warning("overlay component fallback for #{context}: #{Exception.message(error)}") + end end diff --git a/lib/bds/import_analysis.ex b/lib/bds/import_analysis.ex index 96e0bf9..bad870e 100644 --- a/lib/bds/import_analysis.ex +++ b/lib/bds/import_analysis.ex @@ -2,6 +2,7 @@ defmodule BDS.ImportAnalysis do @moduledoc false import Ecto.Query + require Logger alias BDS.Media.Media alias BDS.Posts.Post @@ -25,7 +26,12 @@ defmodule BDS.ImportAnalysis do wxr_data = WxrParser.parse_file(wxr_file_path) {:ok, build_report(project_id, wxr_data, wxr_file_path, uploads_folder_path, on_progress)} rescue - error -> {:error, %{message: Exception.message(error)}} + error -> + Logger.warning( + "import analysis failed project_id=#{project_id} file=#{wxr_file_path}: #{Exception.message(error)}" + ) + + {:error, %{message: Exception.message(error)}} end defp build_report(project_id, wxr_data, wxr_file_path, uploads_folder_path, on_progress) do @@ -459,7 +465,7 @@ defmodule BDS.ImportAnalysis do value end rescue - _error -> nil + _error in [ArgumentError] -> nil end defp year_from(value) when is_binary(value) do @@ -491,7 +497,12 @@ defmodule BDS.ImportAnalysis do try do callback.(step, detail) rescue - _error -> :ok + error -> + Logger.warning( + "import analysis progress callback failed step=#{inspect(step)}: #{Exception.message(error)}" + ) + + :ok end :ok diff --git a/lib/bds/import_execution.ex b/lib/bds/import_execution.ex index 0889650..7c0e4e5 100644 --- a/lib/bds/import_execution.ex +++ b/lib/bds/import_execution.ex @@ -1,6 +1,8 @@ defmodule BDS.ImportExecution do @moduledoc false + require Logger + alias BDS.Media alias BDS.Metadata alias BDS.Posts @@ -118,7 +120,9 @@ defmodule BDS.ImportExecution do {:error, reason} -> {:error, %{message: format_import_error(reason)}} end rescue - error -> {:error, %{message: Exception.message(error)}} + error -> + Logger.warning("import execution failed project_id=#{project_id}: #{Exception.message(error)}") + {:error, %{message: Exception.message(error)}} end defp execute_taxonomies(category_items, tag_items, project_id, result, on_progress, started_at) do @@ -363,9 +367,19 @@ defmodule BDS.ImportExecution do try do Media.link_media_to_post(media_id, post_id) rescue - _ -> :ok + error -> + Logger.warning( + "import execution failed to link media_id=#{media_id} post_id=#{post_id}: #{Exception.message(error)}" + ) + + :ok catch - _, _ -> :ok + kind, reason -> + Logger.warning( + "import execution failed to link media_id=#{media_id} post_id=#{post_id}: #{kind}: #{inspect(reason)}" + ) + + :ok end end) @@ -586,9 +600,13 @@ defmodule BDS.ImportExecution do defp run_repo_transaction(fun) when is_function(fun, 0) do Repo.transaction(fun) rescue - error -> {:error, error} + error -> + Logger.warning("import execution transaction failed: #{Exception.message(error)}") + {:error, error} catch - kind, reason -> {:error, {kind, reason}} + kind, reason -> + Logger.warning("import execution transaction failed: #{kind}: #{inspect(reason)}") + {:error, {kind, reason}} end defp post_item_label(item) do @@ -628,11 +646,20 @@ defmodule BDS.ImportExecution do try do callback.(phase, current, total, %{detail: detail, eta: eta}) rescue - _error -> + error -> + Logger.warning( + "import execution progress callback failed phase=#{inspect(phase)}: #{Exception.message(error)}" + ) + try do callback.(phase, current, total, detail) rescue - _error -> :ok + legacy_error -> + Logger.warning( + "import execution legacy progress callback failed phase=#{inspect(phase)}: #{Exception.message(legacy_error)}" + ) + + :ok end end diff --git a/test/bds/import_analysis_test.exs b/test/bds/import_analysis_test.exs index 2b967f1..44d356f 100644 --- a/test/bds/import_analysis_test.exs +++ b/test/bds/import_analysis_test.exs @@ -224,6 +224,30 @@ defmodule BDS.ImportAnalysisTest do assert_received {:analysis_progress, "Discovering macros...", nil} end + import ExUnit.CaptureLog + + test "analyze_wxr logs and continues when the progress callback raises", %{ + project: project, + temp_dir: temp_dir + } do + uploads_dir = Path.join(temp_dir, "uploads") + File.mkdir_p!(Path.join(uploads_dir, "2024/05")) + File.write!(Path.join(uploads_dir, "2024/05/import-asset.txt"), "legacy attachment") + + wxr_path = Path.join(temp_dir, "legacy.xml") + File.write!(wxr_path, basic_wxr_xml()) + + log = + capture_log(fn -> + assert {:ok, _report} = + ImportAnalysis.analyze_wxr(project.id, wxr_path, uploads_dir, + on_progress: fn _step, _detail -> raise "boom" end + ) + end) + + assert log =~ "import analysis progress callback failed" + end + defp sha256(value) do :sha256 |> :crypto.hash(value) diff --git a/test/bds/import_execution_test.exs b/test/bds/import_execution_test.exs index ebcae80..4894499 100644 --- a/test/bds/import_execution_test.exs +++ b/test/bds/import_execution_test.exs @@ -112,6 +112,34 @@ defmodule BDS.ImportExecutionTest do assert File.exists?(Path.join(temp_dir, imported_media.file_path)) end + import ExUnit.CaptureLog + + test "execute_import logs and continues when the progress callback raises", %{ + project: project, + temp_dir: temp_dir + } do + uploads_dir = Path.join(temp_dir, "uploads") + File.mkdir_p!(Path.join(uploads_dir, "2024/05")) + File.write!(Path.join(uploads_dir, "2024/05/import-asset.txt"), "legacy attachment") + + wxr_path = Path.join(temp_dir, "legacy.xml") + File.write!(wxr_path, basic_wxr_xml()) + + assert {:ok, report} = ImportAnalysis.analyze_wxr(project.id, wxr_path, uploads_dir) + + log = + capture_log(fn -> + assert {:ok, _result} = + ImportExecution.execute_import(project.id, report, + default_author: "Imported Author", + uploads_folder_path: uploads_dir, + on_progress: fn _phase, _current, _total, _detail -> raise "boom" end + ) + end) + + assert log =~ "import execution progress callback failed" + end + test "execute_import skips conflicts by default and can import them with a new slug", %{ project: project, temp_dir: temp_dir