fix error message when querying non existent field (#1817)

close #1816

Signed-off-by: Benjamin Coenen <[email protected]>
Co-authored-by: Jesse Rosenberger <[email protected]>

Author: bnjjj

NEXT_CHANGELOG.md

@@ -104,7 +104,24 @@ By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/p
 
 ## 🛠 Maintenance
 
-### Update apollo-router-scaffold to use the published router crate [PR #1782](https://github.com/apollographql/router/pull/1782)
+### Improve error message when querying non existent field [Issue #1816](https://github.com/apollographql/router/issues/1816)
+
+When querying a non existent field you will get a better error.
+
+```patch
+{
+  "errors": [
+    {
+-       "message": "invalid type error, expected another type than 'Named type Computer'"
++       "message": "Cannot query field \"xxx\" on type \"Computer\""
+    }
+  ]
+}
+```
+
+By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/1817
+
+### Update `apollo-router-scaffold` to use the published `apollo-router` crate [PR #1782](https://github.com/apollographql/router/pull/1782)
 
 Now that apollo-router version "1.0.0-rc.0" is released on [crates.io](https://crates.io/crates/apollo-router), we can update scaffold to it relies on the published crate instead of the git tag.
 

apollo-router/src/spec/field_type.rs

@@ -1,5 +1,4 @@
 use apollo_parser::ast;
-use displaydoc::Display;
 
 use crate::json_ext::Value;
 use crate::json_ext::ValueExt;
@@ -9,7 +8,7 @@ use crate::*;
 pub(crate) struct InvalidValue;
 
 // Primitives are taken from scalars: https://spec.graphql.org/draft/#sec-Scalars
-#[derive(Debug, Display, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub(crate) enum FieldType {
     /// Only used for introspection queries when types are prefixed by __
     Introspection(String),
@@ -31,6 +30,21 @@ pub(crate) enum FieldType {
     Boolean,
 }
 
+impl std::fmt::Display for FieldType {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            FieldType::Introspection(ty) | FieldType::Named(ty) => write!(f, "{ty}"),
+            FieldType::List(ty) => write!(f, "[{ty}]"),
+            FieldType::NonNull(ty) => write!(f, "{ty}!"),
+            FieldType::String => write!(f, "String"),
+            FieldType::Int => write!(f, "Int"),
+            FieldType::Float => write!(f, "Float"),
+            FieldType::Id => write!(f, "ID"),
+            FieldType::Boolean => write!(f, "Boolean"),
+        }
+    }
+}
+
 impl FieldType {
     // This function validates input values according to the graphql specification.
     // Each of the values are validated against the "input coercion" rules.

apollo-router/src/spec/mod.rs

@@ -21,6 +21,8 @@ pub(crate) enum SpecError {
     RecursionLimitExceeded,
     /// invalid type error, expected another type than '{0}'
     InvalidType(String),
+    /// cannot query field '{0}' on type '{1}'
+    InvalidField(String, String),
     /// parsing error: {0}
     ParsingError(String),
     /// subscription operation is not supported

apollo-router/src/spec/query/tests.rs

@@ -1649,7 +1649,7 @@ fn filter_nested_object_errors() {
         }})
         .expected_errors(json! {[
             {
-                "message": "Cannot return null for non-nullable field Named type Review.text2",
+                "message": "Cannot return null for non-nullable field Review.text2",
                 "path": ["me", "reviews1", 0]
             }
         ]})

apollo-router/src/spec/selection.rs

@@ -97,23 +97,25 @@ impl Selection {
                 } else if field_name == "__type" {
                     FieldType::Introspection("__Type".to_string())
                 } else {
-                    current_type
+                    let name = current_type
                         .inner_type_name()
-                        .and_then(|name| {
-                            //looking into object types
+                        .ok_or_else(|| SpecError::InvalidType(current_type.to_string()))?;
+
+                    //looking into object types
+                    schema
+                        .object_types
+                        .get(name)
+                        .and_then(|ty| ty.field(&field_name))
+                        // otherwise, it might be an interface
+                        .or_else(|| {
                             schema
-                                .object_types
+                                .interfaces
                                 .get(name)
                                 .and_then(|ty| ty.field(&field_name))
-                                // otherwise, it might be an interface
-                                .or_else(|| {
-                                    schema
-                                        .interfaces
-                                        .get(name)
-                                        .and_then(|ty| ty.field(&field_name))
-                                })
                         })
-                        .ok_or_else(|| SpecError::InvalidType(current_type.to_string()))?
+                        .ok_or_else(|| {
+                            SpecError::InvalidField(field_name.clone(), current_type.to_string())
+                        })?
                         .clone()
                 };