chore(Turborepo): refactor client side of daemon-backed package discovery (#7644)

### Description

 - Set a clientside timeout at the GRPC level for package discovery
 - Refactor FallbackDiscovery to not use Option, as well as drop the impl for Option
 - Only use FallbackDiscovery when we actually want to have a fallback
 - Throw an error in the case where we've requested only daemon usage, but the daemon is unavailable

### Testing Instructions

Existing test suite


Closes TURBO-2549

Author: Greg Soltis

crates/turborepo-lib/src/daemon/client.rs

@@ -1,8 +1,8 @@
-use std::io;
+use std::{io, time::Duration};
 
 use globwalk::ValidatedGlob;
 use thiserror::Error;
-use tonic::{Code, Status};
+use tonic::{Code, IntoRequest, Status};
 use tracing::info;
 use turbopath::AbsoluteSystemPathBuf;
 
@@ -125,11 +125,10 @@ impl<T> DaemonClient<T> {
     }
 
     pub async fn discover_packages(&mut self) -> Result<DiscoverPackagesResponse, DaemonError> {
-        let response = self
-            .client
-            .discover_packages(proto::DiscoverPackagesRequest {})
-            .await?
-            .into_inner();
+        let req = proto::DiscoverPackagesRequest {};
+        let mut req = req.into_request();
+        req.set_timeout(Duration::from_millis(10));
+        let response = self.client.discover_packages(req).await?.into_inner();
 
         Ok(response)
     }

crates/turborepo-lib/src/run/mod.rs

@@ -45,7 +45,8 @@ use {
     crate::run::package_discovery::DaemonPackageDiscovery,
     std::time::Duration,
     turborepo_repository::discovery::{
-        FallbackPackageDiscovery, LocalPackageDiscoveryBuilder, PackageDiscoveryBuilder,
+        Error as DiscoveryError, FallbackPackageDiscovery, LocalPackageDiscoveryBuilder,
+        PackageDiscoveryBuilder,
     },
 };
 
@@ -286,32 +287,51 @@ impl Run {
                 .with_single_package_mode(self.opts.run_opts.single_package);
 
             #[cfg(feature = "daemon-package-discovery")]
-            let builder = {
-                // if we are forcing the daemon, we don't want to fallback to local discovery
-                let (fallback, duration) = if let Some(true) = self.opts.run_opts.daemon {
-                    (None, Duration::MAX)
-                } else {
-                    (
-                        Some(
-                            LocalPackageDiscoveryBuilder::new(
-                                self.repo_root.clone(),
-                                None,
-                                Some(root_package_json.clone()),
-                            )
-                            .build()?,
-                        ),
-                        Duration::from_millis(10),
+            let graph = match (&daemon, self.opts.run_opts.daemon) {
+                (None, Some(true)) => {
+                    // We've asked for the daemon, but it's not available. This is an error
+                    return Err(package_graph::builder::Error::Discovery(
+                        DiscoveryError::Unavailable,
+                    )
+                    .into());
+                }
+                (Some(daemon), Some(true)) => {
+                    // We have the daemon, and have explicitly asked to only use that
+                    let daemon_discovery = DaemonPackageDiscovery::new(daemon.clone());
+                    builder
+                        .with_package_discovery(daemon_discovery)
+                        .build()
+                        .await
+                }
+                (_, Some(false)) | (None, _) => {
+                    // We have explicitly requested to not use the daemon, or we don't have it
+                    // No change to default.
+                    builder.build().await
+                }
+                (Some(daemon), None) => {
+                    // We have the daemon, and it's not flagged off. Use the fallback strategy
+                    let daemon_discovery = DaemonPackageDiscovery::new(daemon.clone());
+                    let local_discovery = LocalPackageDiscoveryBuilder::new(
+                        self.repo_root.clone(),
+                        None,
+                        Some(root_package_json.clone()),
                     )
-                };
-                let fallback_discovery = FallbackPackageDiscovery::new(
-                    daemon.clone().map(DaemonPackageDiscovery::new),
-                    fallback,
-                    duration,
-                );
-                builder.with_package_discovery(fallback_discovery)
+                    .build()?;
+                    let fallback_discover = FallbackPackageDiscovery::new(
+                        daemon_discovery,
+                        local_discovery,
+                        Duration::from_millis(10),
+                    );
+                    builder
+                        .with_package_discovery(fallback_discover)
+                        .build()
+                        .await
+                }
             };
+            #[cfg(not(feature = "daemon-package-discovery"))]
+            let graph = builder.build().await;
 
-            match builder.build().await {
+            match graph {
                 Ok(graph) => graph,
                 // if we can't find the package.json, it is a bug, and we should report it.
                 // likely cause is that package discovery watching is not up to date.

crates/turborepo-repository/src/discovery.rs

@@ -66,32 +66,6 @@ pub trait PackageDiscoveryBuilder {
     fn build(self) -> Result<Self::Output, Self::Error>;
 }
 
-impl<T: PackageDiscovery + Send + Sync> PackageDiscovery for Option<T> {
-    async fn discover_packages(&self) -> Result<DiscoveryResponse, Error> {
-        tracing::debug!("discovering packages using optional strategy");
-
-        match self {
-            Some(d) => d.discover_packages().await,
-            None => {
-                tracing::debug!("no strategy available");
-                Err(Error::Unavailable)
-            }
-        }
-    }
-
-    async fn discover_packages_blocking(&self) -> Result<DiscoveryResponse, Error> {
-        tracing::debug!("discovering packages using optional strategy");
-
-        match self {
-            Some(d) => d.discover_packages_blocking().await,
-            None => {
-                tracing::debug!("no strategy available");
-                Err(Error::Unavailable)
-            }
-        }
-    }
-}
-
 pub struct LocalPackageDiscovery {
     repo_root: AbsoluteSystemPathBuf,
     package_manager: PackageManager,
@@ -194,13 +168,15 @@ impl PackageDiscovery for LocalPackageDiscovery {
 
 /// Attempts to run the `primary` strategy for an amount of time
 /// specified by `timeout` before falling back to `fallback`
-pub struct FallbackPackageDiscovery<P, F> {
+pub struct FallbackPackageDiscovery<P: PackageDiscovery + Send + Sync, F> {
     primary: P,
     fallback: F,
     timeout: std::time::Duration,
 }
 
-impl<P: PackageDiscovery, F: PackageDiscovery> FallbackPackageDiscovery<P, F> {
+impl<P: PackageDiscovery + Send + Sync, F: PackageDiscovery + Send + Sync>
+    FallbackPackageDiscovery<P, F>
+{
     pub fn new(primary: P, fallback: F, timeout: std::time::Duration) -> Self {
         Self {
             primary,