Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

graph/subgraph misusage errors #459

Merged
merged 5 commits into from
May 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/rover-client/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,7 @@ pub enum RoverClientError {
/// could not parse the latest version
#[error("Could not get the latest release version")]
UnparseableReleaseVersion,

#[error("This endpoint doesn't support subgraph introspection via the Query._service field")]
SubgraphIntrospectionNotAvailable,
}
2 changes: 1 addition & 1 deletion crates/rover-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ pub mod introspection;
/// Module for client related errors.
pub use error::RoverClientError;

/// Module for actually querying studio
#[allow(clippy::upper_case_acronyms)]
/// Module for actually querying studio
pub mod query;

/// Module for getting release info
Expand Down
7 changes: 7 additions & 0 deletions crates/rover-client/src/query/config/is_federated.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
query IsFederatedGraph($graphId: ID!, $graphVariant: String!) {
service(id: $graphId) {
implementingServices(graphVariant: $graphVariant) {
__typename
}
}
}
49 changes: 49 additions & 0 deletions crates/rover-client/src/query/config/is_federated.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// PublishPartialSchemaMutation
use crate::blocking::StudioClient;
use crate::RoverClientError;
use graphql_client::*;

#[derive(GraphQLQuery)]
// The paths are relative to the directory where your `Cargo.toml` is located.
// Both json and the GraphQL schema language are supported as sources for the schema
#[graphql(
query_path = "src/query/config/is_federated.graphql",
schema_path = ".schema/schema.graphql",
response_derives = "PartialEq, Debug, Serialize, Deserialize",
deprecated = "warn"
)]
/// This struct is used to generate the module containing `Variables` and
/// `ResponseData` structs.
/// Snake case of this name is the mod name. i.e. publish_partial_schema_mutation
pub struct IsFederatedGraph;

#[derive(Debug, PartialEq)]
pub struct IsFederatedGraphResponse {
pub result: bool,
}

pub fn run(
variables: is_federated_graph::Variables,
client: &StudioClient,
) -> Result<IsFederatedGraphResponse, RoverClientError> {
let data = client.post::<IsFederatedGraph>(variables)?;
let is_federated_response = data.service.unwrap();
Ok(build_response(is_federated_response))
}

type FederatedResponse = is_federated_graph::IsFederatedGraphService;
type ImplementingServices = is_federated_graph::IsFederatedGraphServiceImplementingServices;

fn build_response(service: FederatedResponse) -> IsFederatedGraphResponse {
match service.implementing_services {
Some(typename) => match typename {
ImplementingServices::FederatedImplementingServices => {
IsFederatedGraphResponse { result: true }
}
ImplementingServices::NonFederatedImplementingService => {
IsFederatedGraphResponse { result: false }
}
},
None => IsFederatedGraphResponse { result: false },
}
}
3 changes: 3 additions & 0 deletions crates/rover-client/src/query/config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
/// runner for rover config whoami
pub mod whoami;

/// runner is_federated check
pub mod is_federated;
16 changes: 14 additions & 2 deletions crates/rover-client/src/query/subgraph/introspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,20 @@ pub fn run(
) -> Result<IntrospectionResponse, RoverClientError> {
// let graph = variables.graph_id.clone();
let variables = introspection_query::Variables {};
let response_data = client.post::<IntrospectionQuery>(variables, headers)?;
build_response(response_data)
let response_data = client.post::<IntrospectionQuery>(variables, headers);
match response_data {
Ok(data) => build_response(data),
Err(e) => {
// this is almost definitely a result of a graph not
// being federated, or not matching the federation spec
if e.to_string().contains("Cannot query field") {
Err(RoverClientError::SubgraphIntrospectionNotAvailable)
} else {
Err(e)
}
}
}
// build_response(response_data)
}

