Merge pull request 6766 from hotfix/v4.3.16 into master

FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngine.cs

@@ -234,8 +234,7 @@ protected override void ProcessEngine(IFlowData data, CloudRequestData aspectDat
                 }
 
                 requestMessage.Content = content;
-                var request = AddCommonHeadersAndSend(requestMessage);
-                jsonResult = ProcessResponse(request.Result);
+                jsonResult = ProcessResponse(AddCommonHeadersAndSend(requestMessage));
             }
 
             aspectData.JsonResponse = jsonResult;
@@ -470,8 +469,7 @@ private void GetCloudProperties()
             using (var requestMessage =
                 new HttpRequestMessage(HttpMethod.Get, url))
             {
-                var request = AddCommonHeadersAndSend(requestMessage);
-                jsonResult = ProcessResponse(request.Result);
+                jsonResult = ProcessResponse(AddCommonHeadersAndSend(requestMessage));
             }
 
             if (string.IsNullOrEmpty(jsonResult) == false)
@@ -498,10 +496,10 @@ private void GetCloudEvidenceKeys()
             using (var requestMessage =
                 new HttpRequestMessage(HttpMethod.Get, _evidenceKeysEndpoint))
             {
-                var request = AddCommonHeadersAndSend(requestMessage);
                 // Note - Don't check for error messages in the response
                 // as it is a flat JSON array.
-                jsonResult = ProcessResponse(request.Result, false);
+                jsonResult = ProcessResponse(
+                    AddCommonHeadersAndSend(requestMessage), false);
             }
 
             if (string.IsNullOrEmpty(jsonResult) == false)
@@ -527,7 +525,7 @@ private void GetCloudEvidenceKeys()
         /// <returns>
         /// The response
         /// </returns>
-        private Task<HttpResponseMessage> AddCommonHeadersAndSend(
+        private HttpResponseMessage AddCommonHeadersAndSend(
             HttpRequestMessage request)
         {
             if (string.IsNullOrEmpty(_cloudRequestOrigin) == false &&
@@ -537,7 +535,32 @@ private Task<HttpResponseMessage> AddCommonHeadersAndSend(
                 request.Headers.Add(Constants.ORIGIN_HEADER_NAME, _cloudRequestOrigin);
             }
 
-            return _httpClient.SendAsync(request);
+            return SendRequestAsync(request);
         }
+
+        /// <summary>
+        /// Send a request and handle any exception if one is thrown.
+        /// </summary>
+        /// <param name="request">
+        /// The request to send
+        /// </param>
+        /// <returns>
+        /// The response
+        /// </returns>
+        private HttpResponseMessage SendRequestAsync(
+            HttpRequestMessage request)
+        {
+            var task = _httpClient.SendAsync(request);
+
+            try
+            {
+                return task.Result;
+            }
+            catch (Exception ex)
+            {
+                throw new CloudRequestException(
+                    Messages.ExceptionCloudResponseFailure, ex);
+            }
+        } 
     }
 }

FiftyOne.Pipeline.CloudRequestEngine/Messages.resx

@@ -129,12 +129,18 @@
   <data name="ExceptionCloudPropertyType" xml:space="preserve">
     <value>Unable to determine the type associated with this property. No c# property found matching definition {0}.{1} and json type '{2}' is not mapped.   </value>
   </data>
+  <data name="ExceptionCloudResponseFailure" xml:space="preserve">
+    <value>Error waiting for response from the cloud service.</value>
+  </data>
   <data name="ExceptionFailedToLoadProperties" xml:space="preserve">
     <value>Failed to load aspect properties for element '{0}'. This is because your resource key does not include access to any properties under '{0}'. For more details on resource keys, see our explainer: https://51degrees.com/documentation/_info__resource_keys.html</value>
   </data>
   <data name="ExceptionResourceKeyNeeded" xml:space="preserve">
     <value>A resource key is required to access the 51Degrees cloud service. Please use the 'SetResourceKey' method to supply your resource key. You can obtain one for free from https://configure.51degrees.com</value>
   </data>
+  <data name="ExceptionTaskCanceledRequest" xml:space="preserve">
+    <value>Failed to send request. A TaskCanceledException was thrown. The likely cause of this would be a connection timeout.</value>
+  </data>
   <data name="MessageErrorCodeReturned" xml:space="preserve">
     <value>Cloud service at '{0}' returned status code '{1}' with content {2}.</value>
   </data>

