Refactor GraphError

- Split each class part to a separated class
- Improve output messages for each error
- Update tests to use the new classes
- Update exception catch to use dedicated classes

Signed-off-by: Uilian Ries <uilianries@gmail.com>

conans/client/graph/graph_builder.py

@@ -5,7 +5,8 @@
 from conans.client.graph.graph import DepsGraph, Node, CONTEXT_HOST, \
     CONTEXT_BUILD, TransitiveRequirement, RECIPE_VIRTUAL
 from conans.client.graph.graph import RECIPE_SYSTEM_TOOL
-from conans.client.graph.graph_error import GraphError
+from conans.client.graph.graph_error import GraphLoopError, GraphConflictError, GraphMissingError, \
+    GraphRuntimeError, GraphError
 from conans.client.graph.profile_node_definer import initialize_conanfile_profile
 from conans.client.graph.provides import check_graph_provides
 from conans.errors import ConanException
@@ -77,7 +78,7 @@ def _expand_require(self, require, node, graph, profile_host, profile_build, gra
             # print("  Existing previous requirements from ", base_previous, "=>", prev_require)
 
             if prev_require is None:
-                raise GraphError.loop(node, require, prev_node)
+                raise GraphLoopError(node, require, prev_node)
 
             prev_ref = prev_node.ref if prev_node else prev_require.ref
             if prev_require.force or prev_require.override:  # override
@@ -112,12 +113,12 @@ def _conflicting_version(require, node,
                 if version_range.contains(prev_ref.version, resolve_prereleases):
                     require.ref = prev_ref
                 else:
-                    raise GraphError.conflict(node, require, prev_node, prev_require, base_previous)
+                    raise GraphConflictError(node, require, prev_node, prev_require, base_previous)
 
         elif prev_version_range is not None:
             # TODO: Check user/channel conflicts first
             if not prev_version_range.contains(require.ref.version, resolve_prereleases):
-                raise GraphError.conflict(node, require, prev_node, prev_require, base_previous)
+                raise GraphConflictError(node, require, prev_node, prev_require, base_previous)
         else:
             def _conflicting_refs(ref1, ref2):
                 ref1_norev = copy.copy(ref1)
@@ -135,7 +136,7 @@ def _conflicting_refs(ref1, ref2):
             # As we are closing a diamond, there can be conflicts. This will raise if so
             conflict = _conflicting_refs(prev_ref, require.ref)
             if conflict:  # It is possible to get conflict from alias, try to resolve it
-                raise GraphError.conflict(node, require, prev_node, prev_require, base_previous)
+                raise GraphConflictError(node, require, prev_node, prev_require, base_previous)
 
     @staticmethod
     def _prepare_node(node, profile_host, profile_build, down_options):
@@ -194,7 +195,7 @@ def _resolve_alias(self, node, require, graph):
                                                 self._check_update)
                 conanfile_path, recipe_status, remote, new_ref = result
             except ConanException as e:
-                raise GraphError.missing(node, require, str(e))
+                raise GraphMissingError(node, require, str(e))
 
             dep_conanfile = self._loader.load_basic(conanfile_path)
             try:
@@ -244,7 +245,7 @@ def _create_new_node(self, node, require, graph, profile_host, profile_build, gr
                 self._resolver.resolve(require, str(node.ref), self._remotes, self._update)
                 resolved = self._resolve_recipe(require.ref, graph_lock)
             except ConanException as e:
-                raise GraphError.missing(node, require, str(e))
+                raise GraphMissingError(node, require, str(e))
 
         new_ref, dep_conanfile, recipe_status, remote = resolved
         # If the node is virtual or a test package, the require is also "root"
@@ -280,12 +281,12 @@ def _create_new_node(self, node, require, graph, profile_host, profile_build, gr
         graph.add_node(new_node)
         graph.add_edge(node, new_node, require)
         if node.propagate_downstream(require, new_node):
-            raise GraphError.runtime(node, new_node)
+            raise GraphRuntimeError(node, new_node)
 
         # This is necessary to prevent infinite loops even when visibility is False
         ancestor = node.check_loops(new_node)
         if ancestor is not None:
-            raise GraphError.loop(new_node, require, ancestor)
+            raise GraphLoopError(new_node, require, ancestor)
 
         return new_node
 

conans/client/graph/graph_error.py

@@ -2,73 +2,66 @@
 
 
 class GraphError(ConanException):
-    # TODO: refactor into multiple classes, do not do type by attribute "kind"
-    LOOP = "graph loop"
-    VERSION_CONFLICT = "version conflict"
-    PROVIDE_CONFLICT = "provide conflict"
-    MISSING_RECIPE = "missing recipe"
-    RUNTIME = "runtime"
+    pass
 
-    def __init__(self, kind):
-        self.kind = kind
+
+class GraphConflictError(GraphError):
+
+    def __init__(self, node, require, prev_node, prev_require, base_previous):
+        self.node = node
+        self.require = require
+        self.prev_node = prev_node
+        self.prev_require = prev_require
+        self.base_previous = base_previous
+
+    def __str__(self):
+        return f"Version conflict: {self.node.ref}->{self.require.ref}, "\
+               f"{self.base_previous.ref}->{self.prev_require.ref}."
+
+
+class GraphLoopError(GraphError):
+
+    def __init__(self, node, require, ancestor):
+        self.node = node
+        self.require = require
+        self.ancestor = ancestor
+
+    def __str__(self):
+        return "There is a cycle/loop in the graph:\n"\
+               f"    Initial ancestor: {self.ancestor}\n" \
+               f"    Require: {self.require.ref}\n" \
+               f"    Dependency: {self.node}"
+
+
+class GraphMissingError(GraphError):
+
+    def __init__(self, node, require, missing_error):
+        self.node = node
+        self.require = require
+        self.missing_error = missing_error
+
+    def __str__(self):
+        return f"Package '{self.require.ref}' not resolved: {self.missing_error}."
+
+
+class GraphProvidesError(ConanException):
+
+    def __init__(self, node, conflicting_node):
+        self.node = node
+        self.conflicting_node = conflicting_node
+        node.error = conflicting_node.error
+
+    def __str__(self):
+        return f"Provide Conflict: Both '{self.node.ref}' and '{self.conflicting_node.ref}' " \
+               f"provide '{self.node.conanfile.provides}'."
+
+
+class GraphRuntimeError(ConanException):
+
+    def __init__(self, node, conflicting_node):
+        self.node = node
+        self.conflicting_node = conflicting_node
 
     def __str__(self):
-        # TODO: Nicer error reporting
-        if self.kind == GraphError.MISSING_RECIPE:
-            return f"Package '{self.require.ref}' not resolved: {self.missing_error}"
-        elif self.kind == GraphError.VERSION_CONFLICT:
-            return f"Version conflict: {self.node.ref}->{self.require.ref}, "\
-                   f"{self.base_previous.ref}->{self.prev_require.ref}."
-        elif self.kind == GraphError.LOOP:
-            return "There is a cycle/loop in the graph:\n"\
-                   f"    Initial ancestor: {self.ancestor}\n" \
-                   f"    Require: {self.require.ref}\n" \
-                   f"    Dependency: {self.node}"
-        return self.kind
-
-    @staticmethod
-    def loop(node, require, ancestor):
-        result = GraphError(GraphError.LOOP)
-        result.node = node
-        result.require = require
-        result.ancestor = ancestor
-        node.error = ancestor.error = result
-        return result
-
-    @staticmethod
-    def runtime(node, conflicting_node):
-        result = GraphError(GraphError.RUNTIME)
-        result.node = node
-        result.conflicting_node = conflicting_node
-        node.error = conflicting_node.error = result
-        return result
-
-    @staticmethod
-    def provides(node, conflicting_node):
-        result = GraphError(GraphError.PROVIDE_CONFLICT)
-        result.node = node
-        result.conflicting_node = conflicting_node
-        node.error = conflicting_node.error = result
-        return result
-
-    @staticmethod
-    def missing(node, require, missing_error):
-        result = GraphError(GraphError.MISSING_RECIPE)
-        result.node = node
-        result.require = require
-        result.missing_error = missing_error
-        node.error = result
-        return result
-
-    @staticmethod
-    def conflict(node, require, prev_node, prev_require, base_previous):
-        result = GraphError(GraphError.VERSION_CONFLICT)
-        result.node = node
-        result.require = require
-        result.prev_node = prev_node
-        result.prev_require = prev_require
-        result.base_previous = base_previous
-        node.error = base_previous.error = result
-        if prev_node:
-            prev_node.error = result
-        return result
+        return f"Runtime Error: Could not process '{self.node.ref}' with " \
+               f"'{self.conflicting_node.ref}'."

conans/client/graph/provides.py

@@ -1,4 +1,4 @@
-from conans.client.graph.graph_error import GraphError
+from conans.client.graph.graph_error import GraphProvidesError
 from conans.model.recipe_ref import RecipeReference
 
 
@@ -18,18 +18,18 @@ def check_graph_provides(dep_graph):
             for provide in dep_provides:
                 # First check if collides with current node
                 if current_provides is not None and provide in current_provides:
-                    raise GraphError.provides(node, dep_node)
+                    raise GraphProvidesError(node, dep_node)
 
                 # Then, check if collides with other requirements
                 new_req = dep_require.copy_requirement()
                 new_req.ref = RecipeReference(provide, new_req.ref.version, new_req.ref.user,
                                               new_req.ref.channel)
                 existing = node.transitive_deps.get(new_req)
                 if existing is not None:
-                    raise GraphError.provides(existing.node, dep_node)
+                    raise GraphProvidesError(existing.node, dep_node)
                 else:
                     existing_provide = provides.get(new_req)
                     if existing_provide is not None:
-                        raise GraphError.provides(existing_provide, dep_node)
+                        raise GraphProvidesError(existing_provide, dep_node)
                     else:
                         provides[new_req] = dep_node

conans/test/integration/graph/core/graph_manager_test.py

@@ -1,7 +1,7 @@
 import pytest
 from parameterized import parameterized
 
-from conans.client.graph.graph_error import GraphError
+from conans.client.graph.graph_error import GraphMissingError, GraphLoopError, GraphConflictError
 from conans.errors import ConanException
 from conans.test.integration.graph.core.graph_manager_base import GraphManagerTest
 from conans.test.utils.tools import GenConanfile
@@ -56,7 +56,7 @@ def test_dependency_missing(self):
         deps_graph = self.build_consumer(consumer, install=False)
 
         # TODO: Better error handling
-        assert deps_graph.error.kind == GraphError.MISSING_RECIPE
+        assert type(deps_graph.error) == GraphMissingError
 
         self.assertEqual(1, len(deps_graph.nodes))
         app = deps_graph.root
@@ -1402,7 +1402,7 @@ def test_diamond_conflict(self):
         consumer = self.recipe_consumer("app/0.1", ["libb/0.1", "libc/0.1"])
         deps_graph = self.build_consumer(consumer, install=False)
 
-        assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
+        assert type(deps_graph.error) == GraphConflictError
 
         self.assertEqual(4, len(deps_graph.nodes))
         app = deps_graph.root
@@ -1426,7 +1426,7 @@ def test_shared_conflict_shared(self):
 
         deps_graph = self.build_consumer(consumer, install=False)
 
-        assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
+        assert type(deps_graph.error) == GraphConflictError
 
         self.assertEqual(4, len(deps_graph.nodes))
         app = deps_graph.root
@@ -1455,7 +1455,7 @@ def test_private_conflict(self):
 
         deps_graph = self.build_consumer(consumer, install=False)
 
-        assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
+        assert type(deps_graph.error) == GraphConflictError
 
         self.assertEqual(5, len(deps_graph.nodes))
         app = deps_graph.root
@@ -1636,7 +1636,7 @@ def test_loop(self):
 
         deps_graph = self.build_consumer(consumer, install=False)
         # TODO: Better error modeling
-        assert deps_graph.error.kind == GraphError.LOOP
+        assert type(deps_graph.error) == GraphLoopError
 
         self.assertEqual(4, len(deps_graph.nodes))
 
@@ -1686,7 +1686,7 @@ def test_diamond_conflict(self):
 
         deps_graph = self.build_consumer(consumer, install=False)
         assert deps_graph.error is not False
-        assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
+        assert type(deps_graph.error) == GraphConflictError
 
         self.assertEqual(2, len(deps_graph.nodes))
         app = deps_graph.root
@@ -1757,7 +1757,7 @@ def test_diamond_reverse_order_conflict(self):
         consumer = self.recipe_consumer("app/0.1", ["dep1/2.0", "dep2/1.0"])
         deps_graph = self.build_consumer(consumer, install=False)
 
-        assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
+        assert type(deps_graph.error) == GraphConflictError
 
         self.assertEqual(3, len(deps_graph.nodes))
         app = deps_graph.root
@@ -1991,7 +1991,7 @@ def test_project_require_transitive_conflict(self):
                                                         build=False, run=True),
                                       install=False)
 
-        assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
+        assert type(deps_graph.error) == GraphConflictError
 
     def test_project_require_apps_transitive(self):
         # project -> app1 (app type) -> lib
@@ -2040,7 +2040,7 @@ def test_project_require_apps_transitive_conflict(self):
                                                                                    "app2/0.1"),
                                       install=False)
 
