Address feedback from Avery

Author: sachindshinde

crates/rover-client/src/error.rs

@@ -139,8 +139,13 @@ pub enum RoverClientError {
         check_response: CheckResponse,
     },
 
-    #[error("The check failed for reasons this Rover version doesn't understand.")]
-    UnknownCheckFailure { target_url: String },
+    // While checking the proposed schema, the operations task (and also build task, if run) succeeded,
+    // but other check tasks failed.
+    #[error("{}", other_check_task_failure_msg(.has_build_task))]
+    OtherCheckTaskFailure {
+        has_build_task: bool,
+        target_url: String,
+    },
 
     /// This error occurs when a user has a malformed Graph Ref
     #[error("Graph IDs must be in the format <NAME> or <NAME>@<VARIANT>, where <NAME> can only contain letters, numbers, or the characters `-` or `_`, and must be 64 characters or less. <VARIANT> must be 64 characters or less.")]
@@ -182,6 +187,18 @@ pub enum RoverClientError {
     ChecksTimeoutError { url: Option<String> },
 }
 
+fn other_check_task_failure_msg(has_build_task: &bool) -> String {
+    let succeeding_tasks = if *has_build_task {
+        "build and operations tasks have".to_string()
+    } else {
+        "operations task has".to_string()
+    };
+    format!(
+        "The {} succeeded, but other check tasks have failed.",
+        succeeding_tasks
+    )
+}
+
 fn operation_check_error_msg(check_response: &CheckResponse) -> String {
     let failure_count = check_response.get_failure_count();
     let plural = match failure_count {

crates/rover-client/src/operations/graph/check_workflow/runner.rs

@@ -108,6 +108,7 @@ fn get_check_response_from_data(
         changes,
         status,
         graph_ref,
+        false,
         core_schema_modified,
     )
 }

crates/rover-client/src/operations/subgraph/check_workflow/runner.rs

@@ -120,6 +120,7 @@ fn get_check_response_from_data(
             changes,
             status,
             graph_ref,
+            true,
             core_schema_modified,
         )
     } else {

crates/rover-client/src/shared/check_response.rs

@@ -30,6 +30,7 @@ impl CheckResponse {
         changes: Vec<SchemaChange>,
         result: ChangeSeverity,
         graph_ref: GraphRef,
+        has_build_task: bool,
         core_schema_modified: bool,
     ) -> Result<CheckResponse, RoverClientError> {
         let mut failure_count = 0;
@@ -56,7 +57,8 @@ impl CheckResponse {
         }
         match check_response.result {
             ChangeSeverity::PASS => Ok(check_response),
-            ChangeSeverity::FAIL => Err(RoverClientError::UnknownCheckFailure {
+            ChangeSeverity::FAIL => Err(RoverClientError::OtherCheckTaskFailure {
+                has_build_task,
                 target_url: check_response.target_url.unwrap_or_else(||
                     // Note that graph IDs and variants don't need percent-encoding due to their regex restrictions.
                     format!("https://studio.apollographql.com/graph/{}/checks?variant={}", graph_ref.name, graph_ref.variant)

src/command/output.rs

@@ -835,6 +835,7 @@ mod tests {
             ChangeSeverity::PASS,
             graph_ref,
             true,
+            true,
         );
         if let Ok(mock_check_response) = mock_check_response {
             let actual_json: JsonOutput = RoverOutput::CheckResponse(mock_check_response).into();
@@ -891,6 +892,7 @@ mod tests {
             ],
             ChangeSeverity::FAIL, graph_ref,
             false,
+            false,
         );
 
         if let Err(operation_check_failure) = check_response {

src/error/metadata/codes/E036.md

@@ -1 +1 @@
-This error occurs when a check fails but this version of Rover doesn't know how to present the check results. Please upgrade Rover, or view the check in [Apollo Studio](https://studio.apollographql.com/) at the provided link. You can read more about client checks [here](https://www.apollographql.com/docs/studio/schema-checks/).
+This check error occurs when the operations task (and build task, if run) have succeeded, but some other check task has failed. Please view the check in [Apollo Studio](https://studio.apollographql.com/) at the provided link to see the failure reason. You can read more about client checks [here](https://www.apollographql.com/docs/studio/schema-checks/).

src/error/metadata/mod.rs

@@ -89,8 +89,11 @@ impl From<&mut saucer::Error> for Metadata {
                     }),
                     Some(Code::E030),
                 ),
-                RoverClientError::UnknownCheckFailure { target_url } => (
-                    Some(Suggestion::FixUnknownCheckFailure {
+                RoverClientError::OtherCheckTaskFailure {
+                    has_build_task,
+                    target_url,
+                } => (
+                    Some(Suggestion::FixOtherCheckTaskFailure {
                         target_url: target_url.clone(),
                     }),
                     Some(Code::E036),

src/error/metadata/suggestion.rs

@@ -43,7 +43,7 @@ pub enum Suggestion {
     FixOperationsInSchema {
         graph_ref: GraphRef,
     },
-    FixUnknownCheckFailure {
+    FixOtherCheckTaskFailure {
         target_url: String,
     },
     IncreaseClientTimeout,
@@ -165,7 +165,7 @@ impl Display for Suggestion {
             Suggestion::FixSubgraphSchema { graph_ref, subgraph } => format!("The changes in the schema you proposed for subgraph {} are incompatible with supergraph {}. See {} for more information on resolving build errors.", Yellow.normal().paint(subgraph.to_string()), Yellow.normal().paint(graph_ref.to_string()), Cyan.normal().paint("https://www.apollographql.com/docs/federation/errors/")),
             Suggestion::FixCompositionErrors => format!("The subgraph schemas you provided are incompatible with each other. See {} for more information on resolving build errors.", Cyan.normal().paint("https://www.apollographql.com/docs/federation/errors/")),
             Suggestion::FixOperationsInSchema { graph_ref } => format!("The changes in the schema you proposed are incompatible with graph {}. See {} for more information on resolving operation check errors.", Yellow.normal().paint(graph_ref.to_string()), Cyan.normal().paint("https://www.apollographql.com/docs/studio/schema-checks/")),
-            Suggestion::FixUnknownCheckFailure { target_url } => format!("See {} to view the failure reason for the check.", Cyan.normal().paint(target_url)),
+            Suggestion::FixOtherCheckTaskFailure { target_url } => format!("See {} to view the failure reason for the check.", Cyan.normal().paint(target_url)),
             Suggestion::IncreaseClientTimeout => "You can try increasing the timeout value by passing a higher value to the --client-timeout option.".to_string(),
             Suggestion::IncreaseChecksTimeout {url} => format!("You can try increasing the timeout value by setting APOLLO_CHECKS_TIMEOUT_SECONDS to a higher value in your env. The default value is 300 seconds. You can also view the live check progress by visiting {}.", Cyan.normal().paint(url.clone().unwrap_or_else(|| "https://studio.apollographql.com".to_string()))),
             Suggestion::FixChecksInput { graph_ref } => format!("Graph {} has no published schema or is not a composition variant. Please publish a schema or use a different variant.", Yellow.normal().paint(graph_ref.to_string())),