Increase request timeout to match default retry attempts. (#1018)

peombwa · georgend · web-flow · commit 2879b49a1dc3 · 2022-01-20T12:51:53.000+03:00
* Increase HTTP client timeout.

* Add -ClientTimeout to Connect-MgGraph.

Co-authored-by: George &lt;1641829+georgend@users.noreply.github.com&gt;
diff --git a/src/Authentication/Authentication.Core/Interfaces/IAuthContext.cs b/src/Authentication/Authentication.Core/Interfaces/IAuthContext.cs
@@ -43,5 +43,6 @@ public interface IAuthContext
         ContextScope ContextScope { get; set; }
         X509Certificate2 Certificate { get; set; }
         Version PSHostVersion { get; set; }
+        TimeSpan ClientTimeout { get; set; }
     }
 }
diff --git a/src/Authentication/Authentication.Test/Helpers/HttpHelpersTests.cs b/src/Authentication/Authentication.Test/Helpers/HttpHelpersTests.cs
@@ -0,0 +1,75 @@
+﻿// ------------------------------------------------------------------------------
+//  Copyright (c) Microsoft Corporation.  All Rights Reserved.  Licensed under the MIT License.  See License in the project root for license information.
+// ------------------------------------------------------------------------------
+namespace Microsoft.Graph.Authentication.Test.Helpers
+{
+    using System;
+    using System.Net.Http;
+    using Microsoft.Graph.PowerShell.Authentication;
+    using Microsoft.Graph.PowerShell.Authentication.Helpers;
+    using Xunit;
+
+    public class HttpHelpersTests
+    {
+        [Fact]
+        public void GetGraphHttpClientWithDefaultParametersShouldReturnHttpClientWithDefaultTimeout()
+        {
+            GraphSession.Initialize(() => new GraphSession());
+            GraphSession.Instance.AuthContext = new AuthContext
+            {
+                AuthType = AuthenticationType.UserProvidedAccessToken,
+                ContextScope = ContextScope.Process
+            };
+
+            HttpClient httpClient = HttpHelpers.GetGraphHttpClient();
+
+            Assert.Equal(GraphSession.Instance.AuthContext.ClientTimeout, TimeSpan.FromSeconds(Constants.ClientTimeout));
+            Assert.Equal(httpClient.Timeout, GraphSession.Instance.AuthContext.ClientTimeout);
+
+            // reset static instance.
+            GraphSession.Reset();
+        }
+
+        [Fact]
+        public void GetGraphHttpClientWithClientTimeoutParameterShouldReturnHttpClientWithSpecifiedTimeout()
+        {
+            GraphSession.Initialize(() => new GraphSession());
+            TimeSpan timeSpan = TimeSpan.FromSeconds(10);
+            var authContext = new AuthContext
+            {
+                AuthType = AuthenticationType.UserProvidedAccessToken,
+                ContextScope = ContextScope.Process
+            };
+            IAuthenticationProvider authProvider = AuthenticationHelpers.GetAuthProvider(authContext);
+
+            HttpClient httpClient = HttpHelpers.GetGraphHttpClient(authProvider, timeSpan);
+
+            Assert.Equal(authContext.ClientTimeout, TimeSpan.FromSeconds(Constants.ClientTimeout));
+            Assert.Equal(httpClient.Timeout, timeSpan);
+
+            // reset static instance.
+            GraphSession.Reset();
+        }
+
+        [Fact]
+        public void GetGraphHttpClientShouldReturnHttpClientWithCustomerProvidedTimeout()
+        {
+            GraphSession.Initialize(() => new GraphSession());
+            TimeSpan timeSpan = TimeSpan.FromSeconds(10);
+            GraphSession.Instance.AuthContext = new AuthContext
+            {
+                AuthType = AuthenticationType.UserProvidedAccessToken,
+                ContextScope = ContextScope.Process,
+                ClientTimeout = timeSpan
+            };
+
+            HttpClient httpClient = HttpHelpers.GetGraphHttpClient();
+
+            Assert.Equal(GraphSession.Instance.AuthContext.ClientTimeout, timeSpan);
+            Assert.Equal(httpClient.Timeout, timeSpan);
+
+            // reset static instance.
+            GraphSession.Reset();
+        }
+    }
+}
diff --git a/src/Authentication/Authentication/Cmdlets/ConnectMgGraph.cs b/src/Authentication/Authentication/Cmdlets/ConnectMgGraph.cs
@@ -101,6 +101,14 @@ public class ConnectMgGraph : PSCmdlet, IModuleAssemblyInitializer, IModuleAssem
             Mandatory = false, HelpMessage = "Use device code authentication instead of a browser control")]
         [Alias("DeviceCode", "DeviceAuth", "Device")]
         public SwitchParameter UseDeviceAuthentication { get; set; }
