feat: report downstream check task results (#1385)

Co-authored-by: Avery Harnish <[email protected]>

crates/rover-client/src/error.rs

@@ -145,11 +145,20 @@ pub enum RoverClientError {
         check_response: CheckResponse,
     },
 
-    // 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))]
+    /// While checking the proposed schema, we encountered changes that would cause checks to fail in
+    /// blocking downstream variants.
+    #[error("{}", downstream_check_error_msg(.blocking_downstream_variants))]
+    DownstreamCheckFailure {
+        blocking_downstream_variants: Vec<String>,
+        target_url: String,
+    },
+
+    /// While checking the proposed schema, the build, operations, and downstream (if run) tasks succeeded
+    /// or are pending, but other check tasks failed.
+    #[error("{}", other_check_task_failure_msg(.has_build_task,.has_downstream_task))]
     OtherCheckTaskFailure {
         has_build_task: bool,
+        has_downstream_task: bool,
         target_url: String,
     },
 
@@ -193,18 +202,6 @@ 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 {
-        "The build and operations tasks".to_string()
-    } else {
-        "The operations task".to_string()
-    };
-    format!(
-        "{} succeeded, but other check tasks 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 {
@@ -217,6 +214,37 @@ fn operation_check_error_msg(check_response: &CheckResponse) -> String {
     )
 }
 
+fn downstream_check_error_msg(downstream_blocking_variants: &Vec<String>) -> String {
+    let variants = downstream_blocking_variants.join(",");
+    let plural_this = match downstream_blocking_variants.len() {
+        1 => "this",
+        _ => "these",
+    };
+    let plural = match downstream_blocking_variants.len() {
+        1 => "",
+        _ => "s",
+    };
+    format!(
+        "The downstream check task has encountered check failures for at least {} blocking downstream variant{}: {}.",
+        plural_this,
+        plural,
+        variants,
+    )
+}
+
+fn other_check_task_failure_msg(has_build_task: &bool, has_downstream_task: &bool) -> String {
+    let succeeding_tasks = match (*has_build_task, *has_downstream_task) {
+        (false, false) => "The operations task",
+        (true, false) => "The build and operations tasks",
+        (true, true) => "The build, operations, and downstream tasks",
+        (false, true) => unreachable!("Can't have a downstream task without a build task"),
+    };
+    format!(
+        "{} succeeded or are pending, but other check tasks failed.",
+        succeeding_tasks
+    )
+}
+
 impl From<introspector_gadget::error::RoverClientError> for RoverClientError {
     fn from(e: introspector_gadget::error::RoverClientError) -> Self {
         match e {

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

@@ -7,8 +7,8 @@ use crate::RoverClientError;
 
 use graphql_client::*;
 
-use self::graph_check_workflow_query::CheckWorkflowStatus;
 use self::graph_check_workflow_query::GraphCheckWorkflowQueryGraphCheckWorkflowTasks::OperationsCheckTask;
+use self::graph_check_workflow_query::{CheckWorkflowStatus, CheckWorkflowTaskStatus};
 
 use super::types::OperationsResult;
 
@@ -69,14 +69,15 @@ fn get_check_response_from_data(
             graph_ref: graph_ref.clone(),
         })?;
 
-    let status = check_workflow.status.into();
+    let workflow_status = check_workflow.status;
+    let mut operations_status = None;
+    let mut operations_target_url = None;
     let mut operations_result: Option<OperationsResult> = None;
-    let mut target_url = None;
     let mut number_of_checked_operations: u64 = 0;
-    let mut changes = Vec::new();
     for task in check_workflow.tasks {
         if let OperationsCheckTask(task) = task {
-            target_url = task.target_url;
+            operations_status = Some(task.status);
+            operations_target_url = task.target_url;
             if let Some(result) = task.result {
                 number_of_checked_operations =
                     result.number_of_checked_operations.try_into().unwrap();
@@ -85,32 +86,48 @@ fn get_check_response_from_data(
         }
     }
 
-    if let Some(result) = operations_result {
+    if matches!(operations_status, Some(CheckWorkflowTaskStatus::FAILED))
+        || matches!(workflow_status, CheckWorkflowStatus::PASSED)
+    {
+        let result = operations_result.ok_or(RoverClientError::MalformedResponse {
+            null_field: "OperationsCheckTask.result".to_string(),
+        })?;
+        let mut changes = Vec::with_capacity(result.changes.len());
         for change in result.changes {
             changes.push(SchemaChange {
                 code: change.code,
                 severity: change.severity.into(),
                 description: change.description,
             });
         }
-    }
 
-    // The `graph` check response does not return this field
-    // only `subgraph` check does. Since `CheckResponse` is shared
-    // between `graph` and `subgraph` checks, defaulting this
-    // to false for now since its currently only used in
-    // `check_response.rs` to format better console messages.
-    let core_schema_modified = false;
+        // The `graph` check response does not return this field
+        // only `subgraph` check does. Since `CheckResponse` is shared
+        // between `graph` and `subgraph` checks, defaulting this
+        // to false for now since its currently only used in
+        // `check_response.rs` to format better console messages.
+        let core_schema_modified = false;
 
-    CheckResponse::try_new(
-        target_url,
-        number_of_checked_operations,
-        changes,
-        status,
-        graph_ref,
-        false,
-        core_schema_modified,
-    )
+        CheckResponse::try_new(
+            operations_target_url,
+            number_of_checked_operations,
+            changes,
+            workflow_status.into(),
+            graph_ref,
+            core_schema_modified,
+        )
+    } else {
+        // Note that graph IDs and variants don't need percent-encoding due to their regex restrictions.
+        let default_target_url = format!(
+            "https://studio.apollographql.com/graph/{}/checks?variant={}",
+            graph_ref.name, graph_ref.variant
+        );
+        Err(RoverClientError::OtherCheckTaskFailure {
+            has_build_task: false,
+            has_downstream_task: false,
+            target_url: operations_target_url.unwrap_or(default_target_url),
+        })
+    }
 }
 
 fn get_target_url_from_data(data: QueryResponseData) -> Option<String> {

crates/rover-client/src/operations/subgraph/check_workflow/check_workflow_query.graphql

@@ -31,7 +31,13 @@ query SubgraphCheckWorkflowQuery($workflowId: ID!, $graphId: ID!) {
             }
           }
         }
+        ... on DownstreamCheckTask {
+          results {
+            downstreamVariantName
+            failsUpstreamWorkflow
+          }
+        }
       }
     }
   }
