ROVER-245 Extra error handling when initialising stream

apollographql · Dec 20, 2024 · e443b00 · e443b00
1 parent 12f9c70
commit e443b00
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 46 deletions.
diff --git a/src/command/lsp/errors.rs b/src/command/lsp/errors.rs
@@ -1,5 +1,7 @@
-use camino::Utf8PathBuf;
+use camino::{FromPathBufError, Utf8PathBuf};
 
+use crate::composition::supergraph::config::resolver::ResolveSupergraphConfigError;
+use crate::composition::supergraph::install::InstallSupergraphError;
 use crate::composition::CompositionError;
 
 #[derive(thiserror::Error, Debug)]
@@ -8,4 +10,10 @@ pub enum StartCompositionError {
     SupergraphYamlUrlConversionFailed(Utf8PathBuf),
     #[error("Could not run initial composition")]
     InitialCompositionFailed(#[from] CompositionError),
+    #[error("Could not install supergraph plugin")]
+    InstallSupergraphPluginFailed(#[from] InstallSupergraphError),
+    #[error("Could not resolve Supergraph Config")]
+    ResolvingSupergraphConfigFailed(#[from] ResolveSupergraphConfigError),
+    #[error("Could not establish temporary directory")]
+    TemporaryDirectoryCouldNotBeEstablished(#[from] FromPathBufError),
 }
diff --git a/src/command/lsp/mod.rs b/src/command/lsp/mod.rs
@@ -4,11 +4,12 @@ use std::collections::HashMap;
 use std::env::temp_dir;
 use std::io::stdin;
 
-use anyhow::Error;
+use apollo_federation_types::config::FederationVersion;
 use apollo_federation_types::config::FederationVersion::LatestFedTwo;
 use apollo_language_server::{ApolloLanguageServer, Config, MaxSpecVersions};
 use camino::Utf8PathBuf;
 use clap::Parser;
+use futures::stream::BoxStream;
 use futures::StreamExt;
 use serde::Serialize;
 use tower_lsp::lsp_types::{Diagnostic, Range};
@@ -197,44 +198,24 @@ async fn start_composition(
 
     // Spawn a separate thread to handle composition and passing that data to the language server
     tokio::spawn(async move {
-        // TODO: Let the supergraph binary exist inside its own task that can respond to being re-installed etc.
-        let supergraph_binary =
-            InstallSupergraph::new(federation_version.clone(), client_config.clone())
-                .install(
-                    None,
-                    lsp_opts.plugin_opts.elv2_license_accepter,
-                    lsp_opts.plugin_opts.skip_update,
-                )
-                .await?;
-
-        let make_fetch_remote_subgraph = MakeFetchRemoteSubgraph::builder()
-            .studio_client_config(client_config.clone())
-            .profile(lsp_opts.plugin_opts.profile.clone())
-            .build();
-
-        // Spin up Runner
-        let mut stream = Runner::default()
-            .setup_subgraph_watchers(
-                lazily_resolved_supergraph_config.subgraphs().clone(),
-                &lsp_opts.plugin_opts.profile,
-                &client_config,
-                500,
-            )
-            .setup_supergraph_config_watcher(lazily_resolved_supergraph_config.clone())
-            .setup_composition_watcher(
-                lazily_resolved_supergraph_config
-                    .fully_resolve(&client_config, make_fetch_remote_subgraph)
-                    .await
-                    .map_err(ResolveSupergraphConfigError::ResolveSubgraphs)?,
-                supergraph_binary,
-                TokioCommand::default(),
-                FsReadFile::default(),
-                FsWriteFile::default(),
-                Utf8PathBuf::try_from(temp_dir())?,
-                true,
-                OutputTarget::InMemory,
-            )
-            .run();
+        let mut stream = match create_composition_stream(
+            lazily_resolved_supergraph_config,
+            client_config,
+            lsp_opts,
+            federation_version,
+        )
+        .await
+        {
+            Ok(stream) => stream,
+            Err(e) => {
+                let message = format!("Could not initialise composition process: {e}");
+                let diagnostic = Diagnostic::new_simple(Range::default(), message);
+                language_server
+                    .publish_diagnostics(supergraph_yaml_url.clone(), vec![diagnostic])
+                    .await;
+                return Err(e);
+            }
+        };
 
         while let Some(event) = stream.next().await {
             match event {
@@ -263,7 +244,10 @@ async fn start_composition(
                         )
                         .await;
                 }
-                CompositionEvent::Error(CompositionError::Build { source: errors }) => {
+                CompositionEvent::Error(CompositionError::Build {
+                    source: errors,
+                    federation_version,
+                }) => {
                     debug!(
                         ?errors,
                         "Composition {federation_version} completed with errors"
@@ -281,8 +265,8 @@ async fn start_composition(
                         .await
                 }
                 CompositionEvent::Error(err) => {
-                    debug!("Composition {federation_version} failed: {err}");
-                    let message = format!("Failed run composition {federation_version}: {err}",);
+                    debug!("Composition failed: {err}");
+                    let message = format!("Failed run composition: {err}",);
                     let diagnostic = Diagnostic::new_simple(Range::default(), message);
                     language_server
                         .publish_diagnostics(supergraph_yaml_url.clone(), vec![diagnostic])
@@ -301,7 +285,54 @@ async fn start_composition(
                 }
             }
         }
-        Ok::<(), Error>(())
+        Ok::<(), StartCompositionError>(())
     });
     Ok(())
 }
+
+async fn create_composition_stream(
+    lazily_resolved_supergraph_config: LazilyResolvedSupergraphConfig,
+    client_config: StudioClientConfig,
+    lsp_opts: LspOpts,
+    federation_version: FederationVersion,
+) -> Result<BoxStream<'static, CompositionEvent>, StartCompositionError> {
+    // TODO: Let the supergraph binary exist inside its own task that can respond to being re-installed etc.
+    let supergraph_binary =
+        InstallSupergraph::new(federation_version.clone(), client_config.clone())
+            .install(
+                None,
+                lsp_opts.plugin_opts.elv2_license_accepter,
+                lsp_opts.plugin_opts.skip_update,
+            )
+            .await?;
+
+    let make_fetch_remote_subgraph = MakeFetchRemoteSubgraph::builder()
+        .studio_client_config(client_config.clone())
+        .profile(lsp_opts.plugin_opts.profile.clone())
+        .build();
+
+    // Spin up Runner
+    let stream = Runner::default()
+        .setup_subgraph_watchers(
+            lazily_resolved_supergraph_config.subgraphs().clone(),
+            &lsp_opts.plugin_opts.profile,
+            &client_config,
+            500,
+        )
+        .setup_supergraph_config_watcher(lazily_resolved_supergraph_config.clone())
+        .setup_composition_watcher(
+            lazily_resolved_supergraph_config
+                .fully_resolve(&client_config, make_fetch_remote_subgraph)
+                .await
+                .map_err(ResolveSupergraphConfigError::ResolveSubgraphs)?,
+            supergraph_binary,
+            TokioCommand::default(),
+            FsReadFile::default(),
+            FsWriteFile::default(),
+            Utf8PathBuf::try_from(temp_dir())?,
+            true,
+            OutputTarget::InMemory,
+        )
+        .run();
+    Ok(stream)
+}
diff --git a/src/composition/mod.rs b/src/composition/mod.rs
@@ -57,7 +57,10 @@ pub enum CompositionError {
         error: Box<dyn std::error::Error + Send + Sync>,
     },
     #[error("Encountered {} while trying to build a supergraph.", .source.length_string())]
-    Build { source: BuildErrors },
+    Build {
+        source: BuildErrors,
+        federation_version: FederationVersion,
+    },
     #[error("Serialization error.\n{}", .0)]
     SerdeYaml(#[from] serde_yaml::Error),
 }

diff --git a/src/composition/supergraph/binary.rs b/src/composition/supergraph/binary.rs
@@ -173,10 +173,11 @@ impl SupergraphBinary {
             .map(|build_output| CompositionSuccess {
                 hints: build_output.hints,
                 supergraph_sdl: build_output.supergraph_sdl,
-                federation_version,
+                federation_version: federation_version.clone(),
             })
             .map_err(|build_errors| CompositionError::Build {
                 source: build_errors,
+                federation_version,
             })
     }