+
+        [Parameter(ParameterSetName = Constants.AppParameterSet)]
+        [Parameter(ParameterSetName = Constants.AccessTokenParameterSet)]
+        [Parameter(ParameterSetName = Constants.UserParameterSet,
+            Mandatory = false,
+            HelpMessage = "Sets the HTTP client timeout in seconds.")]
+        [ValidateNotNullOrEmpty]
+        public double ClientTimeout { get; set; }
         /// <summary>
         ///     Wait for .NET debugger to attach
         /// </summary>
@@ -178,6 +186,7 @@ private async Task ProcessRecordAsync()
             using (NoSynchronizationContext)
             {
                 IAuthContext authContext = new AuthContext { TenantId = TenantId, PSHostVersion = this.Host.Version };
+                if (MyInvocation.BoundParameters.ContainsKey(nameof(ClientTimeout))) { authContext.ClientTimeout = TimeSpan.FromSeconds(ClientTimeout); }
                 // Set selected environment to the session object.
                 GraphSession.Instance.Environment = environment;
                 switch (ParameterSetName)
@@ -233,7 +242,6 @@ private async Task ProcessRecordAsync()
 
                 try
                 {
-
                     GraphSession.Instance.AuthContext = await Authenticator.AuthenticateAsync(authContext, ForceRefresh,
                         _cancellationTokenSource.Token,
                         () => { WriteWarning(Resources.DeviceCodeFallback); });
diff --git a/src/Authentication/Authentication/Cmdlets/InvokeMgGraphRequest.cs b/src/Authentication/Authentication/Cmdlets/InvokeMgGraphRequest.cs
@@ -568,7 +568,7 @@ private IAuthenticationProvider GetAuthProvider()
         private HttpClient GetHttpClient()
         {
             var provider = GetAuthProvider();
-            var client = HttpHelpers.GetGraphHttpClient(provider);
+            var client = HttpHelpers.GetGraphHttpClient(provider, GraphSession.Instance.AuthContext.ClientTimeout);
             return client;
         }
 
@@ -1159,15 +1159,13 @@ private async Task ProcessRecordAsync()
                 {
                     var errorRecord = new ErrorRecord(httpRequestException, ErrorCategory.ConnectionError.ToString(),
                         ErrorCategory.InvalidResult, null);
-                    httpRequestException.Data.Add(ErrorCategory.ConnectionError.ToString(), errorRecord);
-                    throw;
+                    ThrowTerminatingError(errorRecord);
                 }
                 catch (Exception exception)
                 {
                     var errorRecord = new ErrorRecord(exception, ErrorCategory.NotSpecified.ToString(),
                         ErrorCategory.InvalidOperation, null);
-                    exception.Data.Add(ErrorCategory.NotSpecified.ToString(), errorRecord);
-                    throw;
+                    ThrowTerminatingError(errorRecord);
                 }
             }
         }
diff --git a/src/Authentication/Authentication/Constants.cs b/src/Authentication/Authentication/Constants.cs
@@ -7,6 +7,7 @@ namespace Microsoft.Graph.PowerShell.Authentication
     using System.IO;
     public static class Constants
     {
+        public const double ClientTimeout = 300;
         public const int MaxContentLength = 10240;
         public const string SDKHeaderValue = "graph-powershell/{0}.{1}.{2}";
         internal const string UserParameterSet = "UserParameterSet";
diff --git a/src/Authentication/Authentication/Helpers/HttpHelpers.cs b/src/Authentication/Authentication/Helpers/HttpHelpers.cs
@@ -11,6 +11,7 @@ namespace Microsoft.Graph.PowerShell.Authentication.Helpers
     using System.Security.Authentication;
     using Microsoft.Graph.PowerShell.Authentication.Handlers;
     using System.Management.Automation;
+    using System;
 
     /// <summary>
     /// A HTTP helper class.
@@ -66,7 +67,7 @@ public static HttpClient GetGraphHttpClient(IAuthContext authContext = null)
             }
 
             IAuthenticationProvider authProvider = AuthenticationHelpers.GetAuthProvider(authContext);
-            return GetGraphHttpClient(authProvider);
+            return GetGraphHttpClient(authProvider, authContext.ClientTimeout);
         }
 
         /// <summary>
@@ -75,7 +76,7 @@ public static HttpClient GetGraphHttpClient(IAuthContext authContext = null)
         /// </summary>
         /// <param name="authProvider">Custom AuthProvider</param>
         /// <returns></returns>
