From 2632649cdc499dd95b5d12853c4800d4b3aa104f Mon Sep 17 00:00:00 2001
From: Chili Palmer <hugoms@me.com>
Date: Mon, 11 May 2026 20:09:49 +0200
Subject: [PATCH] fix(rendering): replace raising read_template_file with
 try_read in macro templates (CSM-028)

---
 CODESMELL.md                          | 13 +++++---
 lib/bds/rendering/filters.ex          | 44 ++++++++++++++++-----------
 test/bds/csm028_broad_rescue_test.exs | 22 ++++++++++++++
 3 files changed, 58 insertions(+), 21 deletions(-)
 create mode 100644 test/bds/csm028_broad_rescue_test.exs

diff --git a/CODESMELL.md b/CODESMELL.md
index ccc703c..6e21aaf 100644
--- a/CODESMELL.md
+++ b/CODESMELL.md
@@ -422,10 +422,15 @@
 
 ---
 
-### CSM-028 — Broad `rescue` Swallowing Template Errors
-- **File:** `lib/bds/rendering/filters.ex:130-132`
-- **What:** `rescue _error -> ""` swallows all macro template failures silently.
-- **Fix:** Rescue only specific exceptions, or return `{:error, exception}` and let the caller decide.
+### ~~CSM-028 — Broad `rescue` Swallowing Template Errors~~ ✅ FIXED
+- **Fixed:** 2026-05-11
+- **What was done:**
+  - Replaced `Liquex.FileSystem.read_template_file` (which raises on missing templates) with `BDS.Rendering.FileSystem.try_read` (returns `{:ok, source}` / `{:error, :enoent}`).
+  - Missing template now logs a warning and returns `""` without raising.
+  - Extracted `render_macro_source/4` to separate file reading from template parsing/rendering.
+  - `Liquex.render!` rescue remains specific to `Liquex.Error` (no non-bang variant exists in Liquex).
+  - No broad `rescue _error ->` or `rescue _ ->` clauses remain in `filters.ex`.
+  - Added 3 source-level tests in `test/bds/csm028_broad_rescue_test.exs`: no broad rescue clauses, no `read_template_file` usage, `try_read` is used instead.
 
 ---
 
diff --git a/lib/bds/rendering/filters.ex b/lib/bds/rendering/filters.ex
index fbc9561..9813274 100644
--- a/lib/bds/rendering/filters.ex
+++ b/lib/bds/rendering/filters.ex
@@ -135,30 +135,40 @@ defmodule BDS.Rendering.Filters do
         ""
 
       _id ->
-        template_source = Liquex.FileSystem.read_template_file(context.file_system, template_path)
+        case BDS.Rendering.FileSystem.try_read(context.file_system, template_path) do
+          {:ok, template_source} ->
+            render_macro_source(template_path, template_source, assigns, context)
 
-        case Liquex.parse(template_source) do
-          {:ok, template_ast} ->
-            isolated_context = Liquex.Context.new_isolated_subscope(context, assigns)
-
-            try do
-              {result, _context} = Liquex.render!(template_ast, isolated_context)
-              IO.iodata_to_binary(result)
-            rescue
-              e in Liquex.Error ->
-                require Logger
-                Logger.warning("Macro template render failed (#{template_path}): #{e.message}")
-                ""
-            end
-
-          {:error, reason, line} ->
+          {:error, :enoent} ->
             require Logger
-            Logger.warning("Macro template parse failed (#{template_path}): #{reason} at line #{line}")
+            Logger.warning("Macro template not found: #{template_path}")
             ""
         end
     end
   end
 
+  defp render_macro_source(template_path, template_source, assigns, context) do
+    case Liquex.parse(template_source) do
+      {:ok, template_ast} ->
+        isolated_context = Liquex.Context.new_isolated_subscope(context, assigns)
+
+        try do
+          {result, _context} = Liquex.render!(template_ast, isolated_context)
+          IO.iodata_to_binary(result)
+        rescue
+          e in Liquex.Error ->
+            require Logger
+            Logger.warning("Macro template render failed (#{template_path}): #{e.message}")
+            ""
+        end
+
+      {:error, reason, line} ->
+        require Logger
+        Logger.warning("Macro template parse failed (#{template_path}): #{reason} at line #{line}")
+        ""
+    end
+  end
+
   defp render_markdown_html(markdown) do
     case Earmark.as_html(markdown) do
       {:ok, html, _messages} -> html
diff --git a/test/bds/csm028_broad_rescue_test.exs b/test/bds/csm028_broad_rescue_test.exs
new file mode 100644
index 0000000..d1798e4
--- /dev/null
+++ b/test/bds/csm028_broad_rescue_test.exs
@@ -0,0 +1,22 @@
+defmodule BDS.CSM028BroadRescueTest do
+  use ExUnit.Case, async: true
+
+  describe "source-level: no broad rescue in render_macro_template" do
+    test "filters.ex has no rescue _error or rescue _ clauses" do
+      source = File.read!("lib/bds/rendering/filters.ex")
+      refute source =~ ~r/rescue\s+_\w*\s*->/, "Found broad rescue clause in filters.ex"
+    end
+
+    test "filters.ex does not call read_template_file (which raises)" do
+      source = File.read!("lib/bds/rendering/filters.ex")
+
+      refute source =~ "read_template_file",
+             "render_macro_template should use try_read, not read_template_file"
+    end
+
+    test "filters.ex uses FileSystem.try_read for macro templates" do
+      source = File.read!("lib/bds/rendering/filters.ex")
+      assert source =~ "FileSystem.try_read"
+    end
+  end
+end