Add optional timeouts for project managers (#475)

* Update CreateProjectManager to take optional timeout parameter * Add unit test * Add test to simulate delay * Address PR comments: reduce delay time in test
microsoft · Nov 8, 2024 · 7b1a9fd · 7b1a9fd
1 parent ed0e2cf
commit 7b1a9fd
Show file tree

Hide file tree

Showing 28 changed files with 187 additions and 99 deletions.
diff --git a/src/Shared/Contracts/IBaseProjectManager.cs b/src/Shared/Contracts/IBaseProjectManager.cs
@@ -20,6 +20,8 @@ public interface IBaseProjectManager
     /// <seealso href="https://www.github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst"/>
     public string ManagerType { get; }
 
+    public TimeSpan? Timeout { get; }
+
     /// <summary>
     /// Per-object option container.
     /// </summary>

diff --git a/src/Shared/Contracts/IProjectManagerFactory.cs b/src/Shared/Contracts/IProjectManagerFactory.cs
@@ -4,6 +4,7 @@ namespace Microsoft.CST.OpenSource.Contracts;
 
 using PackageManagers;
 using PackageUrl;
+using System;
 
 public interface IProjectManagerFactory
 {
@@ -12,6 +13,7 @@ public interface IProjectManagerFactory
     /// </summary>
     /// <param name="purl">The <see cref="PackageURL"/> for the package to create the project manager for.</param>
     /// <param name="destinationDirectory">The new destination directory, if provided.</param>
+    /// <param name="timeout">The <see cref="TimeSpan"/> to wait before the request times out.</param>
     /// <returns>The implementation of <see cref="BaseProjectManager"/> for this <paramref name="purl"/>'s type.</returns>
-    IBaseProjectManager? CreateProjectManager(PackageURL purl, string destinationDirectory = ".");
+    IBaseProjectManager? CreateProjectManager(PackageURL purl, string destinationDirectory = ".", TimeSpan? timeout = null);
 }
diff --git a/src/Shared/PackageManagers/BaseProjectManager.cs b/src/Shared/PackageManagers/BaseProjectManager.cs
@@ -25,18 +25,22 @@ public abstract class BaseProjectManager : IBaseProjectManager
         /// <inheritdoc />
         public abstract string ManagerType { get; }
 
+        /// <inheritdoc/>
+        public TimeSpan? Timeout { get; } = null;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="BaseProjectManager"/> class.
         /// </summary>
-        public BaseProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory = ".")
+        public BaseProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory = ".", TimeSpan? timeout = null)
         {
             EnvironmentHelper.OverrideEnvironmentVariables(this);
             Options = new Dictionary<string, object>();
             TopLevelExtractionDirectory = destinationDirectory;
             HttpClientFactory = httpClientFactory;
+            Timeout = timeout;
         }
 
-        public BaseProjectManager(string destinationDirectory = ".") : this(new DefaultHttpClientFactory(), destinationDirectory)
+        public BaseProjectManager(string destinationDirectory = ".", TimeSpan? timeout = null) : this(new DefaultHttpClientFactory(), destinationDirectory, timeout)
         {
         }
 
@@ -524,7 +528,12 @@ protected virtual Task<Dictionary<PackageURL, double>> SearchRepoUrlsInPackageMe
 
         protected HttpClient CreateHttpClient()
         {
-            return HttpClientFactory.CreateClient(GetType().Name);
+            HttpClient client = HttpClientFactory.CreateClient(GetType().Name);
+            if (Timeout.HasValue)
+            {
+                client.Timeout = Timeout.Value;
+            }
+            return client;
         }
     }
 }
diff --git a/src/Shared/PackageManagers/CPANProjectManager.cs b/src/Shared/PackageManagers/CPANProjectManager.cs
@@ -34,11 +34,11 @@ internal class CPANProjectManager : BaseProjectManager
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0044:Add readonly modifier", Justification = "Modified through reflection.")]
         public string ENV_CPAN_API_ENDPOINT { get; set; } = "https://fastapi.metacpan.org";
 
-        public CPANProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory) : base(httpClientFactory, destinationDirectory)
+        public CPANProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory, TimeSpan? timeout = null) : base(httpClientFactory, destinationDirectory, timeout)
         {
         }
 
-        public CPANProjectManager(string destinationDirectory) : base(destinationDirectory)
+        public CPANProjectManager(string destinationDirectory, TimeSpan? timeout = null) : base(destinationDirectory, timeout)
         {
         }
 

diff --git a/src/Shared/PackageManagers/CRANProjectManager.cs b/src/Shared/PackageManagers/CRANProjectManager.cs
@@ -26,11 +26,11 @@ internal class CRANProjectManager : BaseProjectManager
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0044:Add readonly modifier", Justification = "Modified through reflection.")]
         public string ENV_CRAN_ENDPOINT { get; set; } = "https://cran.r-project.org";
 
