Skip to content

Commit bb81679

Browse files
JakeDawkinslrlna
andauthored
graph/subgraph misusage errors (#459)
This commit adds the following errors: * error when a non-federated graph is about to be overwritten by a subgraph * error when running subgraph check on a federated graph * error for attempts to introspect a subgraph on graphs which don't support _service Co-authored-by: Irina Shestak <[email protected]>
1 parent 5995c16 commit bb81679

File tree

10 files changed

+145
-21
lines changed

10 files changed

+145
-21
lines changed

crates/rover-client/src/error.rs

+3
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,7 @@ pub enum RoverClientError {
129129
/// could not parse the latest version
130130
#[error("Could not get the latest release version")]
131131
UnparseableReleaseVersion,
132+
133+
#[error("This endpoint doesn't support subgraph introspection via the Query._service field")]
134+
SubgraphIntrospectionNotAvailable,
132135
}

crates/rover-client/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ pub mod introspection;
1515
/// Module for client related errors.
1616
pub use error::RoverClientError;
1717

18-
/// Module for actually querying studio
1918
#[allow(clippy::upper_case_acronyms)]
19+
/// Module for actually querying studio
2020
pub mod query;
2121

2222
/// Module for getting release info
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
query IsFederatedGraph($graphId: ID!, $graphVariant: String!) {
2+
service(id: $graphId) {
3+
implementingServices(graphVariant: $graphVariant) {
4+
__typename
5+
}
6+
}
7+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// PublishPartialSchemaMutation
2+
use crate::blocking::StudioClient;
3+
use crate::RoverClientError;
4+
use graphql_client::*;
5+
6+
#[derive(GraphQLQuery)]
7+
// The paths are relative to the directory where your `Cargo.toml` is located.
8+
// Both json and the GraphQL schema language are supported as sources for the schema
9+
#[graphql(
10+
query_path = "src/query/config/is_federated.graphql",
11+
schema_path = ".schema/schema.graphql",
12+
response_derives = "PartialEq, Debug, Serialize, Deserialize",
13+
deprecated = "warn"
14+
)]
15+
/// This struct is used to generate the module containing `Variables` and
16+
/// `ResponseData` structs.
17+
/// Snake case of this name is the mod name. i.e. publish_partial_schema_mutation
18+
pub struct IsFederatedGraph;
19+
20+
#[derive(Debug, PartialEq)]
21+
pub struct IsFederatedGraphResponse {
22+
pub result: bool,
23+
}
24+
25+
pub fn run(
26+
variables: is_federated_graph::Variables,
27+
client: &StudioClient,
28+
) -> Result<IsFederatedGraphResponse, RoverClientError> {
29+
let data = client.post::<IsFederatedGraph>(variables)?;
30+
let is_federated_response = data.service.unwrap();
31+
Ok(build_response(is_federated_response))
32+
}
33+
34+
type FederatedResponse = is_federated_graph::IsFederatedGraphService;
35+
type ImplementingServices = is_federated_graph::IsFederatedGraphServiceImplementingServices;
36+
37+
fn build_response(service: FederatedResponse) -> IsFederatedGraphResponse {
38+
match service.implementing_services {
39+
Some(typename) => match typename {
40+
ImplementingServices::FederatedImplementingServices => {
41+
IsFederatedGraphResponse { result: true }
42+
}
43+
ImplementingServices::NonFederatedImplementingService => {
44+
IsFederatedGraphResponse { result: false }
45+
}
46+
},
47+
None => IsFederatedGraphResponse { result: false },
48+
}
49+
}
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
11
/// runner for rover config whoami
22
pub mod whoami;
3+
4+
/// runner is_federated check
5+
pub mod is_federated;

crates/rover-client/src/query/subgraph/introspect.rs

