graphql-rust / juniper

GraphQL server library for Rust
Other
5.66k stars 419 forks source link

errors path doesn't include the index when error occurs in an array/vec #491

Open sbditto85 opened 4 years ago

sbditto85 commented 4 years ago

Describe the bug Errors that occur in a list of items do not have the index in the path.

To Reproduce Steps to reproduce the behavior: Building off the actix_web example I used this as my schema.rs

use juniper::{FieldResult, FieldError};
use juniper::RootNode;
use juniper;

use std::collections::HashMap;

pub struct Database {
    pub humans: HashMap<i32, String>,
}

impl juniper::Context for Database {}

pub struct Human {
    id: String,
    name: String,
}

#[juniper::object(
    name = "Human",
    Context = Database,
)]
impl Human {

    pub fn id(&self) -> &str {
        &self.id
    }

    pub fn name(&self, context: &Database) -> FieldResult<Option<&str>> {
        if context.humans.len() > 0 {
            if "testing" == self.name { //Fake Check for permission for the field
                Ok(Some(&self.name))
            } else {
                Err(FieldError::new(
                    "DENIED",
                    graphql_value!({ "internal_error": "DENIED" })
                ))
            }
        } else {
            Err(FieldError::new(
                "Could not open connection to the database",
                graphql_value!({ "internal_error": "Connection refused" })
            ))
        }
    }

    pub fn humans(context: &Database) -> Vec<Human> {
        // Fake a query to get a friend
        vec![Human {id: "456".to_string(), name: "not_testing".to_string()}]
    }

}

#[derive(GraphQLInputObject)]
#[graphql(description = "A humanoid creature in the Star Wars universe")]
struct NewHuman {
    name: String,
}

pub struct QueryRoot;

#[juniper::object(
    name = "QueryRoot",
    Context = Database,
)]
impl QueryRoot {
    pub fn human(id: String, context: &Database) -> FieldResult<Vec<Human>> {
        let humans = context.humans.iter()
            // .filter(|(_, name)| id == **name)
            .map(|(_, name)| Human { id: name.clone(), name: name.clone() })
            .collect();
        Ok(humans)
    }

    pub fn count(context: &Database) -> i32 {
        context.humans.len() as i32
    }
}

pub struct MutationRoot;

#[juniper::object(
    name = "MutationRoot",
    Context = Database,
)]
impl MutationRoot {
    pub fn human(id: String, new_human: NewHuman) -> Human {
        unimplemented!()
        // Human{
        //     id,
        //     name: Ok(new_human.name),
        //     // appears_in: new_human.appears_in,
        //     // home_planet: new_human.home_planet,
        // }
    }
}

pub type Schema = RootNode<'static, QueryRoot, MutationRoot>;

pub fn create_schema() -> Schema {
    Schema::new(QueryRoot {}, MutationRoot {})
}

and this as my main.rs

//! Actix web juniper example
//!
//! A simple example integrating juniper in actix-web
use std::io;
use std::sync::Arc;

#[macro_use]
extern crate juniper;

use actix_web::{middleware, web, App, Error, HttpResponse, HttpServer};
use juniper::http::graphiql::graphiql_source;
use juniper::http::GraphQLRequest;

mod schema;

use crate::schema::{create_schema, Schema, Database};

use std::collections::HashMap;

async fn graphiql() -> HttpResponse {
    let html = graphiql_source("http://127.0.0.1:8080/graphql");
    HttpResponse::Ok()
        .content_type("text/html; charset=utf-8")
        .body(html)
}

async fn graphql(
    st: web::Data<Arc<Schema>>,
    data: web::Json<GraphQLRequest>,
) -> Result<HttpResponse, Error> {
    let user = web::block(move || {
        let mut humans = HashMap::new();
        humans.insert(123, "Luke".to_string());
        humans.insert(321, "testing".to_string());
        let db = Database { humans };
        let res = data.execute(&st, &db);
        Ok::<_, serde_json::error::Error>(serde_json::to_string(&res)?)
    })
    .await?;
    Ok(HttpResponse::Ok()
        .content_type("application/json")
        .header("Access-Control-Allow-Origin", "http://localhost:8080")
        .header("Access-Control-Allow-Credentials", "true")
        .header("Access-Control-Allow-Headers", "content-type")
        .body(user))
}

