ROVER-315 Make it so that the CompositionPipeline can run with bad fe…

…deration_version initially (#2386)

At the moment we have a lot of assumptions that are baked into how
`rover dev` and the LSP work. However, one of those assumptions needs to
be relaxed, which is that the `supergraph` binary could easily fail to
download the first time around. So the big conceptual shift here is that
the current sate of `supergraph now becomes a Result. In the OK case,
things work as before, but now in the Error case we can emit that error
that occurred.

src/command/dev/do_dev.rs

@@ -185,10 +185,12 @@ impl Dev {
         loop {
             match composition_messages.next().await {
                 Some(CompositionEvent::Started) => {
-                    eprintln!(
-                        "composing supergraph with Federation {}",
-                        composition_pipeline.state.supergraph_binary.version()
-                    );
+                    if let Ok(ref binary) = composition_pipeline.state.supergraph_binary {
+                        eprintln!(
+                            "composing supergraph with Federation {}",
+                            binary.version()
+                        );
+                    }
                 },
                 Some(CompositionEvent::Success(success)) => {
                     supergraph_schema = success.supergraph_sdl;

src/composition/mod.rs

@@ -1,11 +1,8 @@
 use std::fmt::Debug;
 
 use anyhow::Error;
-use apollo_federation_types::config::SchemaSource;
-use apollo_federation_types::{
-    config::FederationVersion,
-    rover::{BuildErrors, BuildHint},
-};
+use apollo_federation_types::config::{FederationVersion, SchemaSource};
+use apollo_federation_types::rover::{BuildErrors, BuildHint};
 use camino::Utf8PathBuf;
 use derive_getters::Getters;
 
@@ -79,6 +76,8 @@ pub enum CompositionError {
     ErrorUpdatingFederationVersion(#[from] InstallSupergraphError),
     #[error("Error resolving subgraphs:\n{}", .0)]
     ResolvingSubgraphsError(#[from] ResolveSupergraphConfigError),
+    #[error("Could not install supergraph binary:\n{}", .source)]
+    InstallSupergraphBinaryError { source: InstallSupergraphError },
 }
 
 #[derive(Debug, Eq, PartialEq)]

src/composition/pipeline.rs

@@ -189,7 +189,7 @@ impl CompositionPipeline<state::InstallSupergraph> {
         let supergraph_binary =
             InstallSupergraph::new(self.state.federation_version, studio_client_config)
                 .install(override_install_path, elv2_license_accepter, skip_update)
-                .await?;
+                .await;
 
         Ok(CompositionPipeline {
             state: state::Run {
@@ -244,9 +244,9 @@ impl CompositionPipeline<state::Run> {
                 error: Box::new(err),
             })?;
 
-        let result = self
-            .state
+        self.state
             .supergraph_binary
+            .clone()?
             .compose(
                 exec_command_impl,
                 read_file_impl,
@@ -255,8 +255,7 @@ impl CompositionPipeline<state::Run> {
                     .unwrap_or(OutputTarget::Stdout),
                 supergraph_config_filepath,
             )
-            .await?;
-        Ok(result)
+            .await
     }
 
     #[allow(clippy::too_many_arguments)]
@@ -383,6 +382,7 @@ mod state {
     use crate::composition::supergraph::config::full::introspect::ResolveIntrospectSubgraphFactory;
     use crate::composition::supergraph::config::resolver::fetch_remote_subgraph::FetchRemoteSubgraphFactory;
     use crate::composition::supergraph::config::resolver::InitializedSupergraphConfigResolver;
+    use crate::composition::supergraph::install::InstallSupergraphError;
     use crate::utils::parsers::FileDescriptorType;
 
     pub struct Init;
@@ -401,7 +401,7 @@ mod state {
     pub struct Run {
         pub resolver: InitializedSupergraphConfigResolver,
         pub supergraph_root: Utf8PathBuf,
-        pub supergraph_binary: SupergraphBinary,
+        pub supergraph_binary: Result<SupergraphBinary, InstallSupergraphError>,
         pub resolve_introspect_subgraph_factory: ResolveIntrospectSubgraphFactory,
         pub fetch_remote_subgraph_factory: FetchRemoteSubgraphFactory,
     }

src/composition/runner/mod.rs

@@ -3,42 +3,35 @@
 
 #![warn(missing_docs)]
 
-use std::{
-    collections::{BTreeMap, HashMap},
-    fmt::Debug,
-};
+use std::collections::{BTreeMap, HashMap};
+use std::fmt::Debug;
 
 use camino::Utf8PathBuf;
 use futures::stream::{select, BoxStream, StreamExt};
 use rover_http::HttpService;
 use tower::ServiceExt;
 
 use self::state::SetupSubgraphWatchers;
-use super::{
-    events::CompositionEvent,
-    supergraph::{
-        binary::SupergraphBinary,
-        config::{
-            error::ResolveSubgraphError,
-            full::{introspect::MakeResolveIntrospectSubgraph, FullyResolvedSupergraphConfig},
-            lazy::{LazilyResolvedSubgraph, LazilyResolvedSupergraphConfig},
-            resolver::fetch_remote_subgraph::FetchRemoteSubgraphFactory,
-        },
-    },
-    watchers::{composition::CompositionWatcher, subgraphs::SubgraphWatchers},
-    FederationUpdaterConfig,
-};
+use super::events::CompositionEvent;
+use super::supergraph::binary::SupergraphBinary;
+use super::supergraph::config::error::ResolveSubgraphError;
+use super::supergraph::config::full::introspect::MakeResolveIntrospectSubgraph;
+use super::supergraph::config::full::FullyResolvedSupergraphConfig;
+use super::supergraph::config::lazy::{LazilyResolvedSubgraph, LazilyResolvedSupergraphConfig};
+use super::supergraph::config::resolver::fetch_remote_subgraph::FetchRemoteSubgraphFactory;
+use super::watchers::composition::CompositionWatcher;
+use super::watchers::subgraphs::SubgraphWatchers;
+use super::FederationUpdaterConfig;
 use crate::composition::supergraph::binary::OutputTarget;
 use crate::composition::supergraph::config::full::introspect::ResolveIntrospectSubgraphFactory;
+use crate::composition::supergraph::install::InstallSupergraphError;
 use crate::composition::watchers::federation::FederationWatcher;
-use crate::subtask::{BroadcastSubtask, SubtaskRunUnit};
-use crate::{
-    composition::watchers::watcher::{
-        file::FileWatcher, supergraph_config::SupergraphConfigWatcher,
-    },
-    subtask::{Subtask, SubtaskRunStream},
-    utils::effect::{exec::ExecCommand, read_file::ReadFile, write_file::WriteFile},
-};
+use crate::composition::watchers::watcher::file::FileWatcher;
+use crate::composition::watchers::watcher::supergraph_config::SupergraphConfigWatcher;
+use crate::subtask::{BroadcastSubtask, Subtask, SubtaskRunStream, SubtaskRunUnit};
+use crate::utils::effect::exec::ExecCommand;
+use crate::utils::effect::read_file::ReadFile;
+use crate::utils::effect::write_file::WriteFile;
 
 mod state;
 
@@ -141,7 +134,7 @@ impl Runner<state::SetupCompositionWatcher> {
         self,
         initial_supergraph_config: FullyResolvedSupergraphConfig,
         initial_resolution_errors: BTreeMap<String, ResolveSubgraphError>,
-        supergraph_binary: SupergraphBinary,
+        supergraph_binary: Result<SupergraphBinary, InstallSupergraphError>,
         exec_command: ExecC,
         read_file: ReadF,
         write_file: WriteF,

src/composition/supergraph/install.rs

@@ -2,18 +2,15 @@ use apollo_federation_types::config::FederationVersion;
 use async_trait::async_trait;
 use camino::Utf8PathBuf;
 
-use crate::{
-    command::{install::Plugin, Install},
-    options::LicenseAccepter,
-    utils::{client::StudioClientConfig, effect::install::InstallBinary},
-};
+use super::binary::SupergraphBinary;
+use super::version::{SupergraphVersion, SupergraphVersionError};
+use crate::command::install::Plugin;
+use crate::command::Install;
+use crate::options::LicenseAccepter;
+use crate::utils::client::StudioClientConfig;
+use crate::utils::effect::install::InstallBinary;
 
-use super::{
-    binary::SupergraphBinary,
-    version::{SupergraphVersion, SupergraphVersionError},
-};
-
-#[derive(thiserror::Error, Debug)]
+#[derive(thiserror::Error, Debug, Clone)]
 pub enum InstallSupergraphError {
     #[error("ELV2 license must be accepted")]
     LicenseNotAccepted,
@@ -94,30 +91,27 @@ impl InstallBinary for InstallSupergraph {
 
 #[cfg(test)]
 mod tests {
-    use std::{str::FromStr, time::Duration};
+    use std::str::FromStr;
+    use std::time::Duration;
 
     use anyhow::Result;
     use apollo_federation_types::config::FederationVersion;
     use assert_fs::{NamedTempFile, TempDir};
     use camino::Utf8PathBuf;
-    use flate2::{write::GzEncoder, Compression};
+    use flate2::write::GzEncoder;
+    use flate2::Compression;
     use houston::Config;
     use httpmock::{Method, MockServer};
     use rstest::{fixture, rstest};
     use semver::Version;
     use speculoos::prelude::*;
     use tracing_test::traced_test;
 
-    use crate::{
-        composition::supergraph::version::SupergraphVersion,
-        options::LicenseAccepter,
-        utils::{
-            client::{ClientBuilder, ClientTimeout, StudioClientConfig},
-            effect::install::InstallBinary,
-        },
-    };
-
     use super::InstallSupergraph;
+    use crate::composition::supergraph::version::SupergraphVersion;
+    use crate::options::LicenseAccepter;
+    use crate::utils::client::{ClientBuilder, ClientTimeout, StudioClientConfig};
+    use crate::utils::effect::install::InstallBinary;
 
     #[fixture]
     #[once]

src/composition/supergraph/version.rs

@@ -1,11 +1,13 @@
-use std::{fmt::Display, str::FromStr};
+use std::fmt::Display;
+use std::str::FromStr;
+use std::sync::Arc;
 
 use apollo_federation_types::config::FederationVersion;
 use camino::Utf8PathBuf;
 use semver::Version;
 use serde_json::Value;
 
-#[derive(thiserror::Error, Debug)]
+#[derive(thiserror::Error, Debug, Clone)]
 pub enum SupergraphVersionError {
     #[error("Unsupported Federation version: {}", .version.to_string())]
     UnsupportedFederationVersion { version: SupergraphVersion },
@@ -16,7 +18,7 @@ pub enum SupergraphVersionError {
     #[error("Semver could not be extracted from the installed path")]
     InvalidVersion {
         #[from]
-        source: semver::Error,
+        source: Arc<semver::Error>,
     },
 }
 
@@ -52,7 +54,8 @@ impl TryFrom<&Utf8PathBuf> for SupergraphVersion {
             without_exe
                 .strip_prefix("supergraph-v")
                 .unwrap_or(without_exe),
-        )?;
+        )
+        .map_err(Arc::new)?;
         Ok(SupergraphVersion { version })
     }
 }
@@ -140,12 +143,11 @@ impl TryFrom<FederationVersion> for SupergraphVersion {
 mod tests {
     use std::str::FromStr;
 
-    use super::*;
     use rstest::rstest;
     use semver::Version;
     use speculoos::prelude::*;
 
-    use super::SupergraphVersion;
+    use super::{SupergraphVersion, *};
 
     fn fed_one() -> Version {
         Version::from_str("1.0.0").unwrap()