fix(rendering): replace raising read_template_file with try_read in macro templates (CSM-028)
This commit is contained in:
13
CODESMELL.md
13
CODESMELL.md
@@ -422,10 +422,15 @@
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### CSM-028 — Broad `rescue` Swallowing Template Errors
|
### ~~CSM-028 — Broad `rescue` Swallowing Template Errors~~ ✅ FIXED
|
||||||
- **File:** `lib/bds/rendering/filters.ex:130-132`
|
- **Fixed:** 2026-05-11
|
||||||
- **What:** `rescue _error -> ""` swallows all macro template failures silently.
|
- **What was done:**
|
||||||
- **Fix:** Rescue only specific exceptions, or return `{:error, exception}` and let the caller decide.
|
- 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.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -135,30 +135,40 @@ defmodule BDS.Rendering.Filters do
|
|||||||
""
|
""
|
||||||
|
|
||||||
_id ->
|
_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
|
{:error, :enoent} ->
|
||||||
{: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
|
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
|
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
|
defp render_markdown_html(markdown) do
|
||||||
case Earmark.as_html(markdown) do
|
case Earmark.as_html(markdown) do
|
||||||
{:ok, html, _messages} -> html
|
{:ok, html, _messages} -> html
|
||||||
|
|||||||
22
test/bds/csm028_broad_rescue_test.exs
Normal file
22
test/bds/csm028_broad_rescue_test.exs
Normal file
@@ -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
|
||||||
Reference in New Issue
Block a user