-}
+}

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

@@ -11,9 +11,11 @@ use apollo_federation_types::build::BuildError;
 use graphql_client::*;
 
 use self::subgraph_check_workflow_query::CheckWorkflowStatus;
+use self::subgraph_check_workflow_query::CheckWorkflowTaskStatus;
 use self::subgraph_check_workflow_query::SubgraphCheckWorkflowQueryGraphCheckWorkflowTasksOn::{
-    CompositionCheckTask, OperationsCheckTask,
+    CompositionCheckTask, DownstreamCheckTask, OperationsCheckTask,
 };
+use self::subgraph_check_workflow_query::SubgraphCheckWorkflowQueryGraphCheckWorkflowTasksOnOperationsCheckTaskResult;
 
 #[derive(GraphQLQuery)]
 // The paths are relative to the directory where your `Cargo.toml` is located.
@@ -74,16 +76,21 @@ fn get_check_response_from_data(
             graph_ref: graph_ref.clone(),
         })?;
 
-    let status = check_workflow.status.into();
+    let workflow_status = check_workflow.status;
+    let mut operations_status = None;
+    let mut operations_target_url = None;
     let mut operations_result = None;
-    let mut target_url = None;
     let mut number_of_checked_operations: u64 = 0;
     let mut core_schema_modified = false;
     let mut composition_errors = Vec::new();