+14-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,20 @@ pub fn run(
2424
) -> Result<IntrospectionResponse, RoverClientError> {
2525
// let graph = variables.graph_id.clone();
2626
let variables = introspection_query::Variables {};
27-
let response_data = client.post::<IntrospectionQuery>(variables, headers)?;
28-
build_response(response_data)
27+
let response_data = client.post::<IntrospectionQuery>(variables, headers);
28+
match response_data {
29+
Ok(data) => build_response(data),
30+
Err(e) => {
31+
// this is almost definitely a result of a graph not
32+
// being federated, or not matching the federation spec
33+
if e.to_string().contains("Cannot query field") {
34+
Err(RoverClientError::SubgraphIntrospectionNotAvailable)
35+
} else {
36+
Err(e)
37+
}
38+
}
39+
}
40+
// build_response(response_data)
2941
}
3042

3143
fn build_response(

src/command/subgraph/check.rs

+20-2
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ use ansi_term::Colour::Red;
22
use serde::Serialize;
33
use structopt::StructOpt;
44

5-
use crate::Result;
6-
use rover_client::query::subgraph::check;
5+
use crate::{anyhow, error::RoverError, Result, Suggestion};
6+
use rover_client::query::{config::is_federated, subgraph::check};
77

88
use crate::command::RoverStdout;
99
use crate::utils::client::StudioClientConfig;
@@ -65,6 +65,24 @@ impl Check {
6565

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

68+
// This response is used to check whether or not the current graph is federated.
69+
let federated_response = is_federated::run(
70+
is_federated::is_federated_graph::Variables {
71+
graph_id: self.graph.name.clone(),
72+
graph_variant: self.graph.variant.clone(),
73+
},
74+
&client,
75+
)?;
76+
77+
// We don't want to run subgraph check on a non-federated graph, so
78+
// return an error and recommend running graph check instead.
79+
if !federated_response.result {
80+
let err = anyhow!("Not able to run subgraph check on a non-federated graph.");
81+
let mut err = RoverError::new(err);
82+
err.set_suggestion(Suggestion::UseFederatedGraph);
83+
return Err(err);
84+
}
85+
6886
let partial_schema = check::check_partial_schema_query::PartialSchemaInput {
6987
sdl: Some(sdl),
7088
// we never need to send the hash since the back end computes it from SDL

src/command/subgraph/publish.rs

+42-13
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,22 @@ use ansi_term::Colour::{Cyan, Red, Yellow};
22
use serde::Serialize;
33
use structopt::StructOpt;
44

5-
use crate::command::RoverStdout;
6-
use crate::utils::{
7-
client::StudioClientConfig,
8-
git::GitContext,
9-
loaders::load_schema_from_flag,
10-
parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource},
5+
use crate::{anyhow, Result};
6+
use crate::{command::RoverStdout, error::RoverError};
7+
use crate::{
8+
utils::{
9+
client::StudioClientConfig,
10+
git::GitContext,
11+
loaders::load_schema_from_flag,
12+
parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource},
13+
},
14+
Suggestion,
1115
};
12-
use crate::Result;
1316

14-
use rover_client::query::subgraph::publish::{self, PublishPartialSchemaResponse};
17+
use rover_client::query::{
18+
config::is_federated,
19+
subgraph::publish::{self, PublishPartialSchemaResponse},
20+
};
1521

1622
#[derive(Debug, Serialize, StructOpt)]
1723
pub struct Publish {
@@ -37,6 +43,10 @@ pub struct Publish {
3743
#[serde(skip_serializing)]
3844
subgraph: String,
3945

46+
/// Indicate whether to convert a non-federated graph into a subgraph
47+
#[structopt(short, long)]
48+
convert: bool,
49+
4050
/// Url of a running subgraph that a gateway can route operations to
4151
/// (often a deployed subgraph). May be left empty ("") or a placeholder url
4252
/// if not running a gateway in managed federation mode
@@ -64,6 +74,25 @@ impl Publish {
6474

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

77+
// This response is used to check whether or not the current graph is federated.
78+
let federated_response = is_federated::run(
79+
is_federated::is_federated_graph::Variables {
80+
graph_id: self.graph.name.clone(),
81+
graph_variant: self.graph.variant.clone(),
82+
},
83+
&client,
84+
)?;
85+
86+
// We don't want to implicitly convert non-federated graph to subgraphs.
87+
// Error here if no --convert flag is passed _and_ the current context
88+
// is non-federated. Add a suggestion to require a --convert flag.
89+
if !federated_response.result && !self.convert {
90+
let err = anyhow!("Could not publish a subgraph to a non-federated graph.");
91+
let mut err = RoverError::new(err);
92+
err.set_suggestion(Suggestion::ConvertGraphToSubgraph);
93+
return Err(err);
94+
}
95+
6796
let publish_response = publish::run(
6897
publish::publish_partial_schema_mutation::Variables {
6998
graph_id: self.graph.name.clone(),
@@ -81,12 +110,12 @@ impl Publish {
81110
&client,
82111
)?;
83112

84-
handle_response(publish_response, &self.subgraph, &self.graph.name);
113+
handle_publish_response(publish_response, &self.subgraph, &self.graph.name);
85114
Ok(RoverStdout::None)
86115
}
87116
}
88117

89-
fn handle_response(response: PublishPartialSchemaResponse, subgraph: &str, graph: &str) {
118+
fn handle_publish_response(response: PublishPartialSchemaResponse, subgraph: &str, graph: &str) {
90119
if response.service_was_created {
91120
eprintln!(
92121
"A new subgraph called '{}' for the '{}' graph was created",
@@ -120,7 +149,7 @@ fn handle_response(response: PublishPartialSchemaResponse, subgraph: &str, graph
120149

121150
#[cfg(test)]
122151
mod tests {
123-
use super::{handle_response, PublishPartialSchemaResponse};
152+
use super::{handle_publish_response, PublishPartialSchemaResponse};
124153

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

136-
handle_response(response, "accounts", "my-graph");
165+
handle_publish_response(response, "accounts", "my-graph");
137166
}
138167

139168
#[test]
@@ -148,7 +177,7 @@ mod tests {
148177
]),
149178
};
150179

151-
handle_response(response, "accounts", "my-graph");
180+
handle_publish_response(response, "accounts", "my-graph");
152181
}
153182

154183
// TODO: test the actual output of the logs whenever we do design work

src/error/metadata/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ impl From<&mut anyhow::Error> for Metadata {
5858
RoverClientError::InvalidSeverity => {
5959
(Some(Suggestion::SubmitIssue), Some(Code::E006))
6060
}
61-
RoverClientError::ExpectedFederatedGraph { graph: _ } => {
61+
RoverClientError::SubgraphIntrospectionNotAvailable
62+
| RoverClientError::ExpectedFederatedGraph { graph: _ } => {
6263
(Some(Suggestion::UseFederatedGraph), Some(Code::E007))
6364
}
6465
RoverClientError::NoSchemaForVariant {

src/error/metadata/suggestion.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ pub enum Suggestion {
3030
ProperKey,
3131
NewUserNoProfiles,
3232
CheckServerConnection,
33+
ConvertGraphToSubgraph,
3334
}
3435

3536
impl Display for Suggestion {
@@ -66,7 +67,7 @@ impl Display for Suggestion {
6667
format!("Try resolving the composition errors in your subgraph(s), and publish them with the {} command.", Yellow.normal().paint("`rover subgraph publish`"))
6768
}
6869
Suggestion::UseFederatedGraph => {
69-
"Try running the command on a valid federated graph.".to_string()
70+
"Try running the command on a valid federated graph or use the appropriate `rover graph` command instead of `rover subgraph`.".to_string()
7071
}
7172
Suggestion::CheckGraphNameAndAuth => {
7273
"Make sure your graph name is typed correctly, and that your API key is valid. (Are you using the right profile?)".to_string()
@@ -124,7 +125,8 @@ impl Display for Suggestion {
124125
)
125126
}
126127
Suggestion::Adhoc(msg) => msg.to_string(),
127-
Suggestion::CheckServerConnection => "Make sure the endpoint accepting connections is spelled correctly".to_string()
128+
Suggestion::CheckServerConnection => "Make sure the endpoint accepting connections is spelled correctly".to_string(),
129+
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(),
128130

129131

130132
};

0 commit comments

Comments
 (0)