improved error handling

matt-beanland · matt-beanland · commit d5aad7cb34e4 · 2026-03-19T23:22:08.000+10:30
diff --git a/documentation/dsls/DSL-Diffo.Provider.Instance.Extension.md b/documentation/dsls/DSL-Diffo.Provider.Instance.Extension.md
@@ -56,9 +56,7 @@ Configuration for Instance Features
 features do
   feature :dynamic_line_management do
     is_enabled? true
-    characteristics do
-      characteristic :constraints, Diffo.Access.Constraints
-    end
+    characteristic :constraints, Diffo.Access.Constraints
   end
 end
 
diff --git a/lib/diffo/helpers/uuid.ex b/lib/diffo/helpers/uuid.ex
@@ -63,7 +63,7 @@ defmodule Diffo.Uuid do
 
     case info do
       {:ok, result} ->
-        if result[:version] == 4 do
+        if result[:version] == 4 and result[:variant] == :rfc4122 do
           true
         else
           false
diff --git a/lib/diffo/provider/components/instance/extension/characteristic.ex b/lib/diffo/provider/components/instance/extension/characteristic.ex
@@ -30,6 +30,9 @@ defmodule Diffo.Provider.Instance.Characteristic do
       [] ->
         changeset
 
+      {:error, error} ->
+        Ash.Changeset.add_error(changeset, error)
+
       _ ->
         characteristic_ids = Enum.map(characteristics, &Map.get(&1, :id))
         Ash.Changeset.force_set_argument(changeset, :characteristics, characteristic_ids)
@@ -43,14 +46,20 @@ defmodule Diffo.Provider.Instance.Characteristic do
     characteristics = Info.characteristics(module)
 
     Enum.reduce_while(characteristics, [], fn %{name: name, value_type: value_type}, acc ->
-      value = struct(value_type)
+      try do
+        value = struct(value_type)
 
-      case Provider.create_characteristic(%{name: name, type: type, value: value}) do
-        {:ok, result} ->
-          {:cont, [result | acc]}
+        case Provider.create_characteristic(%{name: name, type: type, value: value}) do
+          {:ok, result} ->
+            {:cont, [result | acc]}
 
-        {:error, _error} ->
-          {:halt, []}
+          {:error, error} ->
+            {:halt, {:error, error}}
+        end
+      rescue
+        _e in UndefinedFunctionError ->
+          {:halt,
+           {:error, "couldn't create characteristic with value of unknown type #{value_type}"}}
       end
     end)
   end
diff --git a/lib/diffo/provider/components/instance/extension/feature.ex b/lib/diffo/provider/components/instance/extension/feature.ex
@@ -30,6 +30,9 @@ defmodule Diffo.Provider.Instance.Feature do
       [] ->
         changeset
 
+      {:error, error} ->
+        Ash.Changeset.add_error(changeset, error)
+
       _ ->
         feature_ids = Enum.map(features, &Map.get(&1, :id))
         Ash.Changeset.force_set_argument(changeset, :features, feature_ids)
@@ -49,28 +52,41 @@ defmodule Diffo.Provider.Instance.Feature do
       fn %{name: name, is_enabled?: isEnabled, characteristics: characteristics}, acc ->
         characteristic_ids =
           Enum.reduce_while(characteristics, [], fn %{name: name, value_type: value_type}, acc ->
-            value = struct(value_type)
+            try do
+              value = struct(value_type)
+
+              case Provider.create_characteristic(%{name: name, value: value, type: :feature}) do
+                {:ok, result} ->
+                  {:cont, [result.id | acc]}
+
+                {:error, error} ->
+                  {:halt, {:error, error}}
+              end
+            rescue
+              _e in UndefinedFunctionError ->
+                {:halt,
+                 {:error,
+                  "couldn't create feature characteristic with value of unknown type #{value_type}"}}
+            end
+          end)
 
-            case Provider.create_characteristic(%{name: name, value: value, type: :feature}) do
+        case characteristic_ids do
+          {:error, error} ->
+            {:halt, {:error, error}}
+
+          _ ->
+            # create feature with feature characteristics
+            case Provider.create_feature(%{
+                   name: name,
+                   isEnabled: isEnabled,
+                   characteristics: characteristic_ids
+                 }) do
               {:ok, result} ->
-                {:cont, [result.id | acc]}
+                {:cont, [result | acc]}
 
-              {:error, _error} ->
-                {:halt, []}
+              {:error, error} ->
+                {:halt, {:error, error}}
             end