+    let mut downstream_status = None;
+    let mut downstream_target_url = None;
+    let mut blocking_downstream_variants = Vec::new();
     for task in check_workflow.tasks {
         match task.on {
             OperationsCheckTask(typed_task) => {
-                target_url = task.target_url;
+                operations_status = Some(task.status);
+                operations_target_url = task.target_url;
                 if let Some(result) = typed_task.result {
                     number_of_checked_operations =
                         result.number_of_checked_operations.try_into().unwrap();
@@ -96,34 +103,22 @@ fn get_check_response_from_data(
                     composition_errors = result.errors;
                 }
             }
+            DownstreamCheckTask(typed_task) => {
+                downstream_status = Some(task.status);
+                downstream_target_url = task.target_url;
+                if let Some(results) = typed_task.results {
+                    blocking_downstream_variants = results
+                        .iter()
+                        .filter(|result| result.fails_upstream_workflow.unwrap_or(false))
+                        .map(|result| result.downstream_variant_name.clone())
+                        .collect();
+                }
+            }
             _ => (),
         }
     }
 
-    if composition_errors.is_empty() {
-        let result = operations_result.ok_or(RoverClientError::AdhocError {
-            msg: "No operation was found for this check.".to_string(),
-        })?;
-
-        let mut changes = Vec::with_capacity(result.changes.len());
-        for change in result.changes {
-            changes.push(SchemaChange {
-                code: change.code,
-                severity: change.severity.into(),
-                description: change.description,
-            });
-        }
-
-        CheckResponse::try_new(
-            target_url,
-            number_of_checked_operations,
-            changes,
-            status,
-            graph_ref,
-            true,
-            core_schema_modified,
-        )
-    } else {
+    if !composition_errors.is_empty() {
         let num_failures = composition_errors.len();
 
         let mut build_errors = Vec::with_capacity(num_failures);
@@ -133,10 +128,47 @@ fn get_check_response_from_data(
                 Some(query_composition_error.message),
             ));
         }
-        Err(RoverClientError::SubgraphBuildErrors {
+        return Err(RoverClientError::SubgraphBuildErrors {
             subgraph,
             graph_ref,
             source: build_errors.into(),
+        });
+    }
+
+    // Note that graph IDs and variants don't need percent-encoding due to their regex restrictions.
+    let default_target_url = format!(
+        "https://studio.apollographql.com/graph/{}/checks?variant={}",
+        graph_ref.name, graph_ref.variant
+    );
+
+    if matches!(operations_status, Some(CheckWorkflowTaskStatus::FAILED)) {
+        get_check_response_from_result(
+            operations_result,
+            operations_target_url,
+            number_of_checked_operations,
+            workflow_status,
+            graph_ref,
+            core_schema_modified,
+        )
+    } else if matches!(downstream_status, Some(CheckWorkflowTaskStatus::FAILED)) {
+        Err(RoverClientError::DownstreamCheckFailure {
+            blocking_downstream_variants,
+            target_url: downstream_target_url.unwrap_or(default_target_url),
+        })
+    } else if matches!(workflow_status, CheckWorkflowStatus::PASSED) {
+        get_check_response_from_result(
+            operations_result,
+            operations_target_url,
+            number_of_checked_operations,
+            workflow_status,
+            graph_ref,
+            core_schema_modified,
+        )
+    } else {
+        Err(RoverClientError::OtherCheckTaskFailure {
+            has_build_task: true,
+            has_downstream_task: downstream_status.is_some(),
+            target_url: operations_target_url.unwrap_or(default_target_url),
         })
     }
 }
@@ -152,3 +184,35 @@ fn get_target_url_from_data(data: QueryResponseData) -> Option<String> {
     }
     target_url
 }
+
+fn get_check_response_from_result(
+    operations_result: Option<
+        SubgraphCheckWorkflowQueryGraphCheckWorkflowTasksOnOperationsCheckTaskResult,
+    >,
+    operations_target_url: Option<String>,
+    number_of_checked_operations: u64,
+    workflow_status: CheckWorkflowStatus,
+    graph_ref: GraphRef,
+    core_schema_modified: bool,
+) -> Result<CheckResponse, RoverClientError> {
+    let result = operations_result.ok_or(RoverClientError::AdhocError {
+        msg: "Operations check task has no result.".to_string(),
+    })?;
+    let mut changes = Vec::with_capacity(result.changes.len());
+    for change in result.changes {
+        changes.push(SchemaChange {
+            code: change.code,
+            severity: change.severity.into(),
+            description: change.description,
+        });
+    }
+
+    CheckResponse::try_new(
+        operations_target_url,
+        number_of_checked_operations,
+        changes,
+        workflow_status.into(),
+        graph_ref,
+        core_schema_modified,
+    )
+}

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

@@ -30,7 +30,6 @@ 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;
@@ -50,20 +49,12 @@ impl CheckResponse {
         };
 
         if failure_count > 0 {
-            return Err(RoverClientError::OperationCheckFailure {
+            Err(RoverClientError::OperationCheckFailure {
                 graph_ref,
                 check_response,
-            });
-        }
-        match check_response.result {
-            ChangeSeverity::PASS => Ok(check_response),
-            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)
-                )
-            }),
+            })
+        } else {
+            Ok(check_response)
         }
     }