-        public static HttpClient GetGraphHttpClient(IAuthenticationProvider authProvider)
+        public static HttpClient GetGraphHttpClient(IAuthenticationProvider authProvider, TimeSpan clientTimeout)
         {
             IList<DelegatingHandler> defaultHandlers = GraphClientFactory.CreateDefaultHandlers(authProvider);
 
@@ -85,6 +86,7 @@ public static HttpClient GetGraphHttpClient(IAuthenticationProvider authProvider
             defaultHandlers.Insert(2, new ODataQueryOptionsHandler());
 
             HttpClient httpClient = GraphClientFactory.Create(defaultHandlers);
+            httpClient.Timeout = clientTimeout;
 
             // Prepend SDKVersion header
             PrependSDKHeader(httpClient, CoreConstants.Headers.SdkVersionHeaderName, AuthModuleVersionHeaderValue);
diff --git a/src/Authentication/Authentication/Models/AuthContext.cs b/src/Authentication/Authentication/Models/AuthContext.cs
@@ -21,6 +21,7 @@ public class AuthContext: IAuthContext
         public ContextScope ContextScope { get ; set ; }
         public X509Certificate2 Certificate { get; set; }
         public Version PSHostVersion { get; set; }
+        public TimeSpan ClientTimeout { get; set; } = TimeSpan.FromSeconds(Constants.ClientTimeout);
 
         public AuthContext()
         {

Original file line number	Diff line number	Diff line change
`@@ -43,5 +43,6 @@ public interface IAuthContext`
`43`	`43`	`ContextScope ContextScope { get; set; }`
`44`	`44`	`X509Certificate2 Certificate { get; set; }`
`45`	`45`	`Version PSHostVersion { get; set; }`
	`46`	`+ TimeSpan ClientTimeout { get; set; }`
`46`	`47`	`}`
`47`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -568,7 +568,7 @@ private IAuthenticationProvider GetAuthProvider()`
`568`	`568`	`private HttpClient GetHttpClient()`
`569`	`569`	`{`
`570`	`570`	`var provider = GetAuthProvider();`
`571`		`- var client = HttpHelpers.GetGraphHttpClient(provider);`
	`571`	`+ var client = HttpHelpers.GetGraphHttpClient(provider, GraphSession.Instance.AuthContext.ClientTimeout);`
`572`	`572`	`return client;`
`573`	`573`	`}`
`574`	`574`
`@@ -1159,15 +1159,13 @@ private async Task ProcessRecordAsync()`
`1159`	`1159`	`{`
`1160`	`1160`	`var errorRecord = new ErrorRecord(httpRequestException, ErrorCategory.ConnectionError.ToString(),`
`1161`	`1161`	`ErrorCategory.InvalidResult, null);`
`1162`		`- httpRequestException.Data.Add(ErrorCategory.ConnectionError.ToString(), errorRecord);`
`1163`		`- throw;`
	`1162`	`+ ThrowTerminatingError(errorRecord);`
`1164`	`1163`	`}`
`1165`	`1164`	`catch (Exception exception)`
`1166`	`1165`	`{`
`1167`	`1166`	`var errorRecord = new ErrorRecord(exception, ErrorCategory.NotSpecified.ToString(),`
`1168`	`1167`	`ErrorCategory.InvalidOperation, null);`
`1169`		`- exception.Data.Add(ErrorCategory.NotSpecified.ToString(), errorRecord);`
`1170`		`- throw;`
	`1168`	`+ ThrowTerminatingError(errorRecord);`
`1171`	`1169`	`}`
`1172`	`1170`	`}`
`1173`	`1171`	`}`
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,7 @@ namespace Microsoft.Graph.PowerShell.Authentication`
`7`	`7`	`using System.IO;`
`8`	`8`	`public static class Constants`
`9`	`9`	`{`
	`10`	`+ public const double ClientTimeout = 300;`
`10`	`11`	`public const int MaxContentLength = 10240;`
`11`	`12`	`public const string SDKHeaderValue = "graph-powershell/{0}.{1}.{2}";`
`12`	`13`	`internal const string UserParameterSet = "UserParameterSet";`
Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,7 @@ public class AuthContext: IAuthContext`
`21`	`21`	`public ContextScope ContextScope { get ; set ; }`
`22`	`22`	`public X509Certificate2 Certificate { get; set; }`
`23`	`23`	`public Version PSHostVersion { get; set; }`
	`24`	`+ public TimeSpan ClientTimeout { get; set; } = TimeSpan.FromSeconds(Constants.ClientTimeout);`
`24`	`25`
`25`	`26`	`public AuthContext()`
`26`	`27`	`{`