-          end)
-
-        # create feature with feature characteristics
-        case Provider.create_feature(%{
-               name: name,
-               isEnabled: isEnabled,
-               characteristics: characteristic_ids
-             }) do
-          {:ok, result} ->
-            {:cont, [result | acc]}
-
-          {:error, _error} ->
-            {:halt, []}
         end
       end
     )
diff --git a/lib/diffo/provider/components/instance/extension/specification.ex b/lib/diffo/provider/components/instance/extension/specification.ex
@@ -29,8 +29,8 @@ defmodule Diffo.Provider.Instance.Specification do
       {:ok, specification} ->
         Ash.Changeset.force_set_argument(changeset, :specified_by, specification.id)
 
-      {:error, _error} ->
-        Logger.error("couldn't find/create required specification")
+      {:error, error} ->
+        Ash.Changeset.add_error(changeset, error)
     end
   end
 
diff --git a/test/instance_extension/assigner_test.exs b/test/instance_extension/assigner_test.exs
@@ -2,7 +2,7 @@
 #
 # SPDX-License-Identifier: MIT
 
-defmodule Diffo.Test.AssignerTest do
+defmodule Diffo.InstanceExtension.AssignerTest do
   @moduledoc false
   use ExUnit.Case
   alias Diffo.Provider.Specification
diff --git a/test/instance_extension/characteristic_test.exs b/test/instance_extension/characteristic_test.exs
@@ -0,0 +1,29 @@
+# SPDX-FileCopyrightText: 2025 diffo contributors <https://github.com/diffo-dev/diffo/graphs.contributors>
+#
+# SPDX-License-Identifier: MIT
+
+defmodule Diffo.InstanceExtension.CharacteristicTest do
+  @moduledoc false
+  use ExUnit.Case
+  alias Diffo.Test.Servo
+
+  setup_all do
+    AshNeo4j.BoltyHelper.start()
+  end
+
+  setup do
+    on_exit(fn ->
+      AshNeo4j.Neo4jHelper.delete_all()
+    end)
+  end
+
+  describe "characteristic" do
+    test "create resource fails when characteristic value type invalid" do
+      {:error, error} = Servo.build_invalid_characteristic(%{})
+      %Ash.Error.Invalid{errors: errors} = error
+
+      assert hd(errors).message ==
+               "couldn't create characteristic with value of unknown type Elixir.InvalidValue"
+    end
+  end
+end
diff --git a/test/instance_extension/feature_test.exs b/test/instance_extension/feature_test.exs
@@ -0,0 +1,29 @@
+# SPDX-FileCopyrightText: 2025 diffo contributors <https://github.com/diffo-dev/diffo/graphs.contributors>
+#
+# SPDX-License-Identifier: MIT
+
+defmodule Diffo.InstanceExtension.FeatureTest do
+  @moduledoc false
+  use ExUnit.Case
+  alias Diffo.Test.Servo
+
+  setup_all do
+    AshNeo4j.BoltyHelper.start()
+  end
+
+  setup do
+    on_exit(fn ->
+      AshNeo4j.Neo4jHelper.delete_all()
+    end)
+  end
+
+  describe "feature" do
+    test "create resource with fails when feature characteristic value type invalid" do
+      {:error, error} = Servo.build_invalid_feature_characteristic(%{})
+      %Ash.Error.Invalid{errors: errors} = error
+
+      assert hd(errors).message ==
+               "couldn't create feature characteristic with value of unknown type Elixir.InvalidValue"
+    end
+  end
+end
diff --git a/test/instance_extension/specification_test.exs b/test/instance_extension/specification_test.exs
@@ -0,0 +1,27 @@
+# SPDX-FileCopyrightText: 2025 diffo contributors <https://github.com/diffo-dev/diffo/graphs.contributors>
+#
+# SPDX-License-Identifier: MIT
+
+defmodule Diffo.InstanceExtension.SpecificationTest do
+  @moduledoc false
+  use ExUnit.Case
+  alias Diffo.Test.Servo
+
+  setup_all do
+    AshNeo4j.BoltyHelper.start()
+  end
+
+  setup do
+    on_exit(fn ->
+      AshNeo4j.Neo4jHelper.delete_all()
+    end)
+  end
+
+  describe "specification" do
+    test "create resource fails when specification id not uuid v4" do
+      {:error, error} = Servo.build_invalid_specification(%{})
+      %Ash.Error.Invalid{errors: errors} = error
+      assert hd(errors).message == "must be a uuid v4 or nil"
+    end
+  end
+end
diff --git a/test/provider/instance_test.exs b/test/provider/instance_test.exs
@@ -12,7 +12,7 @@ defmodule Diffo.Provider.InstanceTest do
   end
 
   setup do
