From 4d1a71d688cfd6d8c597f4214aac4dedda9ac6d0 Mon Sep 17 00:00:00 2001
From: Daryl Lim <5508348+daryllimyt@users.noreply.github.com>
Date: Sun, 26 Jan 2025 02:36:32 +0000
Subject: [PATCH] Use TracecatValidationError over ValueError

---
 tests/unit/test_templates.py        | 32 +++++++++++++++++++++++++++++
 tracecat/registry/actions/models.py |  6 ++++--
 tracecat/types/exceptions.py        |  2 +-
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/tests/unit/test_templates.py b/tests/unit/test_templates.py
index 3b74c3329..e0beb079c 100644
--- a/tests/unit/test_templates.py
+++ b/tests/unit/test_templates.py
@@ -30,6 +30,7 @@
 from tracecat.registry.repository import Repository
 from tracecat.secrets.models import SecretCreate, SecretKeyValue
 from tracecat.secrets.service import SecretsService
+from tracecat.types.exceptions import TracecatValidationError
 
 
 @pytest.fixture
@@ -321,3 +322,34 @@ def get_secrets(action: BoundRegistryAction) -> list[RegistrySecret]:
         "secret_step": "UDF_SECRET_VALUE",
         "nested_secret_step": "UDF_SECRET_VALUE",
     }
+
+
+def test_template_action_definition_validates_self_reference():
+    """Test that TemplateActionDefinition validates against self-referential steps.
+
+    The test verifies:
+    1. A template action cannot reference itself in its steps
+    2. The validation error message is descriptive
+    """
+    with pytest.raises(TracecatValidationError) as exc_info:
+        TemplateActionDefinition(
+            title="Self Referential Action",
+            description="This action tries to reference itself",
+            name="self_ref",
+            namespace="testing",
+            display_group="Testing",
+            expects={},
+            steps=[
+                ActionStep(
+                    ref="self_ref_step",
+                    action="testing.self_ref",  # This references the template itself
+                    args={},
+                ),
+            ],
+            returns="${{ steps.self_ref_step.result }}",
+        )
+
+    assert "Steps cannot reference the template action itself: testing.self_ref" in str(
+        exc_info.value
+    )
+    assert "1 steps reference the template action" in str(exc_info.value)
diff --git a/tracecat/registry/actions/models.py b/tracecat/registry/actions/models.py
index 5b3a3cfff..bbc49f887 100644
--- a/tracecat/registry/actions/models.py
+++ b/tracecat/registry/actions/models.py
@@ -201,12 +201,14 @@ def validate_steps(self):
         # Check for duplicate step refs
         if len(step_refs) != len(unique_step_refs):
             duplicate_step_refs = [ref for ref in step_refs if step_refs.count(ref) > 1]
-            raise ValueError(f"Duplicate step references found: {duplicate_step_refs}")
+            raise TracecatValidationError(
+                f"Duplicate step references found: {duplicate_step_refs}"
+            )
 
         # Check if any step action references the template action
         template_action = f"{self.namespace}.{self.name}"
         if violating_steps := [s for s in self.steps if s.action == template_action]:
-            raise ValueError(
+            raise TracecatValidationError(
                 f"Steps cannot reference the template action itself: {template_action}."
                 f"{len(violating_steps)} steps reference the template action: {violating_steps}"
             )
diff --git a/tracecat/types/exceptions.py b/tracecat/types/exceptions.py
index af519efbe..f2293079a 100644
--- a/tracecat/types/exceptions.py
+++ b/tracecat/types/exceptions.py
@@ -21,7 +21,7 @@ def __init__(self, *args, detail: Any | None = None, **kwargs):
 
 
 class TracecatValidationError(TracecatException):
-    """Tracecat user-facting validation error"""
+    """Tracecat user-facing validation error"""
 
 
 class TracecatDSLError(TracecatValidationError):