fix: Print clear error when fetching/editing a relationship on an id-less node

changelog/1153.fixed.md

@@ -0,0 +1 @@
+Fetching or editing a relationship on a node that has no ID (e.g. a node created locally but not yet saved) now raises a clear error explaining the node needs to be saved first, instead of the generic `At least one filter must be provided to get()` message (for `fetch()`) or sending the caller in a circle between `add()` and `fetch()`.

infrahub_sdk/node/relationship.py

@@ -2,7 +2,7 @@
 
 from collections import defaultdict
 from collections.abc import Iterable
-from typing import TYPE_CHECKING, Any, Generic, cast
+from typing import TYPE_CHECKING, Any, Generic, NoReturn, cast
 
 from ..exceptions import (
     Error,
@@ -19,6 +19,25 @@
     from .node import InfrahubNode, InfrahubNodeSync
 
 
+def _raise_missing_identifier(node: InfrahubNode | InfrahubNodeSync, name: str) -> NoReturn:
+    """Raise a clear error for fetching/editing a relationship on a node with no ID.
+
+    A relationship can only be fetched or edited once the parent node has an ID to look it up
+    by. When it does not, the underlying ``client.get()`` call would otherwise fail with a
+    generic "At least one filter must be provided to get()" message that gives the caller no
+    hint about the real cause.
+
+    Raises:
+        UninitializedError: Always; the parent node has no ID to look the relationship up by.
+
+    """
+    raise UninitializedError(
+        f"Cannot access the '{name}' relationship because the {node._schema.kind} node has no ID to "
+        f"look it up by. This usually means the node was created locally but not saved yet — call "
+        f".save() on it first (or fetch it from Infrahub) before fetching or editing its relationships."
+    )
+
+
 class RelationshipManagerBase(Generic[PeerT]):
     """Base class for :class:`RelationshipManager` and :class:`RelationshipManagerSync`.
 
@@ -243,6 +262,8 @@ async def fetch(self) -> None:
 
         """
         if not self.initialized:
+            if not self.node.id:
+                _raise_missing_identifier(self.node, self.name)
             exclude = self.node._schema.relationship_names + self.node._schema.attribute_names
             exclude.remove(self.schema.name)
             node = await self.client.get(
@@ -293,6 +314,8 @@ def add(self, data: str | RelatedNode | dict) -> None:
 
         """
         if not self.initialized:
+            if not self.node.id:
+                _raise_missing_identifier(self.node, self.name)
             raise UninitializedError("Must call fetch() on RelationshipManager before editing members")
         new_node = cast(
             "RelatedNode[PeerT]", RelatedNode(schema=self.schema, client=self.client, branch=self.branch, data=data)
@@ -337,6 +360,8 @@ def remove(self, data: str | RelatedNode | dict) -> None:
 
         """
         if not self.initialized:
+            if not self.node.id:
+                _raise_missing_identifier(self.node, self.name)
             raise UninitializedError("Must call fetch() on RelationshipManager before editing members")
         node_to_remove = RelatedNode(schema=self.schema, client=self.client, branch=self.branch, data=data)
 
@@ -440,6 +465,8 @@ def fetch(self) -> None:
 
         """
         if not self.initialized:
+            if not self.node.id:
+                _raise_missing_identifier(self.node, self.name)
             exclude = self.node._schema.relationship_names + self.node._schema.attribute_names
             exclude.remove(self.schema.name)
             node = self.client.get(
@@ -490,6 +517,8 @@ def add(self, data: str | RelatedNodeSync | dict) -> None:
 
         """
         if not self.initialized:
+            if not self.node.id:
+                _raise_missing_identifier(self.node, self.name)
             raise UninitializedError("Must call fetch() on RelationshipManager before editing members")
         new_node = cast(
             "RelatedNodeSync[PeerTSync]",
@@ -535,6 +564,8 @@ def remove(self, data: str | RelatedNodeSync | dict) -> None:
 
         """
         if not self.initialized:
+            if not self.node.id:
+                _raise_missing_identifier(self.node, self.name)
             raise UninitializedError("Must call fetch() on RelationshipManager before editing members")
         node_to_remove = RelatedNodeSync(schema=self.schema, client=self.client, branch=self.branch, data=data)
 

tests/unit/sdk/test_node.py

@@ -9,7 +9,7 @@
 
 import pytest
 
-from infrahub_sdk.exceptions import FeatureNotSupportedError, NodeNotFoundError
+from infrahub_sdk.exceptions import FeatureNotSupportedError, NodeNotFoundError, UninitializedError
 from infrahub_sdk.node import (
     InfrahubNode,
     InfrahubNodeBase,
@@ -342,6 +342,48 @@ async def test_cardinality_many_accepts_list(
     assert len(node.tags.peers) == 2
 
 
+@pytest.mark.parametrize("client_type", client_types)
+async def test_fetch_relationship_without_node_id_raises_clear_error(
+    client: InfrahubClient, location_schema: NodeSchemaAPI, client_type: str
+) -> None:
+    """fetch() on a node with no ID explains it isn't saved instead of raising the generic get() filter error."""
+    if client_type == "standard":
+        node = InfrahubNode(client=client, schema=location_schema, data={"name": {"value": "JFK1"}})
+        with pytest.raises(UninitializedError, match=r"has no ID"):
+            await node.tags.fetch()
+    else:
+        node = InfrahubNodeSync(client=client, schema=location_schema, data={"name": {"value": "JFK1"}})
+        with pytest.raises(UninitializedError, match=r"has no ID"):
+            node.tags.fetch()
+
+
+@pytest.mark.parametrize("client_type", client_types)
+async def test_edit_relationship_without_node_id_raises_clear_error(
+    client: InfrahubClient, location_schema: NodeSchemaAPI, client_type: str
+) -> None:
+    """add() on a node with no ID points the user at .save() instead of at fetch(), avoiding the circular guidance."""
+    if client_type == "standard":
+        node = InfrahubNode(client=client, schema=location_schema, data={"name": {"value": "JFK1"}})
+    else:
+        node = InfrahubNodeSync(client=client, schema=location_schema, data={"name": {"value": "JFK1"}})
+    with pytest.raises(UninitializedError, match=r"has no ID"):
+        node.tags.add("11111111-1111-1111-1111-111111111111")
+
+
+@pytest.mark.parametrize("client_type", client_types)
+async def test_edit_relationship_with_node_id_still_requires_fetch(
+    client: InfrahubClient, location_schema: NodeSchemaAPI, client_type: str
+) -> None:
+    """When the node has an ID but the relationship isn't loaded, the original "call fetch()" guidance is preserved."""
+    data = {"id": "22222222-2222-2222-2222-222222222222", "name": {"value": "JFK1"}}
+    if client_type == "standard":
+        node = InfrahubNode(client=client, schema=location_schema, data=data)
+    else:
+        node = InfrahubNodeSync(client=client, schema=location_schema, data=data)
+    with pytest.raises(UninitializedError, match=r"Must call fetch"):
+        node.tags.add("11111111-1111-1111-1111-111111111111")
+
+
 @pytest.mark.parametrize("client_type", client_types)
 async def test_query_data_no_filters_property(
     clients: BothClients, location_schema: NodeSchemaAPI, client_type: str