-   on_exit(fn ->
+    on_exit(fn ->
       AshNeo4j.Neo4jHelper.delete_all()
     end)
   end
diff --git a/test/support/resource/deployment_class_value.ex b/test/support/resource/deployment_class_value.ex
@@ -0,0 +1,32 @@
+# SPDX-FileCopyrightText: 2025 diffo contributors <https://github.com/diffo-dev/diffo/graphs.contributors>
+#
+# SPDX-License-Identifier: MIT
+
+defmodule Diffo.Test.DeploymentClassValue do
+  @moduledoc """
+  Diffo - TMF Service and Resource Management with a difference
+
+  DeploymentClassValue - AshTyped Struct for DeploymentClass Feature Characteristic Value
+  """
+  use Ash.TypedStruct, extensions: [AshJason.TypedStruct, AshOutstanding.TypedStruct]
+
+  jason do
+    pick [:class, :mask]
+    compact true
+  end
+
+  outstanding do
+    expect [:name]
+  end
+
+  typed_struct do
+    field :class, :string, description: "the deployment class"
+    field :mask, :string, description: "the mask name"
+  end
+
+  defimpl String.Chars do
+    def to_string(struct) do
+      inspect(struct)
+    end
+  end
+end
diff --git a/test/support/resource/invalid/invalid_characteristic.ex b/test/support/resource/invalid/invalid_characteristic.ex
@@ -0,0 +1,66 @@
+# SPDX-FileCopyrightText: 2025 diffo contributors <https://github.com/diffo-dev/diffo/graphs.contributors>
+#
+# SPDX-License-Identifier: MIT
+
+defmodule Diffo.Test.InvalidCharacteristic do
+  @moduledoc """
+  Diffo - TMF Service and Resource Management with a difference
+
+  InvalidCharacteristic - Resource Instance with an Invalid Characteristic
+  """
+
+  alias Diffo.Provider.BaseInstance
+  alias Diffo.Provider.Instance.ActionHelper
+
+  alias Diffo.Test.Servo
+
+  use Ash.Resource,
+    fragments: [BaseInstance],
+    domain: Servo
+
+  resource do
+    description "Ash Resource with an invalid characteristic"
+  end
+
+  specification do
+    id "3caf29b9-0b91-4b8f-8568-2960131b1feb"
+    name "invalidCharacteristic"
+    type :resourceSpecification
+    category "Network Resource"
+  end
+
+  characteristics do
+    characteristic :invalid, InvalidValue
+  end
+
+  actions do
+    create :build do
+      description "creates a new InvalidCharacteristic resource instance for build"
+      accept [:id, :name, :type, :which]
+      argument :specified_by, :uuid, public?: false
+      argument :relationships, {:array, :struct}
+      argument :features, {:array, :uuid}, public?: false
+      argument :characteristics, {:array, :uuid}, public?: false
+      argument :places, {:array, :struct}
+      argument :parties, {:array, :struct}
+
+      change set_attribute(:type, :resource)
+
+      change before_action(fn changeset, _context ->
+               ActionHelper.build_before(changeset)
+             end)
+
+      change after_action(fn changeset, result, _context ->
+               ActionHelper.build_after(
+                 changeset,
+                 result,
+                 Servo,
+                 :get_invalid_characteristic_by_id
+               )
+             end)
+
+      change load [:href]
+      upsert? false
+    end
+  end
+end
diff --git a/test/support/resource/invalid/invalid_feature_characteristic.ex b/test/support/resource/invalid/invalid_feature_characteristic.ex
diff --git a/test/support/resource/invalid/invalid_specification.ex b/test/support/resource/invalid/invalid_specification.ex
diff --git a/test/support/resource/shelf.ex b/test/support/resource/shelf.ex
diff --git a/test/support/servo.ex b/test/support/servo.ex

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,7 @@`
`2`	`2`	`#`
`3`	`3`	`# SPDX-License-Identifier: MIT`
`4`	`4`
`5`		`-defmodule Diffo.Test.AssignerTest do`
	`5`	`+defmodule Diffo.InstanceExtension.AssignerTest do`
`6`	`6`	`@moduledoc false`
`7`	`7`	`use ExUnit.Case`
`8`	`8`	`alias Diffo.Provider.Specification`