From 7311712e3353b269884b9fbf819014ec6d0bac78 Mon Sep 17 00:00:00 2001 From: jonathanrainer Date: Fri, 20 Dec 2024 06:47:28 +0000 Subject: [PATCH] ROVER-245 Extra error handling when initialising stream --- src/command/lsp/errors.rs | 10 ++- src/command/lsp/mod.rs | 117 +++++++++++++++++---------- src/composition/mod.rs | 5 +- src/composition/supergraph/binary.rs | 3 +- 4 files changed, 89 insertions(+), 46 deletions(-) diff --git a/src/command/lsp/errors.rs b/src/command/lsp/errors.rs index 53f0c572d..3224f0c15 100644 --- 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 index f5884d54c..34fb7468e 100644 --- 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, 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 index befd09560..8ea0ac111 100644 --- a/src/composition/mod.rs +++ b/src/composition/mod.rs @@ -57,7 +57,10 @@ pub enum CompositionError { error: Box, }, #[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 index d88ce90c7..9266044b9 100644 --- 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, }) }