-        assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
+        assert type(deps_graph.error) == GraphConflictError
 
     def test_project_require_private(self):
         # project -(!visible)-> app1 -> lib1

conans/test/integration/graph/core/test_build_requires.py

@@ -4,7 +4,7 @@
 
 from parameterized import parameterized
 
-from conans.client.graph.graph_error import GraphError
+from conans.client.graph.graph_error import GraphConflictError, GraphLoopError, GraphRuntimeError
 from conans.model.recipe_ref import RecipeReference
 from conans.test.integration.graph.core.graph_manager_base import GraphManagerTest
 from conans.test.utils.tools import GenConanfile, NO_SETTINGS_PACKAGE_ID, TestClient
@@ -203,7 +203,7 @@ def test_build_require_transitive_shared(self):
         deps_graph = self.build_graph(GenConanfile("app", "0.1").with_require("lib/0.1"),
                                       install=False)
 
-        assert deps_graph.error.kind == GraphError.RUNTIME
+        assert type(deps_graph.error) == GraphRuntimeError
 
         self.assertEqual(6, len(deps_graph.nodes))
         app = deps_graph.root
@@ -508,7 +508,7 @@ def test_conflict_diamond(self):
                                                                                "libc/0.1"),
                                       install=False)
 
-        assert deps_graph.error.kind == GraphError.VERSION_CONFLICT
+        assert type(deps_graph.error) == GraphConflictError
 
         # Build requires always apply to the consumer
         self.assertEqual(4, len(deps_graph.nodes))
@@ -617,7 +617,7 @@ def test_direct_loop_error(self):
         deps_graph = self.build_graph(GenConanfile("app", "0.1").with_build_requires("cmake/0.1"),
                                       install=False)
 
-        assert deps_graph.error.kind == GraphError.LOOP
+        assert type(deps_graph.error) == GraphLoopError
 
         # Build requires always apply to the consumer
         self.assertEqual(2, len(deps_graph.nodes))
@@ -636,7 +636,7 @@ def test_indirect_loop_error(self):
         deps_graph = self.build_graph(GenConanfile().with_build_requires("cmake/0.1"),
                                       install=False)
 
-        assert deps_graph.error.kind == GraphError.LOOP
+        assert type(deps_graph.error) == GraphLoopError
 
         # Build requires always apply to the consumer
         self.assertEqual(4, len(deps_graph.nodes))

conans/test/integration/graph/core/test_options.py

@@ -1,4 +1,3 @@
-from conans.client.graph.graph_error import GraphError
 from conans.test.assets.genconanfile import GenConanfile
 from conans.test.integration.graph.core.graph_manager_base import GraphManagerTest
 from conans.test.integration.graph.core.graph_manager_test import _check_transitive