From a4ecbabc2109d7d25234f3e5dc2d87f1e8a641db Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Mon, 11 May 2026 09:07:44 +0200 Subject: [PATCH] fix: return error tuples instead of silent {:ok, ""} in execute_macro (CSM-022) Co-Authored-By: Claude Opus 4.6 --- CODESMELL.md | 10 ++++++---- lib/bds/scripting.ex | 6 +++++- test/bds/scripting/api_test.exs | 2 +- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/CODESMELL.md b/CODESMELL.md index 0fd75c0..4b6bf0e 100644 --- a/CODESMELL.md +++ b/CODESMELL.md @@ -360,10 +360,12 @@ --- -### CSM-022 — Silent Error Swallowing -- **File:** `lib/bds/scripting.ex:64-66` -- **What:** `execute_macro/4` returns `{:ok, ""}` on `{:error, _reason}` with no logging. The caller cannot distinguish success from failure. -- **Fix:** Return the actual error tuple or at least log the failure with `Logger.error/1`. +### ~~CSM-022 — Silent Error Swallowing~~ ✅ FIXED +- **Fixed:** 2026-05-11 +- **What was done:** + - `execute_macro/4` now returns `{:error, reason}` instead of `{:ok, ""}` when the underlying script execution fails. + - Added `Logger.warning/1` call that logs the project ID and error reason before returning the error tuple. + - Updated test in `api_test.exs` to assert `{:error, _reason}` instead of `{:ok, ""}` for failing macros. --- diff --git a/lib/bds/scripting.ex b/lib/bds/scripting.ex index 0f5334a..700a13e 100644 --- a/lib/bds/scripting.ex +++ b/lib/bds/scripting.ex @@ -3,6 +3,8 @@ defmodule BDS.Scripting do Facade for the configured user-script runtime. """ + require Logger + alias BDS.Scripting.Capabilities alias BDS.Scripting.Runtime @@ -63,7 +65,9 @@ defmodule BDS.Scripting do ) do {:ok, nil} -> {:ok, ""} {:ok, value} -> {:ok, to_string(value)} - {:error, _reason} -> {:ok, ""} + {:error, reason} -> + Logger.warning("execute_macro failed for project #{project_id}: #{inspect(reason)}") + {:error, reason} end end diff --git a/test/bds/scripting/api_test.exs b/test/bds/scripting/api_test.exs index dc62100..2dd38fe 100644 --- a/test/bds/scripting/api_test.exs +++ b/test/bds/scripting/api_test.exs @@ -106,7 +106,7 @@ defmodule BDS.Scripting.ApiTest do bad_source = "function render() error('boom') end" - assert {:ok, ""} = BDS.Scripting.execute_macro(project.id, bad_source, []) + assert {:error, _reason} = BDS.Scripting.execute_macro(project.id, bad_source, []) end test "project scripting exposes project, post, script, template, metadata, and task namespaces",