fn build_response(
Expand Down
22 changes: 20 additions & 2 deletions src/command/subgraph/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use ansi_term::Colour::Red;
use serde::Serialize;
use structopt::StructOpt;

use crate::Result;
use rover_client::query::subgraph::check;
use crate::{anyhow, error::RoverError, Result, Suggestion};
use rover_client::query::{config::is_federated, subgraph::check};

use crate::command::RoverStdout;
use crate::utils::client::StudioClientConfig;
Expand Down Expand Up @@ -65,6 +65,24 @@ impl Check {

let sdl = load_schema_from_flag(&self.schema, std::io::stdin())?;

// This response is used to check whether or not the current graph is federated.
let federated_response = is_federated::run(
is_federated::is_federated_graph::Variables {
graph_id: self.graph.name.clone(),
graph_variant: self.graph.variant.clone(),
},
&client,
)?;

// We don't want to run subgraph check on a non-federated graph, so
// return an error and recommend running graph check instead.
if !federated_response.result {
let err = anyhow!("Not able to run subgraph check on a non-federated graph.");
let mut err = RoverError::new(err);
err.set_suggestion(Suggestion::UseFederatedGraph);
return Err(err);
}

let partial_schema = check::check_partial_schema_query::PartialSchemaInput {
sdl: Some(sdl),
// we never need to send the hash since the back end computes it from SDL
Expand Down
55 changes: 42 additions & 13 deletions src/command/subgraph/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,22 @@ use ansi_term::Colour::{Cyan, Red, Yellow};
use serde::Serialize;
use structopt::StructOpt;

use crate::command::RoverStdout;
use crate::utils::{
client::StudioClientConfig,
git::GitContext,
loaders::load_schema_from_flag,
parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource},
use crate::{anyhow, Result};
use crate::{command::RoverStdout, error::RoverError};
use crate::{
utils::{
client::StudioClientConfig,
git::GitContext,
loaders::load_schema_from_flag,
parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource},
},
Suggestion,
};
use crate::Result;

use rover_client::query::subgraph::publish::{self, PublishPartialSchemaResponse};
use rover_client::query::{
config::is_federated,
subgraph::publish::{self, PublishPartialSchemaResponse},
};

#[derive(Debug, Serialize, StructOpt)]
pub struct Publish {
Expand All @@ -37,6 +43,10 @@ pub struct Publish {
#[serde(skip_serializing)]
subgraph: String,

/// Indicate whether to convert a non-federated graph into a subgraph
#[structopt(short, long)]
convert: bool,

/// Url of a running subgraph that a gateway can route operations to
/// (often a deployed subgraph). May be left empty ("") or a placeholder url
/// if not running a gateway in managed federation mode
Expand Down Expand Up @@ -64,6 +74,25 @@ impl Publish {

tracing::debug!("Schema Document to publish:\n{}", &schema_document);

// This response is used to check whether or not the current graph is federated.
let federated_response = is_federated::run(
is_federated::is_federated_graph::Variables {
graph_id: self.graph.name.clone(),
graph_variant: self.graph.variant.clone(),
},
&client,
)?;

// We don't want to implicitly convert non-federated graph to subgraphs.
// Error here if no --convert flag is passed _and_ the current context
// is non-federated. Add a suggestion to require a --convert flag.
if !federated_response.result && !self.convert {
let err = anyhow!("Could not publish a subgraph to a non-federated graph.");
let mut err = RoverError::new(err);
err.set_suggestion(Suggestion::ConvertGraphToSubgraph);
return Err(err);
}

let publish_response = publish::run(
publish::publish_partial_schema_mutation::Variables {
graph_id: self.graph.name.clone(),
Expand All @@ -81,12 +110,12 @@ impl Publish {
&client,
)?;

handle_response(publish_response, &self.subgraph, &self.graph.name);
handle_publish_response(publish_response, &self.subgraph, &self.graph.name);
Ok(RoverStdout::None)
}
}

fn handle_response(response: PublishPartialSchemaResponse, subgraph: &str, graph: &str) {
fn handle_publish_response(response: PublishPartialSchemaResponse, subgraph: &str, graph: &str) {
if response.service_was_created {
eprintln!(
"A new subgraph called '{}' for the '{}' graph was created",
Expand Down Expand Up @@ -120,7 +149,7 @@ fn handle_response(response: PublishPartialSchemaResponse, subgraph: &str, graph

#[cfg(test)]
mod tests {
use super::{handle_response, PublishPartialSchemaResponse};
use super::{handle_publish_response, PublishPartialSchemaResponse};

// this test is a bit weird, since we can't test the output. We just verify it
// doesn't error
Expand All @@ -133,7 +162,7 @@ mod tests {
composition_errors: None,
};

handle_response(response, "accounts", "my-graph");
handle_publish_response(response, "accounts", "my-graph");
}

#[test]
Expand All @@ -148,7 +177,7 @@ mod tests {
]),
};

handle_response(response, "accounts", "my-graph");
handle_publish_response(response, "accounts", "my-graph");
}

// TODO: test the actual output of the logs whenever we do design work
Expand Down
3 changes: 2 additions & 1 deletion src/error/metadata/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ impl From<&mut anyhow::Error> for Metadata {
RoverClientError::InvalidSeverity => {
(Some(Suggestion::SubmitIssue), Some(Code::E006))
}
RoverClientError::ExpectedFederatedGraph { graph: _ } => {
RoverClientError::SubgraphIntrospectionNotAvailable
| RoverClientError::ExpectedFederatedGraph { graph: _ } => {
(Some(Suggestion::UseFederatedGraph), Some(Code::E007))
}
RoverClientError::NoSchemaForVariant {
Expand Down
6 changes: 4 additions & 2 deletions src/error/metadata/suggestion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ pub enum Suggestion {
ProperKey,
NewUserNoProfiles,
CheckServerConnection,
ConvertGraphToSubgraph,
}

impl Display for Suggestion {
Expand Down Expand Up @@ -66,7 +67,7 @@ impl Display for Suggestion {
format!("Try resolving the composition errors in your subgraph(s), and publish them with the {} command.", Yellow.normal().paint("`rover subgraph publish`"))
}
Suggestion::UseFederatedGraph => {
"Try running the command on a valid federated graph.".to_string()
"Try running the command on a valid federated graph or use the appropriate `rover graph` command instead of `rover subgraph`.".to_string()
}
Suggestion::CheckGraphNameAndAuth => {
"Make sure your graph name is typed correctly, and that your API key is valid. (Are you using the right profile?)".to_string()
Expand Down Expand Up @@ -124,7 +125,8 @@ impl Display for Suggestion {
)
}
Suggestion::Adhoc(msg) => msg.to_string(),
Suggestion::CheckServerConnection => "Make sure the endpoint accepting connections is spelled correctly".to_string()
Suggestion::CheckServerConnection => "Make sure the endpoint accepting connections is spelled correctly".to_string(),
Suggestion::ConvertGraphToSubgraph => "If you are sure you want to convert a non-federated graph to a subgraph, you can re-run the same command with a `--convert` flag.".to_string(),


};
Expand Down