FiftyOne.Pipeline.Core/FlowElements/PipelineBuilderBase.cs

@@ -60,7 +60,7 @@ public abstract class PipelineBuilderBase<T>
         /// If true then Pipeline will call Dispose on its child elements
         /// when it is disposed.
         /// </summary>
-        private bool _autoDisposeElements = false;
+        private bool _autoDisposeElements = true;
 
         /// <summary>
         /// If true then Pipeline will suppress exceptions added to

Tests/FiftyOne.Pipeline.CloudRequestEngine.Tests/CloudRequestEngineTests.cs

@@ -33,6 +33,7 @@
 using System.Linq;
 using System.Net;
 using System.Net.Http;
+using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -836,34 +837,85 @@ public void ValidateErrorHandling_HttpDataSetInException()
                 Assert.IsNotNull(ex.ResponseHeaders, "Response headers not populated");
                 Assert.IsTrue(ex.ResponseHeaders.Count > 0, "Response headers not populated");
             }
+        }
+
+        /// <summary>
+        /// Verify that an exception throw by the task that is returned by HttpClient.SendAsync
+        /// will be handled and wrapped in nice informative CloudRequestException. 
+        /// </summary>
+        [TestMethod]
+        public void ValidateErrorHandling_ExceptionInRequestTask()
+        {
+            string resourceKey = "resource_key";
+            string userAgent = "iPhone";
+            Exception exception = null;
+
+            ConfigureMockedClient(r => true, true);
+            var engine = new CloudRequestEngineBuilder(_loggerFactory, _httpClient)
+                .SetResourceKey(resourceKey)
+                .Build();
+
+            try
+            {
+                using (var pipeline = new PipelineBuilder(_loggerFactory).AddFlowElement(engine).Build())
+                {
+                    var data = pipeline.CreateFlowData();
+                    data.AddEvidence("query.User-Agent", userAgent);
+                    data.Process();
+                }
+            }
+            catch (Exception ex)
+            {
+                exception = ex;
+            }
 
+            Assert.IsNotNull(exception, "Expected exception to occur");
+            Assert.IsInstanceOfType(exception, typeof(AggregateException));
+            var aggEx = exception as AggregateException;
+            Assert.AreEqual(aggEx.InnerExceptions.Count, 1);
+            var realEx = aggEx.InnerExceptions[0];
+            Assert.IsInstanceOfType(realEx, typeof(CloudRequestException));
         }
 
         /// <summary>
         /// Setup _httpClient to respond with the configured messages.
         /// </summary>
         private void ConfigureMockedClient(
-            Func<HttpRequestMessage, bool> expectedJsonParameters)
+            Func<HttpRequestMessage, bool> expectedJsonParameters,
+            bool throwExceptionOnJsonRequest = false)
         {
             // ARRANGE
             _handlerMock = new Mock<HttpMessageHandler>(MockBehavior.Strict);
             // Set up the JSON response.
-            _handlerMock
+            var setup = _handlerMock
                .Protected()
                // Setup the PROTECTED method to mock
                .Setup<Task<HttpResponseMessage>>(
                   "SendAsync",
                   ItExpr.Is<HttpRequestMessage>(r => expectedJsonParameters(r)
                       && r.RequestUri.AbsolutePath.ToLower().EndsWith("json")),
                   ItExpr.IsAny<CancellationToken>()
-               )
-               // prepare the expected response of the mocked http call
-               .ReturnsAsync(new HttpResponseMessage()
-               {
-                   StatusCode = HttpStatusCode.OK,
-                   Content = new StringContent(_jsonResponse),
-               })
+               );
+
+            if (throwExceptionOnJsonRequest)
+            {
+                // Configure the call to the json endpoint to throw an exception.
+                var task = new Task<HttpResponseMessage>(() => throw new Exception("TEST"));
+                // We have to start the task or it will never actually run!
+                task.Start();
+                setup.Returns(task);
+            } 
+            else 
+            { 
+               // Prepare the expected response of the mocked http call
+               setup.ReturnsAsync(new HttpResponseMessage()
+                {
+                    StatusCode = HttpStatusCode.OK,
+                    Content = new StringContent(_jsonResponse),
+                })
                .Verifiable();
+            }
+
             // Set up the evidencekeys response.
             _handlerMock
                .Protected()