-        public CRANProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory) : base(httpClientFactory, destinationDirectory)
+        public CRANProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory, TimeSpan? timeout = null) : base(httpClientFactory, destinationDirectory, timeout)
         {
         }
 
-        public CRANProjectManager(string destinationDirectory) : base(destinationDirectory)
+        public CRANProjectManager(string destinationDirectory, TimeSpan? timeout = null) : base(destinationDirectory, timeout)
         {
         }
 

diff --git a/src/Shared/PackageManagers/CargoProjectManager.cs b/src/Shared/PackageManagers/CargoProjectManager.cs
@@ -53,8 +53,9 @@ public class CargoProjectManager : TypedManager<IManagerPackageVersionMetadata,
         public CargoProjectManager(
             string directory,
             IManagerPackageActions<IManagerPackageVersionMetadata>? actions = null,
-            IHttpClientFactory? httpClientFactory = null)
-            : base(actions ?? new NoOpPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory)
+            IHttpClientFactory? httpClientFactory = null,
+            TimeSpan? timeout = null)
+            : base(actions ?? new NoOpPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory, timeout)
         {
         }
 

diff --git a/src/Shared/PackageManagers/CocoapodsProjectManager.cs b/src/Shared/PackageManagers/CocoapodsProjectManager.cs
@@ -17,6 +17,7 @@ namespace Microsoft.CST.OpenSource.PackageManagers
     using System.Security.Cryptography;
     using System.Text;
     using System.Text.RegularExpressions;
+    using System.Threading;
     using System.Threading.Tasks;
 
     internal class CocoapodsProjectManager : BaseProjectManager
@@ -38,11 +39,11 @@ internal class CocoapodsProjectManager : BaseProjectManager
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0044:Add readonly modifier", Justification = "Modified through reflection.")]
         public string ENV_COCOAPODS_METADATA_ENDPOINT { get; set; } = "https://cocoapods.org";
 
-        public CocoapodsProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory) : base(httpClientFactory, destinationDirectory)
+        public CocoapodsProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory, TimeSpan? timeout = null) : base(httpClientFactory, destinationDirectory, timeout)
         {
         }
 
-        public CocoapodsProjectManager(string destinationDirectory) : base(destinationDirectory)
+        public CocoapodsProjectManager(string destinationDirectory, TimeSpan? timeout = null) : base(destinationDirectory, timeout)
         {
         }
 

diff --git a/src/Shared/PackageManagers/ComposerProjectManager.cs b/src/Shared/PackageManagers/ComposerProjectManager.cs
@@ -26,11 +26,11 @@ internal class ComposerProjectManager : BaseProjectManager
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0044:Add readonly modifier", Justification = "Modified through reflection.")]
         public string ENV_COMPOSER_ENDPOINT { get; set; } = "https://repo.packagist.org";
 
-        public ComposerProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory) : base(httpClientFactory, destinationDirectory)
+        public ComposerProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory, TimeSpan? timeout = null) : base(httpClientFactory, destinationDirectory, timeout)
         {
         }
 
-        public ComposerProjectManager(string destinationDirectory) : base(destinationDirectory)
+        public ComposerProjectManager(string destinationDirectory, TimeSpan? timeout = null) : base(destinationDirectory, timeout)
         {
         }
 

diff --git a/src/Shared/PackageManagers/CondaProjectManager.cs b/src/Shared/PackageManagers/CondaProjectManager.cs
@@ -33,8 +33,9 @@ public class CondaProjectManager : TypedManager<IManagerPackageVersionMetadata,
         public CondaProjectManager(
             string directory,
             IManagerPackageActions<IManagerPackageVersionMetadata>? actions = null,
-            IHttpClientFactory? httpClientFactory = null)
-            : base(actions ?? new NoOpPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory)
+            IHttpClientFactory? httpClientFactory = null,
+            TimeSpan? timeout = null)
+            : base(actions ?? new NoOpPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory, timeout)
         {
         }
 

diff --git a/src/Shared/PackageManagers/GemProjectManager.cs b/src/Shared/PackageManagers/GemProjectManager.cs
@@ -27,11 +27,11 @@ internal class GemProjectManager : BaseProjectManager
         public string ENV_RUBYGEMS_ENDPOINT { get; set; } = "https://rubygems.org";
         public string ENV_RUBYGEMS_ENDPOINT_API { get; set; } = "https://api.rubygems.org";
 
-        public GemProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory) : base(httpClientFactory, destinationDirectory)
+        public GemProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory, TimeSpan? timeout = null) : base(httpClientFactory, destinationDirectory, timeout)
         {
         }
 
-        public GemProjectManager(string destinationDirectory) : base(destinationDirectory)
+        public GemProjectManager(string destinationDirectory, TimeSpan? timeout = null) : base(destinationDirectory, timeout)
         {
         }
 

diff --git a/src/Shared/PackageManagers/GitHubProjectManager.cs b/src/Shared/PackageManagers/GitHubProjectManager.cs
@@ -28,11 +28,11 @@ internal class GitHubProjectManager : BaseProjectManager
         private const string DEFAULT_GITHUB_ENDPOINT = "https://github.com";
         public string ENV_GITHUB_ENDPOINT { get; set; } = DEFAULT_GITHUB_ENDPOINT;
 