async fn handle_options() -> Result<HttpResponse, Error> {
    Ok(HttpResponse::Ok()
       .header("Access-Control-Allow-Origin", "http://localhost:8080")
       .header("Access-Control-Allow-Credentials", "true")
       .header("Access-Control-Allow-Headers", "content-type")
       .body("")
    )
}

#[actix_rt::main]
async fn main() -> io::Result<()> {
    std::env::set_var("RUST_LOG", "actix_web=info");
    env_logger::init();

    // Create Juniper schema
    let schema = std::sync::Arc::new(create_schema());

    // Start http server
    HttpServer::new(move || {
        App::new()
            .data(schema.clone())
            .wrap(middleware::Logger::default())
            .service(web::resource("/graphql")
                     .route(web::post().to(graphql))
                     .route(actix_web::Route::new().method(actix_web::http::Method::OPTIONS).to(handle_options))
            )
            .service(web::resource("/graphiql").route(web::get().to(graphiql)))
    })
    .bind("127.0.0.1:8080")?
    .start()
    .await
}

when going to http://localhost:8080/graphiql and running the query:

{
  human(id: "Luke") {
    id
    name
    humans {
      id,
      name
    }
  }
}

I get the result:

{
  "data": {
    "human": [
      {
        "id": "testing",
        "name": "testing",
        "humans": [
          {
            "id": "456",
            "name": null
          }
        ]
      },
      {
        "id": "Luke",
        "name": null,
        "humans": [
          {
            "id": "456",
            "name": null
          }
        ]
      }
    ]
  },
  "errors": [
    {
      "message": "DENIED",
      "locations": [
        {
          "line": 4,
          "column": 5
        }
      ],
      "path": [
        "human",
        "name"
      ],
      "extensions": {
        "internal_error": "DENIED"
      }
    },
    {
      "message": "DENIED",
      "locations": [
        {
          "line": 7,
          "column": 7
        }
      ],
      "path": [
        "human",
        "humans",
        "name"
      ],
      "extensions": {
        "internal_error": "DENIED"
      }
    },
    {
      "message": "DENIED",
      "locations": [
        {
          "line": 7,
          "column": 7
        }
      ],
      "path": [
        "human",
        "humans",
        "name"
      ],
      "extensions": {
        "internal_error": "DENIED"
      }
    }
  ]
}

Expected behavior I expected the path value of the error to contain the index of the "human" that couldn't return its name and as such is null, otherwise I don't know exactly which error maps to which place. In the graphql documentation for a similar example the error includes an index see https://graphql.github.io/graphql-spec/June2018/#example-90475.

Since in my example the query is returning a list of humans that internally have a list of humans it would expect a result like:

{
  "data": {
    "human": [
      {
        "id": "testing",
        "name": "testing",
        "humans": [
          {
            "id": "456",
            "name": null
          }
        ]
      },
      {
        "id": "Luke",
        "name": null,
        "humans": [
          {
            "id": "456",
            "name": null
          }
        ]
      }
    ]
  },
  "errors": [
    {
      "message": "DENIED",
      "locations": [
        {
          "line": 4,
          "column": 5
        }
      ],
      "path": [
        "human",
         1,
        "name"
      ],
      "extensions": {
        "internal_error": "DENIED"
      }
    },
    {
      "message": "DENIED",
      "locations": [
        {
          "line": 7,
          "column": 7
        }
      ],
      "path": [
        "human",
         0,
        "humans",
         0,
        "name"
      ],
      "extensions": {
        "internal_error": "DENIED"
      }
    },
    {
      "message": "DENIED",
      "locations": [
        {
          "line": 7,
          "column": 7
        }
      ],
      "path": [
        "human",
         1,
        "humans",
         0,
        "name"
      ],
      "extensions": {
        "internal_error": "DENIED"
      }
    }
  ]
}

Note the paths now have indexes to indicate which item the error applies to in the path.

sbditto85 commented 4 years ago

Forgot to add my Cargo.toml

[package]
name = "juniper-example"
version = "0.2.0"
authors = ["pyros2097 <pyros2097@gmail.com>"]
workspace = ".."
edition = "2018"

[dependencies]
actix-cors = "0.2.0-alpha.3"
actix-web = "2.0.0-alpha.6"
actix-rt = "1.0.0"
env_logger = "0.7.1"
serde = "1.0.103"
serde_json = "1.0.44"
serde_derive = "1.0.103"
juniper = "0.14.1"

Using cargo tree (or just looking at the lock file) it shows using juniper 0.14.2 (current release on crates.io at time of report)