enhance(query-planner): use less Into and From traits, to have a cleaner error handling (#731)

Author: ardatan

lib/query-planner/src/graph/error.rs

@@ -14,11 +14,5 @@ pub enum GraphError {
     #[error("Field named '{0}' was not found in definition name '{1}'")]
     FieldDefinitionNotFound(String, String),
     #[error("Supergraph state error: {0}")]
-    SupergraphStateError(Box<SupergraphStateError>),
-}
-
-impl From<SupergraphStateError> for GraphError {
-    fn from(err: SupergraphStateError) -> Self {
-        GraphError::SupergraphStateError(Box::new(err))
-    }
+    SupergraphStateError(#[from] SupergraphStateError),
 }

lib/query-planner/src/planner/best.rs

@@ -95,10 +95,10 @@ impl Candidate {
 
     #[inline]
     fn get_tree(&self, graph: &Graph) -> Result<QueryTree, QueryPlanError> {
-        self.tree
+        Ok(self
+            .tree
             .get_or_create(|(p, mp)| QueryTree::from_path(graph, &p, mp))
-            .clone()
-            .map_err(Into::into)
+            .clone()?)
     }
 
     #[inline]

lib/query-planner/src/planner/error.rs

@@ -8,9 +8,9 @@ use super::fetch::error::FetchGraphError;
 #[derive(Debug, Clone, thiserror::Error)]
 pub enum QueryPlanError {
     #[error("FetchGraph error: {0}")]
-    FetchGraphFailure(Box<FetchGraphError>),
+    FetchGraphFailure(#[from] FetchGraphError),
     #[error("Graph error: {0}")]
-    GraphFailure(Box<GraphError>),
+    GraphFailure(#[from] GraphError),
     #[error("Root fetch is missing")]
     NoRoot,
     #[error("Failed to build a plan")]
@@ -24,15 +24,3 @@ pub enum QueryPlanError {
     #[error(transparent)]
     CancellationError(#[from] CancellationError),
 }
-
-impl From<FetchGraphError> for QueryPlanError {
-    fn from(error: FetchGraphError) -> Self {
-        QueryPlanError::FetchGraphFailure(Box::new(error))
-    }
-}
-
-impl From<GraphError> for QueryPlanError {
-    fn from(error: GraphError) -> Self {
-        QueryPlanError::GraphFailure(Box::new(error))
-    }
-}

lib/query-planner/src/planner/fetch/error.rs

@@ -10,7 +10,7 @@ pub enum FetchGraphError {
     #[error("Internal Error: {0}")]
     Internal(String),
     #[error("Graph error: {0}")]
-    GraphFailure(Box<GraphError>),
+    GraphFailure(#[from] GraphError),
     #[error("Missing FetchStep: {0} {1}")]
     MissingStep(usize, String),
     #[error("Missing parent of FetchStep: {0}")]
@@ -30,7 +30,7 @@ pub enum FetchGraphError {
     #[error("Expected different indexes: {0}")]
     SameNodeIndex(usize),
     #[error("Failed ot find satisfiable key for @requires: {0}")]
-    SatisfiableKeyFailure(Box<WalkOperationError>),
+    SatisfiableKeyFailure(#[from] WalkOperationError),
     #[error("Expected a FetchStep with Mutation to have its order defined")]
     MutationStepWithNoOrder,
     #[error("Index mapping got lost")]
@@ -52,9 +52,3 @@ pub enum FetchGraphError {
     #[error(transparent)]
     CancellationError(#[from] CancellationError),
 }
-
-impl From<GraphError> for FetchGraphError {
-    fn from(error: GraphError) -> Self {
-        FetchGraphError::GraphFailure(Box::new(error))
-    }
-}

lib/query-planner/src/planner/fetch/fetch_graph.rs

@@ -1630,8 +1630,7 @@ fn find_satisfiable_key<'a>(
             // as the result of this function is guaranteed to be successful,
             // and fast.
             &CancellationToken::new(),
-        )
-        .map_err(|err| FetchGraphError::SatisfiableKeyFailure(Box::new(err)))?
+        )?
         .is_some()
         {
             return edge_ref

lib/query-planner/src/planner/mod.rs

@@ -37,52 +37,17 @@ pub struct Planner {
 #[derive(Debug, Clone, thiserror::Error)]
 pub enum PlannerError {
     #[error("failed to initalize relations graph: {0}")]
-    GraphInitError(Box<GraphError>),
+    GraphInitError(#[from] GraphError),
     #[error("failed to locate operation to execute")]
     MissingOperationToExecute,
     #[error("walker failed to locate path: {0}")]
-    PathLocatorError(Box<WalkOperationError>),
+    PathLocatorError(#[from] WalkOperationError),
     #[error("failed to build fetch graph: {0}")]
-    FailedToConstructFetchGraph(Box<FetchGraphError>),
+    FailedToConstructFetchGraph(#[from] FetchGraphError),
     #[error("failed to build plan: {0}")]
-    QueryPlanBuildFailed(Box<QueryPlanError>),
-    #[error("cancelled")]
-    Cancelled,
-    #[error("timedout")]
-    Timedout,
-}
-
-impl From<GraphError> for PlannerError {
-    fn from(value: GraphError) -> Self {
-        PlannerError::GraphInitError(Box::new(value))
-    }
-}
-
-impl From<WalkOperationError> for PlannerError {
-    fn from(value: WalkOperationError) -> Self {
-        PlannerError::PathLocatorError(Box::new(value))
-    }
-}
-
-impl From<FetchGraphError> for PlannerError {
-    fn from(value: FetchGraphError) -> Self {
-        PlannerError::FailedToConstructFetchGraph(Box::new(value))
-    }
-}
-
-impl From<QueryPlanError> for PlannerError {
-    fn from(value: QueryPlanError) -> Self {
-        PlannerError::QueryPlanBuildFailed(Box::new(value))
-    }
-}
-
-impl From<CancellationError> for PlannerError {
-    fn from(value: CancellationError) -> Self {
-        match value {
-            CancellationError::Cancelled => PlannerError::Cancelled,
-            CancellationError::TimedOut => PlannerError::Timedout,
-        }
-    }
+    QueryPlanBuildFailed(#[from] QueryPlanError),
+    #[error(transparent)]
+    CancellationError(#[from] CancellationError),
 }
 
 impl Planner {