-        public GitHubProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory) : base(httpClientFactory, destinationDirectory)
+        public GitHubProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory, TimeSpan? timeout = null) : base(httpClientFactory, destinationDirectory, timeout)
         {
         }
 
-        public GitHubProjectManager(string destinationDirectory) : base(destinationDirectory)
+        public GitHubProjectManager(string destinationDirectory, TimeSpan? timeout = null) : base(destinationDirectory, timeout)
         {
         }
 

diff --git a/src/Shared/PackageManagers/GolangProjectManager.cs b/src/Shared/PackageManagers/GolangProjectManager.cs
@@ -37,8 +37,9 @@ public class GolangProjectManager : TypedManager<IManagerPackageVersionMetadata,
         public GolangProjectManager(
             string directory,
             IManagerPackageActions<IManagerPackageVersionMetadata>? actions = null,
-            IHttpClientFactory? httpClientFactory = null)
-            : base(actions ?? new NoOpPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory)
+            IHttpClientFactory? httpClientFactory = null,
+            TimeSpan? timeout = null)
+            : base(actions ?? new NoOpPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory, timeout)
         {
         }
 

diff --git a/src/Shared/PackageManagers/HackageProjectManager.cs b/src/Shared/PackageManagers/HackageProjectManager.cs
@@ -26,11 +26,11 @@ internal class HackageProjectManager : BaseProjectManager
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0044:Add readonly modifier", Justification = "Modified through reflection.")]
         public string ENV_HACKAGE_ENDPOINT { get; set; } = "https://hackage.haskell.org";
 
-        public HackageProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory) : base(httpClientFactory, destinationDirectory)
+        public HackageProjectManager(IHttpClientFactory httpClientFactory, string destinationDirectory, TimeSpan? timeout = null) : base(httpClientFactory, destinationDirectory, timeout)
         {
         }
 
-        public HackageProjectManager(string destinationDirectory) : base(destinationDirectory)
+        public HackageProjectManager(string destinationDirectory, TimeSpan? timeout = null) : base(destinationDirectory, timeout)
         {
         }
 

diff --git a/src/Shared/PackageManagers/MavenProjectManager.cs b/src/Shared/PackageManagers/MavenProjectManager.cs
@@ -33,8 +33,9 @@ public class MavenProjectManager : TypedManager<IManagerPackageVersionMetadata,
         public MavenProjectManager(
             string directory,
             IManagerPackageActions<IManagerPackageVersionMetadata>? actions = null,
-            IHttpClientFactory? httpClientFactory = null)
-            : base(actions ?? new NoOpPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory)
+            IHttpClientFactory? httpClientFactory = null,
+            TimeSpan? timeout = null)
+            : base(actions ?? new NoOpPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory, timeout)
         {
         }
 

diff --git a/src/Shared/PackageManagers/NPMProjectManager.cs b/src/Shared/PackageManagers/NPMProjectManager.cs
@@ -43,8 +43,9 @@ public class NPMProjectManager : TypedManager<IManagerPackageVersionMetadata, NP
         public NPMProjectManager(
             string directory,
             IManagerPackageActions<IManagerPackageVersionMetadata>? actions = null,
-            IHttpClientFactory? httpClientFactory = null)
-            : base(actions ?? new NoOpPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory)
+            IHttpClientFactory? httpClientFactory = null,
+            TimeSpan? timeout = null)
+            : base(actions ?? new NoOpPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory, timeout)
         {
         }
 
@@ -129,6 +130,11 @@ public override async Task<IEnumerable<string>> DownloadVersionAsync(PackageURL
                     downloadedPaths.Add(extractionPath);
                 }
             }
+            catch (TaskCanceledException ex)
+            {
+                Logger.Debug(ex, "Downloading NPM package timed out: {0}", ex.Message);
+                throw;
+            }
             catch (Exception ex)
             {
                 Logger.Debug(ex, "Error downloading NPM package: {0}", ex.Message);

diff --git a/src/Shared/PackageManagers/NuGetProjectManager.cs b/src/Shared/PackageManagers/NuGetProjectManager.cs
@@ -46,8 +46,9 @@ public class NuGetProjectManager : TypedManager<NuGetPackageVersionMetadata, NuG
         public NuGetProjectManager(
             string directory,
             IManagerPackageActions<NuGetPackageVersionMetadata>? actions = null,
-            IHttpClientFactory? httpClientFactory = null)
-            : base(actions ?? new NuGetPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory)
+            IHttpClientFactory? httpClientFactory = null,
+            TimeSpan? timeout = null)
+            : base(actions ?? new NuGetPackageActions(), httpClientFactory ?? new DefaultHttpClientFactory(), directory, timeout)
         {
             GetRegistrationEndpointAsync().Wait();
         }